From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Will Deacon <will@kernel.org>,
Masahiro Yamada <yamada.masahiro@socionext.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
Andrew Morton <akpm@linux-foundation.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
linux-arch <linux-arch@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Stefan Wahren <wahrenst@gmx.net>, Kees Cook <keescook@google.com>,
Arnd Bergmann <arnd@arndb.de>,
clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH] compiler: enable CONFIG_OPTIMIZE_INLINING forcibly
Date: Tue, 1 Oct 2019 22:58:27 +0100 [thread overview]
Message-ID: <20191001215827.GP25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <CAKwvOdmkmdM14BNurK3WwyG3Qc5wOFeajMtQ1D+na9mLfkim+w@mail.gmail.com>
On Tue, Oct 01, 2019 at 02:32:54PM -0700, Nick Desaulniers wrote:
> On Tue, Oct 1, 2019 at 2:26 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Oct 01, 2019 at 09:59:38PM +0100, Russell King - ARM Linux admin wrote:
> > > On Tue, Oct 01, 2019 at 01:21:44PM -0700, Nick Desaulniers wrote:
> > > > On Tue, Oct 1, 2019 at 11:14 AM Russell King - ARM Linux admin
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > The whole "let's make inline not really mean inline" is nothing more
> > > > > than a band-aid to the overuse (and abuse) of "inline".
> > > >
> > > > Let's triple check the ISO C11 draft spec just to be sure:
> > > > § 6.7.4.6: A function declared with an inline function specifier is an
> > > > inline function. Making a
> > > > function an inline function suggests that calls to the function be as
> > > > fast as possible.
> > > > The extent to which such suggestions are effective is
> > > > implementation-defined. 139)
> > > > 139) For example, an implementation might never perform inline
> > > > substitution, or might only perform inline
> > > > substitutions to calls in the scope of an inline declaration.
> > > > § J.3.8 [Undefined Behavior] Hints: The extent to which suggestions
> > > > made by using the inline function specifier are effective (6.7.4).
> > > >
> > > > My translation:
> > > > "Please don't assume inline means anything."
> > > >
> > > > For the unspecified GNU C extension __attribute__((always_inline)), it
> > > > seems to me like it's meant more for performing inlining (an
> > > > optimization) at -O0. Whether the compiler warns or not seems like a
> > > > nice side effect, but provides no strong guarantee otherwise.
> > > >
> > > > I'm sorry that so much code may have been written with that
> > > > assumption, and I'm sorry to be the bearer of bad news, but this isn't
> > > > a recent change. If code was written under false assumptions, it
> > > > should be rewritten. Sorry.
> > >
> > > You may quote C11, but that is not relevent. The kernel is coded to
> > > gnu89 standard - see the -std=gnu89 flag.
> >
> > There's more to this and why C11 is entirely irrelevant. The "inline"
> > you see in our headers is not the compiler keyword that you find in
> > various C standards, it is a macro that gets expanded to either:
> >
> > #define inline inline __attribute__((__always_inline__)) __gnu_inline \
> > __maybe_unused notrace
> >
> > or
> >
> > #define inline inline __gnu_inline \
> > __maybe_unused notrace
> >
> > __gnu_inline is defined as:
> >
> > #define __gnu_inline __attribute__((__gnu_inline__))
> >
> > So this attaches the gnu_inline attribute to the function:
> >
> > `gnu_inline'
> > This attribute should be used with a function that is also declared
> > with the `inline' keyword. It directs GCC to treat the function
> > as if it were defined in gnu90 mode even when compiling in C99 or
> > gnu99 mode.
> > ...
> > Since ISO C99 specifies a different semantics for `inline', this
> > function attribute is provided as a transition measure and as a
> > useful feature in its own right. This attribute is available in
> > GCC 4.1.3 and later. It is available if either of the
> > preprocessor macros `__GNUC_GNU_INLINE__' or
> > `__GNUC_STDC_INLINE__' are defined. *Note An Inline Function is
> > As Fast As a Macro: Inline.
> >
> > which is quite clear that C99 semantics do not apply to _this_ inline.
> > The manual goes on to explain:
> >
> > GCC implements three different semantics of declaring a function
> > inline. One is available with `-std=gnu89' or `-fgnu89-inline' or when
> > `gnu_inline' attribute is present on all inline declarations, another
> > when `-std=c99', `-std=c11', `-std=gnu99' or `-std=gnu11' (without
> > `-fgnu89-inline'), and the third is used when compiling C++.
>
> (I wrote the kernel patch for gnu_inline; it only comes into play when
> `inline` appears on a function *also defined as `extern`*).
From what I can tell reading the GCC manual, the patch adding
gnu_inline should have no effect. Maybe it was written before
-std=gnu89 was in use by the kernel makefiles?
> > I'd suggest gnu90 mode is pretty similar to gnu89 mode, and as we build
> > the kernel in gnu89 mode, that is the inlining definition that is
> > appropriate.
> >
> > When it comes to __always_inline, the GCC manual is the definitive
> > reference, since we use the GCC attribute for that:
> >
> > #define __always_inline inline __attribute__((__always_inline__))
> >
> > and I've already quoted what the GCC manual says for always_inline.
> >
> > Arguing about what the C11 spec says about inlining when we aren't
> > using C11 dialect in the kernel, but are using GCC features, does
> > not move the discussion on.
> >
> > Thanks anyway, maybe it will become relevent in the future if we
> > decide to move to C11.
>
> It's not like the semantics of inline are better specified by an older
> standard, or changed (The only real semantic change involving `inline`
> between ISO C90 and ISO C99 has to do with whether `extern inline`
> emits the function with external linkage as you noted). But that's
> irrelevant to the discussion.). I quoted C11 because ctrl+f doesn't
> work for the C90 ISO spec pdf. C90 spec doesn't even have a section
> on Function Specifiers. From what I can tell, `inline` wasn't
> specified until ISO C99.
>
> GNU modes are often modifiers off of ISO C bases; gnu89 corresponds to
> ISO C90. They may permit the use of features from newer ISO C specs
> and GNU C extensions without warning under -Wpedantic. There aren't a
> whole lot of semantic differences, at least that I'm aware of.
Right, so GCC had inlining support before ISO C added it (which I
distinctly remember, I've been involved in Linux since 1994.)
Unless ISO C based their definition in some way off GCC's
implementation, I still don't see how quoting ISO C documents
helps this discussion when it's the GCC feature that we're using.
And none of this is relevent anyway if we use the GCC
always_inline extension *which is obviously the right way to
resolve this instance*.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
next prev parent reply other threads:[~2019-10-01 21:58 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 3:43 [PATCH] compiler: enable CONFIG_OPTIMIZE_INLINING forcibly Masahiro Yamada
2019-08-30 20:54 ` Nick Desaulniers
2019-09-26 8:54 ` Geert Uytterhoeven
2019-09-26 9:02 ` Masahiro Yamada
2019-09-26 9:26 ` Geert Uytterhoeven
2019-09-26 9:46 ` Masahiro Yamada
2019-09-27 10:43 ` Nicolas Saenz Julienne
2019-09-27 10:59 ` Charles Keepax
2019-09-27 22:08 ` Nick Desaulniers
2019-09-27 22:38 ` Linus Torvalds
2019-09-30 11:26 ` Will Deacon
2019-09-30 12:05 ` Masahiro Yamada
2019-09-30 12:18 ` Will Deacon
2019-09-30 21:50 ` Nick Desaulniers
2019-09-30 22:08 ` Miguel Ojeda
2019-09-30 22:34 ` Nick Desaulniers
2019-10-01 9:28 ` Will Deacon
2019-10-01 16:32 ` Nick Desaulniers
2019-10-01 17:01 ` Will Deacon
2019-10-01 17:44 ` Nick Desaulniers
2019-10-01 17:55 ` Russell King - ARM Linux admin
2019-10-01 18:00 ` Nick Desaulniers
2019-10-01 18:14 ` Russell King - ARM Linux admin
2019-10-01 20:21 ` Nick Desaulniers
2019-10-01 20:53 ` Arnd Bergmann
2019-10-01 21:06 ` Miguel Ojeda
2019-10-01 21:14 ` Nick Desaulniers
2019-10-01 20:59 ` Russell King - ARM Linux admin
2019-10-01 21:26 ` Russell King - ARM Linux admin
2019-10-01 21:32 ` Nick Desaulniers
2019-10-01 21:58 ` Russell King - ARM Linux admin [this message]
2019-10-02 12:55 ` Geert Uytterhoeven
2019-10-02 18:51 ` Will Deacon
2019-10-02 20:39 ` Linus Torvalds
2019-10-03 2:10 ` Masahiro Yamada
2019-10-03 17:01 ` Linus Torvalds
2019-10-03 17:08 ` Linus Torvalds
2019-10-03 17:23 ` Masahiro Yamada
2019-10-03 17:29 ` Linus Torvalds
2019-10-03 20:21 ` Miguel Ojeda
2019-10-04 7:37 ` Geert Uytterhoeven
2019-10-03 16:36 ` Will Deacon
2019-10-12 10:15 ` Stefan Wahren
2019-10-12 11:12 ` Masahiro Yamada
2019-10-12 14:45 ` Russell King - ARM Linux admin
2019-10-01 9:39 ` Masahiro Yamada
2019-10-01 10:40 ` Will Deacon
2019-09-27 10:58 ` Nicolas Saenz Julienne
2019-09-30 6:04 ` Masahiro Yamada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191001215827.GP25745@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=clang-built-linux@googlegroups.com \
--cc=keescook@google.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=mingo@redhat.com \
--cc=ndesaulniers@google.com \
--cc=nsaenzjulienne@suse.de \
--cc=torvalds@linux-foundation.org \
--cc=wahrenst@gmx.net \
--cc=will@kernel.org \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).