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

Jeff Law via Gcc-cvs <[email protected]>
Newsgroups gmane.comp.gcc.cvs
Message-ID <[email protected]>
https://gcc.gnu.org/g:db3b32439f9c8e98cea5625b9026cc42fd066917

commit r17-3552-gdb3b32439f9c8e98cea5625b9026cc42fd066917
Author: Dominic P <[email protected]>
Date:   Sat Aug 22 20:51:42 2026 -0600

    [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)
    
            PR target/71048
    gcc/ChangeLog:
    
            * 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.

Diff:
---
 gcc/expmed.cc                          | 10 ++++++++++
 gcc/testsuite/gcc.target/arm/pr71048.c | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index 3cfe58d4d9b2..9c70e48e3117 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 000000000000..70bf60f61800
--- /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 } } */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.