From 83021ec08730f43336464ab8762b45d329362aa9 Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Sun, 3 Nov 2019 15:43:36 -0800 Subject: [PATCH] Clean up U8TO*_LE macro implementations The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize byte-swapping by doing unaligned loads, but accessing data through unaligned pointers is undefined behavior in C. Moreover, compilers are more than capable of recognizing these open-coded byte-swap patterns and emitting a bswap instruction, or an unaligned load instruction, or a combined load, etc. There's no need for multiple paths to attain the desired result. The same commit went into perl in commit e8864dba8095 but with some embarrassing bugs. See https://rt.perl.org/Ticket/Display.html?id=133495 See https://github.com/Perl/perl5/issues/17244 --- mph_hv_macro.h | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/mph_hv_macro.h b/mph_hv_macro.h index 05b0b4f..c219502 100644 --- a/mph_hv_macro.h +++ b/mph_hv_macro.h @@ -6,7 +6,7 @@ #endif /*----------------------------------------------------------------------------- - * Endianess, misalignment capabilities and util macros + * Endianess and util macros * * The following 3 macros are defined in this section. The other macros defined * are only needed to help derive these 3. @@ -20,29 +20,22 @@ * ROTR64(x,r) Rotate x right by r bits */ -#ifndef U32_ALIGNMENT_REQUIRED +#ifndef U8TO16_LE #if (BYTEORDER == 0x1234 || BYTEORDER == 0x12345678) - #define U8TO16_LE(ptr) (*((const U16*)(ptr))) - #define U8TO32_LE(ptr) (*((const U32*)(ptr))) - #define U8TO64_LE(ptr) (*((const U64*)(ptr))) + #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8|(U32)(ptr)[2]<<16|(U32)(ptr)[3]<<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]<<8|(U64)(ptr)[2]<<16|(U64)(ptr)[3]<<24|\ + (U64)(ptr)[4]<<32|(U64)(ptr)[5]<<40|\ + (U64)(ptr)[6]<<48|(U64)(ptr)[7]<<56) #elif (BYTEORDER == 0x4321 || BYTEORDER == 0x87654321) - #if defined(__GNUC__) && (__GNUC__>4 || (__GNUC__==4 && __GNUC_MINOR__>=3)) - #define U8TO16_LE(ptr) (__builtin_bswap16(*((U16*)(ptr)))) - #define U8TO32_LE(ptr) (__builtin_bswap32(*((U32*)(ptr)))) - #define U8TO64_LE(ptr) (__builtin_bswap64(*((U64*)(ptr)))) - #endif + #define U8TO16_LE(ptr) ((U32)(ptr)[1]|(U32)(ptr)[0]<<8) + #define U8TO32_LE(ptr) ((U32)(ptr)[3]|(U32)(ptr)[2]<<8|(U32)(ptr)[1]<<16|(U32)(ptr)[0]<<24) + #define U8TO64_LE(ptr) ((U64)(ptr)[7]|(U64)(ptr)[6]<<8|(U64)(ptr)[5]<<16|(U64)(ptr)[4]<<24|\ + (U64)(ptr)[3]<<32|(U64)(ptr)[2]<<40|\ + (U64)(ptr)[1]<<48|(U64)(ptr)[0]<<56) #endif #endif -#ifndef U8TO16_LE - /* Without a known fast bswap32 we're just as well off doing this */ - #define U8TO16_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8) - #define U8TO32_LE(ptr) ((U32)(ptr)[0]|(U32)(ptr)[1]<<8|(U32)(ptr)[2]<<16|(U32)(ptr)[3]<<24) - #define U8TO64_LE(ptr) ((U64)(ptr)[0]|(U64)(ptr)[1]<<8|(U64)(ptr)[2]<<16|(U64)(ptr)[3]<<24|\ - (U64)(ptr)[4]<<32|(U64)(ptr)[5]<<40|\ - (U64)(ptr)[6]<<48|(U64)(ptr)[7]<<56) -#endif - #ifdef CAN64BITHASH #ifndef U64TYPE /* This probably isn't going to work, but failing with a compiler error due to