On Wed, Oct 21, 2026 at 9:06 AM Uros Bizjak <[email protected]> wrote:
>
> On Wed, Oct 21, 2026 at 7:46 AM Richard Biener
> <[email protected]> wrote:
> >
> > On Wed, Oct 21, 2026 at 6:42 AM Jeff Law <[email protected]>
> > wrote:
> > >
> > >
> > >
> > > On 10/17/26 3:23 AM, Uros Bizjak wrote:
> > > >> So you're going to hate me :-)
> > > >>
> > > >> RTL is largely unstructured. So you *could* have something like (mem
> > > >> (mem ... )) as the destination. The inner mem is a read that supplies
> > > >> the address to the outer mem and that inner mem might be volatile. vax
> > > >> might have this capability, though I don't know for sure.
> > > >>
> > > >> This would also seem to miss the case when the insn is not a single
> > > >> set,
> > > >> so maybe it's a PARALLEL with a bunch of reads/writes inside or other
> > > >> forms that do not satisfy single_set.
> > > >>
> > > >> So while I think your patch improves things, I think it's still leaving
> > > >> some pretty big holes.
> > > > How about we leave combine as is and leave the task of checking
> > > > patterns w.r.t volatile operands to the target code?
> > > But that means every target has to be sure to follow the rules. That
> > > seems like a major development and maintenance headache.
> > >
> > > >
> > > > Combine is just a fancy forward propagator, it creates more or less
> > > > sensible combined patterns (and performs some RTX simplifications) and
> > > > leaves it to the target-dependent code to check if the combined
> > > > pattern is acceptable.
> >
> > But it still has to produce semantically equivalent RTL which the
> > volatile case is not considering an RTL abstract machine?
>
> True, because when *two or more* volatile memory operands get combined
> into a single RTX pattern, we don't know the order of operands
> anymore. x86 example is e.g. *add<dwi>3_doubleword_concat that swaps
> and mixes two possibly volatile operands, with a counter example of
> double mode add that has *one* possibly volatile volatile operand. If
> middle-end ensures that only one volatile operand gets propagated into
> RTX (please note that combine won't propagate memory over other
> memory), then volatile accesses won't get reordered.
To illustrate the above, let's analyze how (unpatched) compiler
combines testcase from pr127851 (x and y are both volatiles):
5: r99:SI=[`x']
6: r100:SI=[`y']
7: flags:CCGC=cmp(r99:SI,r100:SI)
REG_DEAD r100:SI
REG_DEAD r99:SI
combine pass:
Trying 5 -> 7:
5: r99:SI=[`x']
7: flags:CCGC=cmp(r99:SI,r100:SI)
REG_DEAD r100:SI
REG_DEAD r99:SI
Can't combine i2 into i3 <- i2 passes insn 6, a volatile read, so
already rejected by combine.
Trying 6 -> 7:
6: r100:SI=[`y']
7: flags:CCGC=cmp(r99:SI,r100:SI)
REG_DEAD r100:SI
REG_DEAD r99:SI
Successfully matched this instruction:
(set (reg:CCGC 17 flags)
(compare:CCGC (reg/v:SI 99 [ a ])
(mem/v/c:SI (symbol_ref:DI ("y") [flags 0x40] <var_decl
0x7fd1a93d6ed8 y>) [1 y+0 S4 A32])))
allowing combination of insns 6 and 7 <- OK
Trying 5 -> 7:
5: r99:SI=[`x']
7: flags:CCGC=cmp(r99:SI,[`y'])
REG_DEAD r99:SI
Successfully matched this instruction:
(set (reg:CCGC 17 flags)
(compare:CCGC (mem/v/c:SI (symbol_ref:DI ("x") [flags 0x40]
<var_decl 0x7fd1a93d6e40 x>) [1 x+0 S4 A32])
(mem/v/c:SI (symbol_ref:DI ("y") [flags 0x40] <var_decl
0x7fd1a93d6ed8 y>) [1 y+0 S4 A32])))
allowing combination of insns 5 and 7 <- this should *NOT* be
allowed, there is already volatile read in the RTX, so combination
with another volatile read should be rejected.
IMO rejecting the last combination in the middle-end would resolve
this class of problems.
Uros.
