Re: [PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y

[David Laight <david.laight.linux@gmail.com>]

On Thu, 29 Jan 2026 11:12:49 +0100
Marco Elver <elver@google.com> wrote:

> On Thu, 29 Jan 2026 at 11:03, David Laight <david.laight.linux@gmail.com> wrote:
> >
> > On Thu, 29 Jan 2026 01:52:33 +0100
> > Marco Elver <elver@google.com> wrote:
> >  
> > > Rework arm64 LTO __READ_ONCE() to improve code generation as follows:
> > >
> > > 1. Replace _Generic-based __unqual_scalar_typeof() with more complete
> > >    __rwonce_typeof_unqual(). This strips qualifiers from all types, not
> > >    just integer types, which is required to be able to assign (must be
> > >    non-const) to __u.__val in the non-atomic case (required for #2).
> > >
> > > Once our minimum compiler versions are bumped, this just becomes
> > > TYPEOF_UNQUAL() (or typeof_unqual() should we decide to adopt C23
> > > naming).  Sadly the fallback version of __rwonce_typeof_unqual() cannot
> > > be used as a general TYPEOF_UNQUAL() fallback (see code comments).
> > >
> > > One subtle point here is that non-integer types of __val could be const
> > > or volatile within the union with the old __unqual_scalar_typeof(), if
> > > the passed variable is const or volatile. This would then result in a
> > > forced load from the stack if __u.__val is volatile; in the case of
> > > const, it does look odd if the underlying storage changes, but the
> > > compiler is told said member is "const" -- it smells like UB.
> > >
> > > 2. Eliminate the atomic flag and ternary conditional expression. Move
> > >    the fallback volatile load into the default case of the switch,
> > >    ensuring __u is unconditionally initialized across all paths.
> > >    The statement expression now unconditionally returns __u.__val.
> > >  
> > ...  
> > > Signed-off-by: Marco Elver <elver@google.com>
> > > ---
> > > v2:
> > > * Add __rwonce_typeof_unqual() as fallback for old compilers.
> > > ---
> > >  arch/arm64/include/asm/rwonce.h | 24 ++++++++++++++++++++----
> > >  1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> > > index fc0fb42b0b64..712de3238f9a 100644
> > > --- a/arch/arm64/include/asm/rwonce.h
> > > +++ b/arch/arm64/include/asm/rwonce.h
> > > @@ -19,6 +19,23 @@
> > >               "ldapr" #sfx "\t" #regs,                                \
> > >       ARM64_HAS_LDAPR)
> > >
> > > +#ifdef USE_TYPEOF_UNQUAL
> > > +#define __rwonce_typeof_unqual(x) TYPEOF_UNQUAL(x)
> > > +#else
> > > +/*
> > > + * Fallback for older compilers to infer an unqualified type.
> > > + *
> > > + * Uses the fact that auto is supposed to drop qualifiers. Unlike  
> >
> > Maybe:
> >         In all versions of clang 'auto' correctly drops qualifiers.
> > A reminder in here that this is clang only might also clarify things.  
> 
> Will add.
> 
> > > + * typeof_unqual(), the type must be complete (defines an unevaluated local
> > > + * variable); this must trivially hold because __READ_ONCE() returns a value.  
> >
> > Not sure that is needed.  
> 
> Trying to warn against someone copy-pasting this as a TYPEOF_UNQUAL
> fallback implementation. typeof() and typeof_unqual() do happily take
> incomplete struct declarations. E.g. this works:
> 
> struct foo;
> ...
> struct foo *f;
> typeof_unqual(*f) *x = f;
> 
> Whereas with the __rwonce_typeof_unqual() fallback this doesn't work.
> I can try to make it clearer.

It fails to compile - they'll find out soon enough :-)
gcc < 11 and the array/pointer decay are probably more relevant.
Could catch out the unwary.

	David

> 
> > > + *
> > > + * Another caveat is that because of array-to-pointer decay, an array is
> > > + * inferred as a pointer type; this is fine for __READ_ONCE usage, but is
> > > + * unsuitable as a general fallback implementation for TYPEOF_UNQUAL.  
> >
> > gcc < 11.0 stops it being used elsewhere.
> > Something shorter?
> >         The array-to-pointer decay doesn't matter here.  
> 
> Ack.
> 
> Thanks!