[PATCH] D152706: [AMDGPU] Use SSAUpdater in PromoteAlloca

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

As a follow up patch should remove the sroa run from the pass pipeline



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:143
     AU.setPreservesCFG();
+    AU.setPreservesAll();
     FunctionPass::getAnalysisUsage(AU);
----------------
I think this is an aggressive statement, why not just preserves DominatorTree (also, I'd expect that to be implied by setPreservesCFG)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:557-559
+  // Sort the worklist by dominance so we can use SSAUpdater.
+  sort(WorkList,
+       [&](Instruction *A, Instruction *B) { return DT.dominates(A, B); });
----------------
I don't understand why you would need to do this. You aren't moving any instructions around, so the dominance properties should be implied by the original values.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:683
+  // Alloca should now be dead too.
+  assert(Alloca.getNumUses() == 0);
+  Alloca.eraseFromParent();
----------------
use_empty


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152706/new/

https://reviews.llvm.org/D152706