[PATCH] D109134: [MemorySSA] Support invariant.group metadata
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 3 00:54:12 PDT 2021
nikic added inline comments.
================
Comment at: llvm/lib/Analysis/MemorySSA.cpp:2512
+
+ for (const Use &Us : Ptr->uses()) {
+ auto *U = dyn_cast<Instruction>(Us.getUser());
----------------
You can iterate directly over `Ptr->users()` here.
================
Comment at: llvm/lib/Analysis/MemorySSA.cpp:2534
+ getLoadStorePointerOperand(U) == Ptr) {
+ if (DT.dominates(U, MostDominatingInstruction))
+ MostDominatingInstruction = U;
----------------
This check is now redundant, already checked above.
================
Comment at: llvm/lib/Analysis/MemorySSA.cpp:2559
+ assert(ClobberMA);
+ if (auto *LI = dyn_cast<LoadInst>(I)) {
+ auto *ClobberDA = ClobberMA->getDefiningAccess();
----------------
If the found load is atomic or volatile, wouldn't it itself be the clobbering access? If yes, we should check whether ClobberMA is a MemoryUse rather than whether it is a load. Not sure whether invariant group semantics somehow preclude this.
================
Comment at: llvm/test/Analysis/MemorySSA/invariant-groups.ll:5
+define i32 @global() {
+; CHECK: 1 = MemoryDef(liveOnEntry)
----------------
Could use a TODO/FIXME comment that this is just a missing optimization, not a correctness test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109134/new/
https://reviews.llvm.org/D109134
More information about the llvm-commits
mailing list