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

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.