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. Following this approach, the target should
*also* check if the combined pattern is acceptable when volatile
operands are combined.
Agreed, to a point. I certainly don't mind seeing the backends do
additional checking, but ISTM that from a long term maintenance
standpoint, combine should try to be volatile safe before it passes
anything along to the backends.
Combine already has to be volatile aware since it can't do things like
eliminate a volatile read. So I'm not sure how the case Vineet is
looking at is meaningfully different.
Jeff
