[llvm] [RFC] Memory Model Relaxation Annotations (PR #78569)
Pierre van Houtryve via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 21 06:58:10 PST 2024
================
@@ -698,6 +700,16 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
assert(I1->getOpcode() == I2->getOpcode() &&
"Can not compare special state of different instructions");
+ // MMRAs may change semantics of an operation, e.g. make a fence only
+ // affect a given address space.
+ //
+ // FIXME: Not sure if this stinks or not. Maybe we should just look at
+ // all callers and make them check MMRAs.
+ // OTOH, MMRAs can really alter semantics so this is technically correct
+ // (the best kind of correct).
+ if (MMRAMetadata(*this) != MMRAMetadata(*I2))
----------------
Pierre-vh wrote:
Most of these checks are necessary, for instance one of the use case we tested basically has (in pseudocode):
```
if (A)
fence release, !mmra !0
else if (B)
fence release, !mmra !1
else
fence release, !mmra !2
```
And SimplifyCFG would just remove all CF and leave `fence release`, undoing all the work. Such cases would arise in OpenCL code whenever one or two address spaces are being fenced instead of all of them.
I'm really hesitant about making this an instruction operand. I really don't mind doing the work if it makes a better end result, but I'm not yet convinced it's strictly better.
This is unfortunately right in the middle between MD and operand so I guess which one should be used depends on the point of view. IMO it's better to make it metadata because this has much more in common with other MD nodes than with something like syncscope (which can never be dropped/merged/etc.)
Also we wouldn't treat every instance of dropped metadata like a bug, it's more nuanced. Dropping MMRAs doesn't affect correctness, it just has a performance cost of X, but if an optimization being disabled has a performance cost of Y and Y > X, then the optimization must take priority and be allowed to run and drop MMRAs anyway.
If the amount of pass changes is going to be a blocker I'm open to taking a less conservative approach and just leave the Transform changes that I can prove actively hurt MMRAs (so if I have real-world evidence that that pass caused problems). IIRC that'd only leave SimplifyCFG but I'd need to run more tests. I can then make more changes over time if needed. Would that be better?
https://github.com/llvm/llvm-project/pull/78569
More information about the llvm-commits
mailing list