Re: [PATCH] middle-end: Don't read a memory destination fully overwritten by a bit-field store

I think PATCH[2/5],  PATCH[3/5], PATCH[5/5] are fine to continue with.

On Fri, 7 Aug 2026 at 16:32, Richard Earnshaw <[email protected]>
wrote:

> On 13/07/2026 11:22, Richard Earnshaw 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.
> >
>
> Dominic,
>
> The AI policy has now been published: https://gcc.gnu.org/ai-policy.html
>
> Are your patches in compliance with that and still suitable for inclusion
> in GCC?
>
> R.
>
>