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.
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} } } */
