From: David Laight <david.laight.linux@gmail.com>
To: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Will Deacon <will@kernel.org>, Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Boqun Feng <boqun.feng@gmail.com>,
Waiman Long <longman@redhat.com>,
Bart Van Assche <bvanassche@acm.org>,
llvm@lists.linux.dev, Catalin Marinas <catalin.marinas@arm.com>,
Arnd Bergmann <arnd@arndb.de>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Date: Thu, 29 Jan 2026 10:03:32 +0000 [thread overview]
Message-ID: <20260129100332.500248d3@pumpkin> (raw)
In-Reply-To: <20260129005645.747680-3-elver@google.com>
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.
> + * 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.
> + *
> + * 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 arrary-to-pointer decay doesn't matter here.
David
> + */
> +#define __rwonce_typeof_unqual(x) typeof(({ auto ____t = (x); ____t; }))
> +#endif
> +
> /*
> * When building with LTO, there is an increased risk of the compiler
> * converting an address dependency headed by a READ_ONCE() invocation
> @@ -32,8 +49,7 @@
> #define __READ_ONCE(x) \
> ({ \
> typeof(&(x)) __x = &(x); \
> - int atomic = 1; \
> - union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u; \
> + union { __rwonce_typeof_unqual(*__x) __val; char __c[1]; } __u; \
> switch (sizeof(x)) { \
> case 1: \
> asm volatile(__LOAD_RCPC(b, %w0, %1) \
> @@ -56,9 +72,9 @@
> : "Q" (*__x) : "memory"); \
> break; \
> default: \
> - atomic = 0; \
> + __u.__val = *(volatile typeof(*__x) *)__x; \
> } \
> - atomic ? (typeof(*__x))__u.__val : (*(volatile typeof(*__x) *)__x);\
> + __u.__val; \
> })
>
> #endif /* !BUILD_VDSO */
next prev parent reply other threads:[~2026-01-29 10:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-29 0:52 [PATCH v2 0/3] arm64: Fixes for __READ_ONCE() with CONFIG_LTO=y Marco Elver
2026-01-29 0:52 ` [PATCH v2 1/3] arm64: Fix non-atomic " Marco Elver
2026-01-29 1:21 ` Boqun Feng
2026-01-29 1:32 ` Marco Elver
2026-01-29 0:52 ` [PATCH v2 2/3] arm64: Optimize " Marco Elver
2026-01-29 10:03 ` David Laight [this message]
2026-01-29 10:12 ` Marco Elver
2026-01-29 10:39 ` David Laight
2026-01-29 0:52 ` [PATCH v2 3/3] arm64, compiler-context-analysis: Permit alias analysis through " Marco Elver
2026-01-29 2:41 ` Boqun Feng
2026-01-29 9:52 ` David Laight
2026-01-30 12:04 ` Marco Elver
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=20260129100332.500248d3@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=arnd@arndb.de \
--cc=boqun.feng@gmail.com \
--cc=bvanassche@acm.org \
--cc=catalin.marinas@arm.com \
--cc=elver@google.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=longman@redhat.com \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.