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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 04:26:49 PDT 2023


arsenm added a comment.

I just came across this when trying to figure out what your issue is:

  /// Helper class for promoting a collection of loads and stores into SSA
  /// Form using the SSAUpdater.
  ///
  **/// This handles complexities that SSAUpdater doesn't, such as multiple loads
  /// and stores in one block.**
  ///
  /// Clients of this class are expected to subclass this and implement the
  /// virtual methods.
  class LoadAndStorePromoter {

If you move the vector insert/extract details into an implementation of this, do the dominance issues disappear?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:341-358
+  bool HasSeenBB = false;
+  BasicBlock *BB = I->getParent();
+  while (InsertIt != WorkList.end()) {
+    // -> Instruction is in the same basic block. If the new instruction comes
+    // before it, insert here.
+    //
+    // -> Instruction is in a different basic block: If we've already seen the
----------------
This whole thing feels wrong to me. You shouldn't need to figure out any ordering, much less by using comesBefore repeatedly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152706



More information about the llvm-commits mailing list