[PATCH 3/3] middle-end: don't read a memory destination fully overwritten by a bit-field store [PR71048]

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.

A field that does not fill its unit is untouched and keeps its
read-modify-write, as does a store to a register; the guard fires only
where the old code would have computed (op0 & 0) | value.

The test pins -mno-unaligned-access so that it exercises the decomposed
store everywhere rather than only on strict-alignment configurations:
with unaligned access allowed the field is stored as a single unaligned
str and there is nothing to read back.  Counting occurrences in the
emitted assembly, with and without the patch:

  -mcpu=arm1176jzf-s        4 loads -> 0     strb 4 -> 4
  -march=armv7-a -marm      4 loads -> 0     strb 4 -> 4
  -mcpu=cortex-m4 -mthumb   4 loads -> 0     strb 4 -> 4
  -march=armv5te -marm      5 loads -> 0     strb 4 -> 4

The series was bootstrapped on x86_64-pc-linux-gnu at trunk 7f549ea2b47
with the stage2/stage3 comparison successful, and a full make check shows
no regressions: 227924 gcc and 278399 g++ expected passes, and every one
of the 112 unexpected results also occurs with the series reverted.

Assisted-by: Claude Opus 5 (Anthropic)

gcc/ChangeLog:

	PR target/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:

	PR target/71048
	* gcc.target/arm/pr71048.c: New test.

Signed-off-by: Dominic P <[email protected]>
---
 gcc/expmed.cc                          | 10 ++++++++++
 gcc/testsuite/gcc.target/arm/pr71048.c | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/arm/pr71048.c

diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index 3cfe58d4d9b..9c70e48e311 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -1351,6 +1351,16 @@ store_fixed_bit_field_1 (rtx op0, scalar_int_mode mode,
   if (reverse)
     value = flip_storage_order (mode, value);
 
+  /* A field filling the whole of a MEM has no surrounding bits to preserve,
+     so store it directly: the read of a volatile OP0 cannot be removed later
+     and would be a spurious access with side effects (PR71048).  */
+  if (MEM_P (op0) && bitnum == 0 && bitsize == GET_MODE_BITSIZE (mode))
+    {
+      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.target/arm/pr71048.c b/gcc/testsuite/gcc.target/arm/pr71048.c
new file mode 100644
index 00000000000..70bf60f6180
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr71048.c
@@ -0,0 +1,19 @@
+/* PR target/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 -mno-unaligned-access" } */
+
+struct __attribute__((packed)) S { unsigned char pad; volatile unsigned val; };
+
+void
+wr (struct S *m, unsigned x)
+{
+  m->val = x;
+}
+
+/* No load of the volatile destination should be emitted (matches ldr, ldrb,
+   ldrh).  */
+/* { dg-final { scan-assembler-not {\mldr[bhd]?\M} } } */
+/* { dg-final { scan-assembler-times {\mstrb\M} 4 } } */
-- 
2.55.0