================
@@ -32,6 +33,68 @@ namespace mlir {
namespace {
+/// Find the `cir.store` operation that stores to the given alloca and
dominates
+/// the given load operation. Dominance calculation is done through the given
+/// DominanceInfo object.
+///
+/// Return nullptr if no such store operation exists. If multiple store
+/// operations satisfy the criteria, any of them may be returned.
+cir::StoreOp findDominatingInitOp(cir::AllocaOp alloca, cir::LoadOp load,
+ const DominanceInfo &domInfo) {
+ // Walk through all uses of the alloca and visit the store operations that
+ // store to the alloca
+ for (const mlir::OpOperand &use : alloca->getUses()) {
+ auto store = mlir::dyn_cast<cir::StoreOp>(use.getOwner());
+ if (!store)
+ continue;
+
+ // `cir.store` has two operands, we're only interested if the store is
+ // storing into the alloca, not if the store is storing the address of the
+ // alloca slot into somewhere else
+ if (use.getOperandNumber() != cir::StoreOp::odsIndex_addr)
----------------
erichkeane wrote:
Not sure if said function belongs on the LoadOp or or StoreOp, but SOMETHING
that looks less fragile perhaps? I was hoping someone with more opt experience
would come along and tell us that this is just 'the way' vs a better way.
>This is a very good point. I feel that it's OK to fold the load away if the
>corresponding store is atomic/volatile, but I would definitely also agree that
>we could ask some C/C++ language expert for this.
As a BIT of a language expert: I can't think of a reason why an atomic store
would be problematic, but the volatile definitely seems like it would be
problematic. ONE of the meanings of volatile is "we don't expect what we store
into a volatile variable to necessarily be what we load out of it". It seems
WEIRD to me that we'd have a volatile store but not a volatile load? But
presumably we'd want to skip out in that case too?
Atomics are a little weird to me in this case. I might suggest just skipping
on them as well, at least until we can prove out that it is ok. Again, a case
where having an atomic store but NOT load seems like it is perhaps a bug (since
atomic/volatile is part of the TYPE not the operation?), but I think the same
logic follows.
https://github.com/llvm/llvm-project/pull/212284
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
