Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1670678

Re: [PATCH] compiler, clang: Add always_inline attribute to inline

From Mark Rutland <mark.rutland@arm.com>
Newsgroups linux.kernel
Subject Re: [PATCH] compiler, clang: Add always_inline attribute to inline
Date 2017-06-20 13:10 +0200
Message-ID <tUpcK-7R1-29@gated-at.bofh.it> (permalink)
References (1 earlier) <tU7fQ-4Bo-13@gated-at.bofh.it> <tUbt9-7sP-35@gated-at.bofh.it> <tUcfv-7ZZ-3@gated-at.bofh.it> <tUcIy-89O-25@gated-at.bofh.it> <tUdbA-7o-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Jun 19, 2017 at 03:19:27PM -0700, Sodagudi Prasad wrote:
> On 2017-06-19 14:42, David Rientjes wrote:
> >Yes, the arch/arm64/include/asm/cmpxchg.h instance appears to need
> >__always_inline as several other functions need __always_inline in
> >arch/arm64/include/*.  It's worth making that change as you
> >suggested in
> >your original patch.
> >
> >The concern, however, is inlining all "inline" functions
> >forcefully.  The
> >only reason this is done for gcc is because of suboptimal inlining
> >decisions in gcc < 4.
> >
> >So the question is whether this is a single instance that can be fixed
> >where clang un-inlining causes problems or whether that instance
> >suggests
> >all possible inline usage for clang absolutely requires __always_inline
> >due to a suboptimal compiler implementation.  I would suggest the
> >former.
> 
> Hi David,
> 
>  I am not 100% sure about the best approach for this problem. We may
> have to
> replace inline with always_inline for all inline functions where
> BUILD_BUG() used.
> 
> So far inline as always_inline for ARM64, if we do not continue same
> settings,
> will there not be any performance differences?
> 
> Hi Will and Mark,
> 
> Please suggest the best solution to this problem. Currently
> __xchg_mb is only having issue
> based on compiler -inline-threshold configuration. But there are
> many other instances
> in arch/arm64/* where BUILD_BUG() used for inline functions and
> which may fail later.

As with my reply to David, my preference would be that we:

1) Align compiler-clang.h with the compiler-gcc.h inlining behaviour, so
   that things work by default.

2) Fix up the arm64 core code (and drivers for architected / common
   peripherals) to use __always_inline where we always require inlining.

3) Have arm64 select CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING, and have
   people test-build configurations with CONFIG_OPTIMIZE_INLINING, with
   both GCC and clang.

4) Fix up drivers, etc, as appropriate.

5) Once that's largely stable, and if there's a benefit, have arm64
   select CONFIG_OPTIMIZE_INLINING by default.

That should avoid undue breakage, while enabling this ASAP.

Thanks,
Mark.

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: [PATCH] compiler, clang: Add always_inline attribute to inline Mark Rutland <mark.rutland@arm.com> - 2017-06-20 13:10 +0200
  Re: [PATCH] compiler, clang: Add always_inline attribute to inline David Rientjes <rientjes@google.com> - 2017-06-21 01:20 +0200
    Re: [PATCH] compiler, clang: Add always_inline attribute to inline Mark Rutland <mark.rutland@arm.com> - 2017-06-22 11:50 +0200
      Re: [PATCH] compiler, clang: Add always_inline attribute to inline Sodagudi Prasad <psodagud@codeaurora.org> - 2017-06-23 08:50 +0200

csiph-web