On Sun, 12 Jul 2026, Dominic P wrote:
> store_fixed_bit_field_1 implements a bit-field store as a read-modify-
> write: it reads the destination storage unit into a register, masks out
> the field's bits and ors in the new value, then writes it back. When the
> field occupies the whole unit there are no surrounding bits to preserve,
> yet the read is still emitted. For a non-volatile destination the read is
> dead and later removed, but a volatile read cannot be removed and survives
> as a spurious extra memory access.
>
> On a strict-alignment target a misaligned volatile store is decomposed
> into per-unit stores, so e.g.
>
> struct __attribute__((packed)) {
> unsigned char pad; volatile unsigned v;
> } *p;
> p->v = x;
>
> emits a dead volatile load before every byte/half-word store of the value.
> For a memory-mapped I/O register with read side effects (read-to-clear,
> FIFO pop, W1C) this is a wrong-code bug.
>
> When OP0 is in memory and the store fills the whole unit, store VALUE
> directly with no read. This is deliberately restricted to memory: for a
> register destination the read-modify-write is how a lowpart insertion is
> expressed, which a target may match with a dedicated pattern (e.g. x86
> bswaphisi2_lowpart), and the redundant read is eliminated later anyway.
>
> Genuine volatile bit-fields are unaffected. Under
> -fstrict-volatile-bitfields (the Arm default) a bit-field that fills its
> container is written through the container-width access path in
> store_bit_field_1 and never reaches this code, so it already stores once
> with no read. This was verified to give byte-identical output for byte,
> half-word and word volatile bit-fields, both aligned and in a packed
> struct, with and without -fstrict-volatile-bitfields. The only stores
> this shortcut changes are the decomposed per-unit writes of a misaligned
> volatile scalar -- the motivating case -- which are not bit-fields and
> carry no container-access contract.
>
> The fix is size-neutral -- it removes only the dead volatile read a rare
> whole-unit volatile bit-field store would emit. On five freestanding /
> bare-metal code bases at -O2, .text over the translation units that build
> with both an unpatched and a patched compiler:
>
> Linux kernel (arch/arm) 408 TUs 2,696,101 B -28 (-0.001%)
> SQLite amalgamation (arm1176) 1 TUs 853,260 B -48 (-0.006%)
> Pico SDK (Cortex-M0+) 74 TUs 62,682 B +0 (+0.000%)
> Circle (arm1176, C++) 77 TUs 140,537 B -12 (-0.009%)
> PiTubeDirect (arm1176) 88 TUs 966,021 B -20 (-0.002%)
>
> Bootstrapped and regression-tested on x86_64-pc-linux-gnu with no new
> failures (this is a generic expmed.cc change); also cross-tested on
> arm-none-eabi, where gcc.target/arm (arm.exp) is unchanged against master
> and the gcc.c-torture execute suite gives identical results under QEMU
> between an unpatched and a patched compiler. The new test
> gcc.target/arm/pr71048.c passes.
>
> gcc/ChangeLog:
>
> PR middle-end/71048
> * expmed.cc (store_fixed_bit_field_1): When OP0 is a memory
> reference that the field fills entirely, store VALUE directly
> instead of doing a read-modify-write, so that no read of OP0 is
> emitted.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/pr71048.c: New test.
> * gcc.dg/torture/pr71048-1.c: New test.
>
> Signed-off-by: Dominic P <[email protected]>
> ---
>
> This patch was prepared with the assistance of an AI coding tool. Every line
> of
> code, every test and every measurement was written, reviewed and verified by
> the
> author, who takes responsibility for the patch; the Signed-off-by above
> certifies
> the Developer Certificate of Origin.
>
> gcc/expmed.cc | 17 +++++++++++++
> gcc/testsuite/gcc.dg/torture/pr71048-1.c | 36 ++++++++++++++++++++++++
> gcc/testsuite/gcc.target/arm/pr71048.c | 19 +++++++++++++
> 3 files changed, 72 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr71048-1.c
> create mode 100644 gcc/testsuite/gcc.target/arm/pr71048.c
>
> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
> index da1b5b632..87b03278f 100644
> --- a/gcc/expmed.cc
> +++ b/gcc/expmed.cc
> @@ -1344,6 +1344,23 @@ store_fixed_bit_field_1 (rtx op0, scalar_int_mode mode,
> if (reverse)
> value = flip_storage_order (mode, value);
>
> + /* If OP0 is a memory reference and the field fills the whole of OP0, there
> + are no surrounding bits to preserve, so store VALUE directly instead of
> + doing a read-modify-write. This does not read OP0 at all: for a
> volatile
> + MEM (such as a memory-mapped I/O register) that read cannot be removed
> by
> + later passes, so it would be a spurious extra access with observable
> side
> + effects (PR71048). This is restricted to memory: for a register
> + destination the read-modify-write is how a lowpart insertion is
> expressed,
> + which a target may match with a dedicated pattern, and the redundant
> read
> + is eliminated later anyway. */
> + if (MEM_P (op0) && bitnum == 0 && bitsize == GET_MODE_BITSIZE (mode))
> + {
> + /* Copy OP0 so the store does not share the caller's MEM rtx. */
> + op0 = copy_rtx (op0);
> + emit_move_insn (op0, value);
I'm not sure if we should have catched this upthread, but also I think
you need to
value = force_reg (mode, value);
before storing the value, both becuase the move might otherwise
be not recognizable and because it might be a MEM overlapping with OP0,
creating a correctness issue?
Richard.
> + return;
> + }
> +
> /* Now clear the chosen bits in OP0,
> except that if VALUE is -1 we need not bother. */
> /* We keep the intermediates in registers to allow CSE to combine
> diff --git a/gcc/testsuite/gcc.dg/torture/pr71048-1.c
> b/gcc/testsuite/gcc.dg/torture/pr71048-1.c
> new file mode 100644
> index 000000000..f2dc83a1b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr71048-1.c
> @@ -0,0 +1,36 @@
> +/* PR middle-end/71048: storing a value that fills a whole memory unit must
> go
> + in without a read-modify-write. This checks, target-independently, that
> the
> + store-directly path produces the correct value for volatile destinations
> of
> + several widths, including the misaligned (packed) case that a
> strict-alignment
> + target decomposes into per-unit stores. */
> +/* { dg-do run } */
> +
> +extern void abort (void);
> +
> +struct __attribute__((packed)) Sp { unsigned char pad; volatile unsigned w;
> };
> +struct Sa { volatile unsigned w; };
> +struct Sh { volatile unsigned short h; };
> +struct Sb { volatile unsigned char b; };
> +
> +static struct Sp sp;
> +static struct Sa sa;
> +static struct Sh sh;
> +static struct Sb sb;
> +
> +static const unsigned vals[] =
> + { 0u, 1u, 0xffu, 0x1234u, 0xdeadbeefu, 0x80000000u, 0xffffffffu,
> 0xa5a5a5a5u };
> +
> +int
> +main (void)
> +{
> + for (unsigned i = 0; i < sizeof (vals) / sizeof (vals[0]); i++)
> + {
> + unsigned v = vals[i];
> +
> + sp.w = v; if (sp.w != v) abort ();
> + sa.w = v; if (sa.w != v) abort ();
> + sh.h = (unsigned short) v; if (sh.h != (unsigned short) v) abort ();
> + sb.b = (unsigned char) v; if (sb.b != (unsigned char) v) abort ();
> + }
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/pr71048.c
> b/gcc/testsuite/gcc.target/arm/pr71048.c
> new file mode 100644
> index 000000000..9cd243635
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr71048.c
> @@ -0,0 +1,19 @@
> +/* PR middle-end/71048: a write-only store to a misaligned volatile object
> must
> + not read the destination back. The store is decomposed into byte/halfword
> + accesses on a strict-alignment target; each unit is fully written, so no
> + read-modify-write (and in particular no spurious volatile read) is
> needed. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +struct __attribute__((packed)) S { unsigned char pad; volatile unsigned val;
> };
> +
> +void
> +wr (struct S *m, unsigned x)
> +{
> + m->val = x;
> +}
> +
> +/* No load through the destination pointer (r0) should be emitted -- that is
> the
> + spurious volatile read the fix removes. Matching only loads based on r0
> (not
> + any incidental ldr, such as a constant-pool load) keeps this robust to
> tuning. */
> +/* { dg-final { scan-assembler-not {ldr[bh]?\t[^\n]*\[r0} } } */
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
