Skip to content

Commit 4edcea1

Browse files
committed
MbedCRC: handle init values better
Init values often need reflection, and use of `__RBIT` prevents constant init being done at compile time (unless `__RBIT` uses a compiler intrinsic, which it doesn't for GCC). Rather than try to handle constants 0U and -1U with a special case to avoid the RBIT, which can in turn lead to runtime bloat for nonconstant inits, use a C++20 style is_constant_evaluated() check to switch between C and assembly forms. This reduces code-size for non-constant init, by eliminating a runtime condition, and allows the bit-reversal of any constant init to happen at compile time.
1 parent 27719da commit 4edcea1

File tree

1 file changed

+98
-76
lines changed

1 file changed

+98
-76
lines changed

drivers/MbedCRC.h

Lines changed: 98 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include "platform/SingletonPtr.h"
3030
#include "platform/PlatformMutex.h"
3131

32-
#include <type_traits>
32+
#include <mstd_type_traits>
3333

3434
namespace mbed {
3535
/** \addtogroup drivers-public-api */
@@ -175,11 +175,13 @@ class MbedCRC {
175175
* polynomials with different initial/final/reflect values
176176
*
177177
*/
178+
MSTD_CONSTEXPR_FN_14
178179
MbedCRC(uint32_t initial_xor, uint32_t final_xor, bool reflect_data, bool reflect_remainder) :
179180
crc_impl(initial_xor, final_xor, reflect_data, reflect_remainder)
180181
{
181182
}
182183

184+
MSTD_CONSTEXPR_FN_14
183185
MbedCRC();
184186

185187
/** Compute CRC for the data input
@@ -279,6 +281,7 @@ class MbedCRC {
279281
public:
280282
typedef size_t crc_data_size_t;
281283

284+
MSTD_CONSTEXPR_FN_14
282285
MbedCRC(uint32_t initial_xor, uint32_t final_xor, bool reflect_data, bool reflect_remainder) :
283286
_initial_value(adjust_initial_value(initial_xor, reflect_data)),
284287
_final_xor(final_xor),
@@ -400,7 +403,7 @@ class MbedCRC {
400403
}
401404
} else {
402405
/* CRC has MSB in top bit of register */
403-
p_crc = _reflect_remainder ? reflect_register(p_crc) : shift_right(p_crc);
406+
p_crc = _reflect_remainder ? reflect(p_crc) : shift_right(p_crc);
404407
}
405408
} else { // TABLE
406409
/* CRC has MSB in bottom bit of register */
@@ -417,45 +420,91 @@ class MbedCRC {
417420
}
418421

419422
private:
423+
/** Guaranteed constexpr reflection (all toolchains)
424+
*
425+
* @note This should never be run-time evaluated - very inefficient
426+
* @param Register value to be reflected (full 32-bit value)
427+
* @return Reflected value (full 32-bit value)
428+
*/
429+
static constexpr uint32_t reflect_constant(uint32_t data)
430+
{
431+
/* Doing this hard way to keep it C++11 constexpr and hence ARM C 5 compatible */
432+
return ((data & 0x00000001) << 31) |
433+
((data & 0x00000002) << 29) |
434+
((data & 0x00000004) << 27) |
435+
((data & 0x00000008) << 25) |
436+
((data & 0x00000010) << 23) |
437+
((data & 0x00000020) << 21) |
438+
((data & 0x00000040) << 19) |
439+
((data & 0x00000080) << 17) |
440+
((data & 0x00000100) << 15) |
441+
((data & 0x00000200) << 13) |
442+
((data & 0x00000400) << 11) |
443+
((data & 0x00000800) << 9) |
444+
((data & 0x00001000) << 7) |
445+
((data & 0x00002000) << 5) |
446+
((data & 0x00004000) << 3) |
447+
((data & 0x00008000) << 1) |
448+
((data & 0x00010000) >> 1) |
449+
((data & 0x00020000) >> 3) |
450+
((data & 0x00040000) >> 5) |
451+
((data & 0x00080000) >> 7) |
452+
((data & 0x00100000) >> 9) |
453+
((data & 0x00200000) >> 11) |
454+
((data & 0x00400000) >> 13) |
455+
((data & 0x00800000) >> 15) |
456+
((data & 0x01000000) >> 17) |
457+
((data & 0x02000000) >> 19) |
458+
((data & 0x04000000) >> 21) |
459+
((data & 0x08000000) >> 23) |
460+
((data & 0x10000000) >> 25) |
461+
((data & 0x20000000) >> 27) |
462+
((data & 0x40000000) >> 29) |
463+
((data & 0x80000000) >> 31);
464+
}
465+
466+
/** General reflection
467+
*
468+
* @note This is used when we may need to perform run-time computation, so
469+
* we need the possibility to produce the optimal run-time RBIT instruction. But
470+
* if the compiler doesn't treat RBIT as a built-in, it's useful to have a C fallback
471+
* for the constant case, avoiding runtime RBIT(0) computations. This is an
472+
* optimization only available for some toolchains; others will always use runtime
473+
* RBIT. If we require a constant expression, use reflect_constant instead.
474+
*
475+
* @param Register value to be reflected (full 32-bit value)
476+
* @return Reflected value (full 32-bit value)
477+
*/
478+
#ifdef MSTD_HAS_IS_CONSTANT_EVALUATED
479+
static constexpr uint32_t reflect(uint32_t data)
480+
{
481+
return mstd::is_constant_evaluated() ? reflect_constant(data) : __RBIT(data);
482+
}
483+
#else
484+
static uint32_t reflect(uint32_t data)
485+
{
486+
return __RBIT(data);
487+
}
488+
#endif
489+
490+
/** Data bytes may need to be reflected.
491+
*
492+
* @param data value to be reflected (bottom 8 bits)
493+
* @return Reflected value (bottom 8 bits)
494+
*/
495+
static MSTD_CONSTEXPR_IF_HAS_IS_CONSTANT_EVALUATED
496+
uint_fast32_t reflect_byte(uint_fast32_t data)
497+
{
498+
return reflect(data) >> 24;
499+
}
500+
420501
/** Get the current CRC polynomial, reflected at bottom of register.
421502
*
422503
* @return Reflected polynomial value (so x^width term would be at bit -1)
423504
*/
424505
static constexpr uint32_t get_reflected_polynomial()
425506
{
426-
/* Doing this hard way to keep it C++11 constexpr and hence ARM C 5 compatible */
427-
return shift_right(((polynomial & 0x00000001) << 31) |
428-
((polynomial & 0x00000002) << 29) |
429-
((polynomial & 0x00000004) << 27) |
430-
((polynomial & 0x00000008) << 25) |
431-
((polynomial & 0x00000010) << 23) |
432-
((polynomial & 0x00000020) << 21) |
433-
((polynomial & 0x00000040) << 19) |
434-
((polynomial & 0x00000080) << 17) |
435-
((polynomial & 0x00000100) << 15) |
436-
((polynomial & 0x00000200) << 13) |
437-
((polynomial & 0x00000400) << 11) |
438-
((polynomial & 0x00000800) << 9) |
439-
((polynomial & 0x00001000) << 7) |
440-
((polynomial & 0x00002000) << 5) |
441-
((polynomial & 0x00004000) << 3) |
442-
((polynomial & 0x00008000) << 1) |
443-
((polynomial & 0x00010000) >> 1) |
444-
((polynomial & 0x00020000) >> 3) |
445-
((polynomial & 0x00040000) >> 5) |
446-
((polynomial & 0x00080000) >> 7) |
447-
((polynomial & 0x00100000) >> 9) |
448-
((polynomial & 0x00200000) >> 11) |
449-
((polynomial & 0x00400000) >> 13) |
450-
((polynomial & 0x00800000) >> 15) |
451-
((polynomial & 0x01000000) >> 17) |
452-
((polynomial & 0x02000000) >> 19) |
453-
((polynomial & 0x04000000) >> 21) |
454-
((polynomial & 0x08000000) >> 23) |
455-
((polynomial & 0x10000000) >> 25) |
456-
((polynomial & 0x20000000) >> 27) |
457-
((polynomial & 0x40000000) >> 29) |
458-
((polynomial & 0x80000000) >> 31));
507+
return shift_right(reflect_constant(polynomial));
459508
}
460509

461510
/** Get the current CRC polynomial, at top of register.
@@ -484,21 +533,8 @@ class MbedCRC {
484533
static const crc_table_t _crc_table[MBED_CRC_TABLE_SIZE];
485534
#endif
486535

487-
static uint32_t adjust_initial_value(uint32_t initial_xor, bool reflect_data)
536+
static MSTD_CONSTEXPR_FN_14 uint32_t adjust_initial_value(uint32_t initial_xor, bool reflect_data)
488537
{
489-
/* As initial_xor is almost certain to be constant all zeros or ones, try to
490-
* process that a constant, avoiding an RBIT instruction (or worse).
491-
*/
492-
if (initial_xor == 0 || initial_xor == (get_crc_mask() & -1U)) {
493-
/* Only possible adjustment is shifting to top for bitwise */
494-
if (mode == CrcMode::BITWISE && !reflect_data) {
495-
return shift_left(initial_xor);
496-
} else {
497-
return initial_xor;
498-
}
499-
}
500-
501-
/* Weird or non-constant initial value - need to think about reflection */
502538
if (mode == CrcMode::BITWISE) {
503539
/* For bitwise calculation, CRC register is reflected if data is, to match input.
504540
* (MSB at bottom of register). If not reflected, it is at the top of the register
@@ -545,34 +581,15 @@ class MbedCRC {
545581
return (uint32_t)((uint32_t)2U << (width - 1)) - 1U;
546582
}
547583

548-
/** Data bytes may need to be reflected.
549-
*
550-
* @param data value to be reflected (bottom 8 bits)
551-
* @return Reflected value (bottom 8 bits)
552-
*/
553-
static uint_fast32_t reflect_byte(uint_fast32_t data)
554-
{
555-
return __RBIT(data) >> 24;
556-
}
557-
558584
/** CRC values may need to be reflected.
559585
*
560586
* @param CRC value to be reflected (width bits at bottom of 32-bit word)
561587
* @return Reflected value (still at bottom of 32-bit word)
562588
*/
563-
static uint32_t reflect_crc(uint32_t data)
564-
{
565-
return __RBIT(data) >> (32 - width);
566-
}
567-
568-
/** Register values may need to be reflected.
569-
*
570-
* @param Register value to be reflected (full 32-bit value)
571-
* @return Reflected value (full 32-bit value)
572-
*/
573-
static uint32_t reflect_register(uint32_t data)
589+
static MSTD_CONSTEXPR_IF_HAS_IS_CONSTANT_EVALUATED
590+
uint32_t reflect_crc(uint32_t data)
574591
{
575-
return __RBIT(data);
592+
return reflect(data) >> (32 - width);
576593
}
577594

578595
/** Register values may need to be shifted left.
@@ -823,27 +840,32 @@ const uint32_t MbedCRC<POLY_32BIT_ANSI, 32, CrcMode::TABLE>::_crc_table[MBED_CRC
823840
/* Default values for different types of polynomials
824841
*/
825842
template<>
826-
inline MbedCRC<POLY_32BIT_ANSI, 32>::MbedCRC() : MbedCRC(0xFFFFFFFF, 0xFFFFFFFF, true, true)
843+
inline MSTD_CONSTEXPR_FN_14
844+
MbedCRC<POLY_32BIT_ANSI, 32>::MbedCRC() : MbedCRC(0xFFFFFFFF, 0xFFFFFFFF, true, true)
827845
{
828846
}
829847

830848
template<>
831-
inline MbedCRC<POLY_16BIT_IBM, 16>::MbedCRC() : MbedCRC(0, 0, true, true)
849+
inline MSTD_CONSTEXPR_FN_14
850+
MbedCRC<POLY_16BIT_IBM, 16>::MbedCRC() : MbedCRC(0, 0, true, true)
832851
{
833852
}
834853

835854
template<>
836-
inline MbedCRC<POLY_16BIT_CCITT, 16>::MbedCRC() : MbedCRC(0xFFFF, 0, false, false)
855+
inline MSTD_CONSTEXPR_FN_14
856+
MbedCRC<POLY_16BIT_CCITT, 16>::MbedCRC() : MbedCRC(0xFFFF, 0, false, false)
837857
{
838858
}
839859

840860
template<>
841-
inline MbedCRC<POLY_7BIT_SD, 7>::MbedCRC(): MbedCRC(0, 0, false, false)
861+
inline MSTD_CONSTEXPR_FN_14
862+
MbedCRC<POLY_7BIT_SD, 7>::MbedCRC(): MbedCRC(0, 0, false, false)
842863
{
843864
}
844865

845866
template<>
846-
inline MbedCRC<POLY_8BIT_CCITT, 8>::MbedCRC(): MbedCRC(0, 0, false, false)
867+
inline MSTD_CONSTEXPR_FN_14
868+
MbedCRC<POLY_8BIT_CCITT, 8>::MbedCRC(): MbedCRC(0, 0, false, false)
847869
{
848870
}
849871

0 commit comments

Comments
 (0)