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