-----Original Message----- From: <[email protected]> on behalf of Josh Poimboeuf <[email protected]> Date: Wednesday, April 27, 2016 at 3:11 PM To: Arnd Bergmann <[email protected]> Cc: Martin Jambor <[email protected]>, "Martin K. Petersen" <[email protected]>, James Bottomley <[email protected]>, linux-scsi <[email protected]>, linux-kernel <[email protected]>, Denys Vlasenko <[email protected]>, Thomas Graf <[email protected]>, Peter Zijlstra <[email protected]>, David Rientjes <[email protected]>, Andrew Morton <[email protected]>, Ingo Molnar <[email protected]>, Himanshu Madhani <[email protected]>, Dept-Eng QLA2xxx Upstream <[email protected]>, Jan Hubicka <[email protected]> Subject: Re: [PATCH, RFT] byteswap: try to avoid __builtin_constant_p gcc bug
>On Thu, Apr 28, 2016 at 12:00:36AM +0200, Arnd Bergmann wrote: >> This is another attempt to avoid a regression in wwn_to_u64() >> after that started using get_unaligned_be64(), which in turn >> ran into a bug on gcc-4.9 through 6.1. >> >> As part of the problem is how __builtin_constant_p gets evaluated >> on an argument passed by reference into an inline function, this >> avoids the use of __builtin_constant_p() for all architectures >> that set CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not >> set ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably >> do not suffer from the problem in the qla2xxx driver, but they >> might still run into it elsewhere. >> >> I have not been able to reproduce the original problem, so I don't >> know if this patch solves it, but at least it leads to simpler >> code doing the same thing, so at least there should be no downsides. >> >> Please test. >> >> Signed-off-by: Arnd Bergmann <[email protected]> > >Nice patch. I can confirm it fixes the issue with gcc 5.3.1. > >Tested-by: Josh Poimboeuf <[email protected]> >Reviewed-by: Josh Poimboeuf <[email protected]> > >> diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h >> index 3f10e5317b46..de56fd54428d 100644 >> --- a/include/uapi/linux/swab.h >> +++ b/include/uapi/linux/swab.h >> @@ -45,9 +45,7 @@ >> >> static inline __attribute_const__ __u16 __fswab16(__u16 val) >> { >> -#ifdef __HAVE_BUILTIN_BSWAP16__ >> - return __builtin_bswap16(val); >> -#elif defined (__arch_swab16) >> +#if defined (__arch_swab16) >> return __arch_swab16(val); >> #else >> return ___constant_swab16(val); >> @@ -56,9 +54,7 @@ static inline __attribute_const__ __u16 __fswab16(__u16 >> val) >> >> static inline __attribute_const__ __u32 __fswab32(__u32 val) >> { >> -#ifdef __HAVE_BUILTIN_BSWAP32__ >> - return __builtin_bswap32(val); >> -#elif defined(__arch_swab32) >> +#if defined(__arch_swab32) >> return __arch_swab32(val); >> #else >> return ___constant_swab32(val); >> @@ -67,9 +63,7 @@ static inline __attribute_const__ __u32 __fswab32(__u32 >> val) >> >> static inline __attribute_const__ __u64 __fswab64(__u64 val) >> { >> -#ifdef __HAVE_BUILTIN_BSWAP64__ >> - return __builtin_bswap64(val); >> -#elif defined (__arch_swab64) >> +#if defined (__arch_swab64) >> return __arch_swab64(val); >> #elif defined(__SWAB_64_THRU_32__) >> __u32 h = val >> 32; >> @@ -102,28 +96,40 @@ static inline __attribute_const__ __u32 >> __fswahb32(__u32 val) >> * __swab16 - return a byteswapped 16-bit value >> * @x: value to byteswap >> */ >> +#ifdef __HAVE_BUILTIN_BSWAP16__ >> +#define __swab16(x) __builtin_bswap16((__u16)(x)) >> +#else >> #define __swab16(x) \ >> (__builtin_constant_p((__u16)(x)) ? \ >> ___constant_swab16(x) : \ >> __fswab16(x)) >> +#endif >> >> /** >> * __swab32 - return a byteswapped 32-bit value >> * @x: value to byteswap >> */ >> +#ifdef __HAVE_BUILTIN_BSWAP32__ >> +#define __swab32(x) __builtin_bswap32((__u32)(x)) >> +#else >> #define __swab32(x) \ >> (__builtin_constant_p((__u32)(x)) ? \ >> ___constant_swab32(x) : \ >> __fswab32(x)) >> +#endif >> >> /** >> * __swab64 - return a byteswapped 64-bit value >> * @x: value to byteswap >> */ >> +#ifdef __HAVE_BUILTIN_BSWAP64__ >> +#define __swab64(x) __builtin_bswap64((__u64)(x)) >> +#else >> #define __swab64(x) \ >> (__builtin_constant_p((__u64)(x)) ? \ >> ___constant_swab64(x) : \ >> __fswab64(x)) >> +#endif >> >> /** >> * __swahw32 - return a word-swapped 32-bit value >> The patch works. Thanks. > >-- >Josh >-- >To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >the body of a message to [email protected] >More majordomo info at http://vger.kernel.org/majordomo-info.html
