Skip to content

Commit 094f229

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 07d43b7 commit 094f229

File tree

1 file changed

+102
-75
lines changed

1 file changed

+102
-75
lines changed

drivers/MbedCRC.h

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

32+
#ifdef UNITTEST
3233
#include <type_traits>
34+
#define MSTD_CONSTEXPR_IF_HAS_IS_CONSTANT_EVALUATED
35+
#else
36+
#include <mstd_type_traits>
37+
#endif
3338

3439
namespace mbed {
3540
/** \addtogroup drivers-public-api */
@@ -175,11 +180,13 @@ class MbedCRC {
175180
* polynomials with different initial/final/reflect values
176181
*
177182
*/
183+
constexpr
178184
MbedCRC(uint32_t initial_xor, uint32_t final_xor, bool reflect_data, bool reflect_remainder) :
179185
crc_impl(initial_xor, final_xor, reflect_data, reflect_remainder)
180186
{
181187
}
182188

189+
constexpr
183190
MbedCRC();
184191

185192
/** Compute CRC for the data input
@@ -279,6 +286,7 @@ class MbedCRC {
279286
public:
280287
typedef size_t crc_data_size_t;
281288

289+
constexpr
282290
MbedCRC(uint32_t initial_xor, uint32_t final_xor, bool reflect_data, bool reflect_remainder) :
283291
_initial_value(adjust_initial_value(initial_xor, reflect_data)),
284292
_final_xor(final_xor),
@@ -400,7 +408,7 @@ class MbedCRC {
400408
}
401409
} else {
402410
/* CRC has MSB in top bit of register */
403-
p_crc = _reflect_remainder ? reflect_register(p_crc) : shift_right(p_crc);
411+
p_crc = _reflect_remainder ? reflect(p_crc) : shift_right(p_crc);
404412
}
405413
} else { // TABLE
406414
/* CRC has MSB in bottom bit of register */
@@ -417,45 +425,91 @@ class MbedCRC {
417425
}
418426

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

461515
/** Get the current CRC polynomial, at top of register.
@@ -484,21 +538,8 @@ class MbedCRC {
484538
static const crc_table_t _crc_table[MBED_CRC_TABLE_SIZE];
485539
#endif
486540

487-
static uint32_t adjust_initial_value(uint32_t initial_xor, bool reflect_data)
541+
static constexpr uint32_t adjust_initial_value(uint32_t initial_xor, bool reflect_data)
488542
{
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 */
502543
if (mode == CrcMode::BITWISE) {
503544
/* For bitwise calculation, CRC register is reflected if data is, to match input.
504545
* (MSB at bottom of register). If not reflected, it is at the top of the register
@@ -545,34 +586,15 @@ class MbedCRC {
545586
return (uint32_t)((uint32_t)2U << (width - 1)) - 1U;
546587
}
547588

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-
558589
/** CRC values may need to be reflected.
559590
*
560591
* @param CRC value to be reflected (width bits at bottom of 32-bit word)
561592
* @return Reflected value (still at bottom of 32-bit word)
562593
*/
563-
static uint32_t reflect_crc(uint32_t data)
594+
static MSTD_CONSTEXPR_IF_HAS_IS_CONSTANT_EVALUATED
595+
uint32_t reflect_crc(uint32_t data)
564596
{
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)
574-
{
575-
return __RBIT(data);
597+
return reflect(data) >> (32 - width);
576598
}
577599

578600
/** Register values may need to be shifted left.
@@ -823,27 +845,32 @@ const uint32_t MbedCRC<POLY_32BIT_ANSI, 32, CrcMode::TABLE>::_crc_table[MBED_CRC
823845
/* Default values for different types of polynomials
824846
*/
825847
template<>
826-
inline MbedCRC<POLY_32BIT_ANSI, 32>::MbedCRC() : MbedCRC(0xFFFFFFFF, 0xFFFFFFFF, true, true)
848+
inline MSTD_CONSTEXPR_FN_14
849+
MbedCRC<POLY_32BIT_ANSI, 32>::MbedCRC() : MbedCRC(0xFFFFFFFF, 0xFFFFFFFF, true, true)
827850
{
828851
}
829852

830853
template<>
831-
inline MbedCRC<POLY_16BIT_IBM, 16>::MbedCRC() : MbedCRC(0, 0, true, true)
854+
inline MSTD_CONSTEXPR_FN_14
855+
MbedCRC<POLY_16BIT_IBM, 16>::MbedCRC() : MbedCRC(0, 0, true, true)
832856
{
833857
}
834858

835859
template<>
836-
inline MbedCRC<POLY_16BIT_CCITT, 16>::MbedCRC() : MbedCRC(0xFFFF, 0, false, false)
860+
inline MSTD_CONSTEXPR_FN_14
861+
MbedCRC<POLY_16BIT_CCITT, 16>::MbedCRC() : MbedCRC(0xFFFF, 0, false, false)
837862
{
838863
}
839864

840865
template<>
841-
inline MbedCRC<POLY_7BIT_SD, 7>::MbedCRC(): MbedCRC(0, 0, false, false)
866+
inline MSTD_CONSTEXPR_FN_14
867+
MbedCRC<POLY_7BIT_SD, 7>::MbedCRC(): MbedCRC(0, 0, false, false)
842868
{
843869
}
844870

845871
template<>
846-
inline MbedCRC<POLY_8BIT_CCITT, 8>::MbedCRC(): MbedCRC(0, 0, false, false)
872+
inline MSTD_CONSTEXPR_FN_14
873+
MbedCRC<POLY_8BIT_CCITT, 8>::MbedCRC(): MbedCRC(0, 0, false, false)
847874
{
848875
}
849876

0 commit comments

Comments
 (0)