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

Marco Elver posted 3 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Posted by Marco Elver 1 week, 4 days ago
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.

This refactoring appears to help the compiler improve (or fix) code
generation.

With a defconfig + LTO + debug options builds, we observe different
codegen for the following functions:

	btrfs_reclaim_sweep (708 -> 1032 bytes)
	btrfs_sinfo_bg_reclaim_threshold_store (200 -> 204 bytes)
	check_mem_access (3652 -> 3692 bytes) [inlined bpf_map_is_rdonly]
	console_flush_all (1268 -> 1264 bytes)
	console_lock_spinning_disable_and_check (180 -> 176 bytes)
	igb_add_filter (640 -> 636 bytes)
	igb_config_tx_modes (2404 -> 2400 bytes)
	kvm_vcpu_on_spin (480 -> 476 bytes)
	map_freeze (376 -> 380 bytes)
	netlink_bind (1664 -> 1656 bytes)
	nmi_cpu_backtrace (404 -> 400 bytes)
	set_rps_cpu (516 -> 520 bytes)
	swap_cluster_readahead (944 -> 932 bytes)
	tcp_accecn_third_ack (328 -> 336 bytes)
	tcp_create_openreq_child (1764 -> 1772 bytes)
	tcp_data_queue (5784 -> 5892 bytes)
	tcp_ecn_rcv_synack (620 -> 628 bytes)
	xen_manage_runstate_time (944 -> 896 bytes)
	xen_steal_clock (340 -> 296 bytes)

Increase of some functions are due to more aggressive inlining due to
better codegen (in this build, e.g. bpf_map_is_rdonly is no longer
present due to being inlined completely).

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
+ * typeof_unqual(), the type must be complete (defines an unevaluated local
+ * variable); this must trivially hold because __READ_ONCE() returns a value.
+ *
+ * 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.
+ */
+#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 */
-- 
2.53.0.rc1.217.geba53bf80e-goog
Re: [PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Posted by David Laight 1 week, 3 days ago
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 */
Re: [PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Posted by Marco Elver 1 week, 3 days ago
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.

> > + *
> > + * 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.

Ack.

Thanks!
Re: [PATCH v2 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Posted by David Laight 1 week, 3 days ago
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!