On Mon, 13 Jul 2026 at 11:22, Richard Earnshaw <[email protected]>
wrote:
> On 12/07/2026 00:07, 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.
>
> Hi Dominic,
>
> This reply applies to all of your recent patches.
>
> The GCC Steering Committee is currently discussing how they wish to
> handle contributions generated with AI assistance. Until that is
> complete, I don't think we can process these patches.
>
> R.
>
> Hi Richard,
Thank you for flagging this — and understood.
Dominic
> >
> > 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);
> > + 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} } } */
>
>
>
