Re: [PATCH] compiler: enable CONFIG_OPTIMIZE_INLINING forcibly

[Nick Desaulniers]

On Tue, Oct 1, 2019 at 10:01 AM Will Deacon <will@xxxxxxxxxx> wrote:
>
> On Tue, Oct 01, 2019 at 09:32:25AM -0700, Nick Desaulniers wrote:
> > On Tue, Oct 1, 2019 at 2:28 AM Will Deacon <will@xxxxxxxxxx> wrote:
> > > On Mon, Sep 30, 2019 at 02:50:10PM -0700, Nick Desaulniers wrote:
> > > > In this case, if there's a known codegen bug in a particular compiler
> > > > or certain versions of it, I recommend the use of either the C
> > > > preprocessor or __attribute__((no_inline)) to get the desired behavior
> > > > localized to the function in question, and for us to proceed with
> > > > Masahiro's cleanup.
> > >
> > > Hmm, I don't see how that would help. The problem occurs when things
> > > are moved out of line by the compiler (see below).
> >
> > It's being moved out of line because __attribute__((always_inline)) or
> > just inline provide no guarantees that outlining does not occur.  It
> > would help to make functions that need to be inlined macros, because
> > the C preprocessor doesn't have that issue.
>
> Hmm, let me try to put it another way: enabling CONFIG_OPTIMIZE_INLINING
> has been shown to cause miscompilation with many versions of GCC on arm64
> because the compiler moves things out of line that it otherwise doesn't
> appear to do.

Yes.  That code should use the C preprocessor to force inlining. Then
you can enable CONFIG_OPTIMIZE_INLINING for arm64.

> I don't see how __attribute__((no_inline)) helps with that,

Because that *wasn't* the point of what I said about
__attribute__((no_inline)). My point is to use the C preprocessor to
guarantee that no outlining occurs.

My point related to __attribute__((no_inline)) was the simple decision
matrix that should be used:

Do I need strong guarantees that my code must be inlined? Use the C
preprocessor.
Do I need strong guarantees that my code is *not* to be inlined? Use
__attribute__((no_inline)).

The *former* is what applies here, not the latter.

inline and __attribute__((always_inline)) don't provide strong
guarantees (despite their fun sounding names;
-funsafe-math-optimizations/-Ofast being the poster child for "that
sounds nice, I would like my math to be fun, safe, and fast, let's use
that, WCGW?").  The more we use them, the more we continuously be
bitten by bugs related to outlining. Like the one you cited and the
arm32 patch Masahiro wrote.

Would using the C preprocessor to force inlining fix the GCC/arm64
miscompilation bug you described above? I suspect it would.

> and replacing static functions with macros isn't great for type-checking
> and argument evaluation.

See typecheck (include/linux/typecheck.h).

> > You don't have to convince compiler folks about correctness. ;)
> > Correctness trumps all, especially performance.
>
> Then why is this conversation so difficult? :(

I apologize; I don't mean to be difficult.  I would just like to avoid
surprises when code written with the assumption that it will be
inlined is not.  It sounds like we found one issue in arm32 and one in
arm64 related to outlining.  If we fix those two cases, I think we're
close to proceeding with Masahiro's cleanup, which I view as a good
thing for the health of the Linux kernel codebase.
-- 
Thanks,
~Nick Desaulniers