Messages in this thread | | | Subject | Re: [PATCH] scsi: fc: force inlining of wwn conversion functions | From | Chris Metcalf <> | Date | Thu, 28 Apr 2016 11:48:57 -0400 |
| |
On 4/28/2016 11:23 AM, Arnd Bergmann wrote: > On Thursday 28 April 2016 10:58:43 Chris Metcalf wrote: >> (Resending as text/plain) >> >> On 4/27/2016 5:34 PM, Arnd Bergmann wrote: >>> This won't help on TILE, which is the one architecture that sets >>> ARCH_SUPPORTS_OPTIMIZED_INLINING but does not set ARCH_USE_BUILTIN_BSWAP. >>> Chris Metcalf should be able to figure out whether we can just >>> set ARCH_USE_BUILTIN_BSWAP for tile as well. >> We certainly could enable ARCH_USE_BUILTIN_BSWAP. The only problem is >> that we never added explicit support for bswap16() in gcc, which is >> efficiently done on tilegx via the "revbytes" instruction and a 48-bit >> right-shift. So gcc instead does a generic thing with four >> instructions in three bundles, so really not as good as our asm/swab.h. >> >> I'm not sure how to weigh the implications of converting to >> builtin_bswap16 (and possibly upstreaming a better implementation to >> gcc), vs. disabling ARCH_SUPPORTS_OPTIMIZED_INLINING (which no one >> else but x86 uses anyway), vs. just ignoring the compiler bug and >> hoping it's not an issue in practice > How about figuring out whether you hit the gcc bug on tile as a > first step?
I don't have an affected build of gcc handy (just 4.8 and 4.4). I will pass this to our compiler folks and see what they know.
> Another idea would be to adapt this section in include/linux/compiler-gcc.h: > > #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \ > !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__ < 4) > #define inline inline __attribute__((always_inline)) notrace > #define __inline__ __inline__ __attribute__((always_inline)) notrace > #define __inline __inline __attribute__((always_inline)) notrace > #else > /* A lot of inline functions can cause havoc with function tracing */ > #define inline inline notrace > #define __inline__ __inline__ notrace > #define __inline __inline notrace > #endif > > to work around the issue. We already check for gcc before 4.0, and > we could also check for the affected releases (4.9, 5.x, 6.1) in the > same place, possibly conditional on ARCH_USE_BUILTIN_BSWAP with > a comment pointing to the gcc bug tracker.
This does seem like a more robust solution anyway, since more instances of the bad inline pattern might get introduced in the future in other places. I wouldn't make it conditional on ARCH_USE_BUILTIN_BSWAP for the same reason.
-- Chris Metcalf, Mellanox Technologies http://www.mellanox.com
| |