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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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