Re: [PATCH v4 1/3] compiler-gcc.h: add gnu_inline to all inline declarations

[off-list ref],Greg KH [off-list ref],jarkko.sakkinen@linux.intel.com,jgross@suse.com,Josh Poimboeuf [off-list ref],Matthias Kaehlcke [off-list ref],thomas.lendacky@amd.com,Thiebaud Weksteen [off-list ref],mjg59@google.com,joe@perches.com
From: hpa@zytor.com
Message-ID: [off-list ref]

On June 12, 2018 11:33:14 AM PDT, Nick Desaulniers [off-list ref] wrote:

On Fri, Jun 8, 2018 at 3:04 AM Sedat Dilek [off-list ref]
wrote:

quoted

On Fri, Jun 8, 2018 at 9:59 AM, Arnd Bergmann [off-list ref] wrote:

quoted

On Thu, Jun 7, 2018 at 10:49 PM, Nick Desaulniers
[off-list ref] wrote:

quoted

Functions marked extern inline do not emit an externally visible
function when the gnu89 C standard is used. Some KBUILD Makefiles
overwrite KBUILD_CFLAGS. This is an issue for GCC 5.1+ users as

without

quoted

quoted

quoted

an explicit C standard specified, the default is gnu11. Since c99,

the

quoted

quoted

quoted

semantics of extern inline have changed such that an externally

visible

quoted

quoted

quoted

function is always emitted. This can lead to multiple definition

errors

quoted

quoted

quoted

of extern inline functions at link time of compilation units whose

build

quoted

quoted

quoted

files have removed an explicit C standard compiler flag for users

of GCC

quoted

quoted

quoted

5.1+ or Clang.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Suggested-by: Joe Perches <joe@perches.com>

I suspect this will break Geert's gcc-4.1.2, which I think doesn't

have that

quoted

quoted

attribute yet (4.1.3 or higher have it according to the

documentation.

quoted

quoted

It wouldn't be hard to work around that if we want to keep that

version

quoted

quoted

working, or we could decide that it's time to officially stop

supporting

quoted

quoted

that version, but we should probably decide on one or the other.

Heh, so earlier we decided against compiler flags (-std=gnu89 or
-fgnu89-inline) in preference to function attributes.  The function
attribute is preferable as some of the Makefiles [accidentally?]
overwrite KBUILD_CFLAGS, which is problematic for gcc 5.1 users as the
implicit c standard used was changed to gnu11 from gnu89.  What's nice
is that to support gcc 4.1 users, we simply don't need to add any
attribute, as their implicit c standard is gnu89 which has the
semantics for extern inline that we want.  I have a simple change to
this patch that can support users of various gcc versions, see below:

quoted

Good point.
What is the minimum requirement of GCC version currently?
AFAICS x86/asm-goto support requires GCC >= 4.5?

Yes, but that's only for x86, IIUC.  It seems the kernel may have
different minimum required versions of GCC based on arch then?  That
may be ok, but I'm not sure that's easy to keep track of without
having it explicitly stated somewhere like the docs perhaps?

quoted

Just FYI...
...saw the last days in upstream commits that kbuild/kconfig for
4.18-rc1 offers possibilities to check for cc-version dependencies.

Those will be helpful.  If we want to pursue compiler flags, which get
set some Makefiles, then yes.  But I think a simpler change to my
patch would be as below.

It seems gcc did not get __has_attribute [0] until 5.1, but will
define __GNUC_GNU_INLINE__ if supported. [1]  Unfortunately, Clang
does not define __GNUC_GNU_INLINE__ [2].  So a proper feature test
might look like:

#ifndef __has_attribute
#define __has_attribute(x) 0
#endif

#if defined(__GNUC_GNU_INLINE__) || __has_attribute(gnu_inline)
#define __gnu_inline __attribute__(gnu_inline)
#endif

#define inline inline __attribute__((always_inline, unused)) notrace
__gnu_inline

Thoughts on this approach? I can send a v5 tomorrow if there's no
major issues.  Feedback appreciated, as always.

[0] https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute
[1]
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
[2] https://bugs.llvm.org/show_bug.cgi?id=37784

Please fix clang. It isn't all that hard to fix.

However, __GCC_GNU_INLINE__ means you are in GNU mode by default, on gcc's new enough to have multiple misses.

The right thing to look for is __GCC_STDC_INLINE__ in which case you need the attribute.

By the way, you should check clang against gcc's predefined macros by doing:

gcc [options] -x c -Wp,-dM -E /dev/null | sort

Options can change the predefined macros substantially, especially the, -std=, arch and -O options. -x c can be replaced with e.g. -x c++, objective-c, assembler-with-cpp etc.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.