Moved DxilEraseDeadRegions to after DxilRemoveDeadBlocks (#4397)

include/dxc/DXIL/DxilUtil.h

@@ -169,6 +169,10 @@ namespace dxilutil {
     return mdv_users_begin(V) == mdv_users_end(V);
   }
 
+  /// Finds all allocas that only have stores and delete them.
+  /// These allocas hold on to values that do not contribute to the
+  /// shader's results.
+  bool DeleteDeadAllocas(llvm::Function &F);
 }
 
 }

lib/DXIL/DxilUtil.cpp

@@ -994,5 +994,86 @@ Value *TryReplaceBaseCastWithGep(Value *V) {
   return nullptr;
 }
 
+struct AllocaDeleter {
+  SmallVector<Value *, 10> WorkList;
+  std::unordered_set<Value *> Seen;
+
+  void Add(Value *V) {
+    if (!Seen.count(V)) {
+      Seen.insert(V);
+      WorkList.push_back(V);
+    }
+  }
+
+  bool TryDeleteUnusedAlloca(AllocaInst *AI) {
+    Seen.clear();
+    WorkList.clear();
+
+    Add(AI);
+    while (WorkList.size()) {
+      Value *V = WorkList.pop_back_val();
+      // Keep adding users if we encounter one of these.
+      // None of them imply the alloca is being read.
+      if (isa<GEPOperator>(V) ||
+          isa<BitCastOperator>(V) ||
+          isa<AllocaInst>(V) ||
+          isa<StoreInst>(V))
+      {
+        for (User *U : V->users())
+          Add(U);
+      }
+      // If it's anything else, we'll assume it's reading the
+      // alloca. Give up.
+      else {
+        return false;
+      }
+    }
+
+    if (!Seen.size())
+      return false;
+
+    // Delete all the instructions associated with the
+    // alloca.
+    for (Value *V : Seen) {
+      Instruction *I = dyn_cast<Instruction>(V);
+      if (I) {
+        I->dropAllReferences();
+      }
+    }
+    for (Value *V : Seen) {
+      Instruction *I = dyn_cast<Instruction>(V);
+      if (I) {
+        I->eraseFromParent();
+      }
+    }
+
+    return true;
+  }
+};
+
+bool DeleteDeadAllocas(llvm::Function &F) {
+  if (F.empty())
+    return false;
+
+  AllocaDeleter Deleter;
+  BasicBlock &Entry = *F.begin();
+  bool Changed = false;
+
+  while (1) {
+    bool LocalChanged = false;
+    for (auto it = Entry.begin(), end = Entry.end(); it != end;) {
+      AllocaInst *AI = dyn_cast<AllocaInst>(&*(it++));
+      if (!AI)
+        continue;
+      LocalChanged |= Deleter.TryDeleteUnusedAlloca(AI);
+    }
+    Changed |= LocalChanged;
+    if (!LocalChanged)
+      break;
+  }
+
+  return Changed;
+}
+
 }
 }

lib/Transforms/IPO/PassManagerBuilder.cpp

@@ -375,9 +375,9 @@ void PassManagerBuilder::populateModulePassManager(
     if (!HLSLHighLevel) {
       MPM.add(createDxilConvergentClearPass());
       MPM.add(createDxilSimpleGVNEliminateRegionPass());
-      MPM.add(createDxilEraseDeadRegionPass());
       MPM.add(createDeadCodeEliminationPass());
       MPM.add(createDxilRemoveDeadBlocksPass());
+      MPM.add(createDxilEraseDeadRegionPass());
       MPM.add(createDxilNoOptSimplifyInstructionsPass());
       MPM.add(createGlobalOptimizerPass());
       MPM.add(createMultiDimArrayToOneDimArrayPass());
@@ -937,4 +937,4 @@ void LLVMPassManagerBuilderPopulateLTOPassManager(LLVMPassManagerBuilderRef PMB,
 
   Builder->populateLTOPassManager(*LPM);
 }
-#endif
+#endif

lib/Transforms/Scalar/DxilEraseDeadRegion.cpp

@@ -34,6 +34,7 @@
 #include "dxc/DXIL/DxilOperations.h"
 #include "dxc/DXIL/DxilMetadataHelper.h"
 #include "dxc/HLSL/DxilNoops.h"
+#include "dxc/DXIL/DxilUtil.h"
 
 #include <unordered_map>
 #include <unordered_set>
@@ -358,6 +359,7 @@ struct DxilEraseDeadRegion : public FunctionPass {
     while (1) {
       bool LocalChanged = false;
 
+      LocalChanged |= hlsl::dxilutil::DeleteDeadAllocas(F);
       LocalChanged |= this->TryRemoveSimpleDeadLoops(LI);
 
       for (Function::iterator It = F.begin(), E = F.end(); It != E; It++) {

lib/Transforms/Scalar/DxilRemoveDeadBlocks.cpp

@@ -44,94 +44,6 @@ static void RemoveIncomingValueFrom(BasicBlock *SuccBB, BasicBlock *BB) {
   }
 }
 
-namespace {
-
-struct AllocaDeleter {
-  SmallVector<Value *, 10> WorkList;
-  std::unordered_set<Value *> Seen;
-
-  void Add(Value *V) {
-    if (!Seen.count(V)) {
-      Seen.insert(V);
-      WorkList.push_back(V);
-    }
-  }
-
-  bool TryDeleteUnusedAlloca(AllocaInst *AI) {
-    Seen.clear();
-    WorkList.clear();
-
-    Add(AI);
-    while (WorkList.size()) {
-      Value *V = WorkList.pop_back_val();
-      // Keep adding users if we encounter one of these.
-      // None of them imply the alloca is being read.
-      if (isa<GEPOperator>(V) ||
-          isa<BitCastOperator>(V) ||
-          isa<AllocaInst>(V) ||
-          isa<StoreInst>(V))
-      {
-        for (User *U : V->users())
-          Add(U);
-      }
-      // If it's anything else, we'll assume it's reading the
-      // alloca. Give up.
-      else {
-        return false;
-      }
-    }
-
-    if (!Seen.size())
-      return false;
-
-    // Delete all the instructions associated with the
-    // alloca.
-    for (Value *V : Seen) {
-      Instruction *I = dyn_cast<Instruction>(V);
-      if (I) {
-        I->dropAllReferences();
-      }
-    }
-    for (Value *V : Seen) {
-      Instruction *I = dyn_cast<Instruction>(V);
-      if (I) {
-        I->eraseFromParent();
-      }
-    }
-
-    return true;
-  }
-};
-
-}
-
-// Finds all allocas that only have stores and delete them.
-// These allocas hold on to values that do not contribute to the
-// shader's results.
-static bool DeleteDeadAllocas(Function &F) {
-  if (F.empty())
-    return false;
-
-  AllocaDeleter Deleter;
-  BasicBlock &Entry = *F.begin();
-  bool Changed = false;
-
-  while (1) {
-    bool LocalChanged = false;
-    for (auto it = Entry.begin(), end = Entry.end(); it != end;) {
-      AllocaInst *AI = dyn_cast<AllocaInst>(&*(it++));
-      if (!AI)
-        continue;
-      LocalChanged |= Deleter.TryDeleteUnusedAlloca(AI);
-    }
-    Changed |= LocalChanged;
-    if (!LocalChanged)
-      break;
-  }
-
-  return Changed;
-}
-
 struct DeadBlockDeleter {
   bool Run(Function &F, DxilValueCache *DVC);
 
@@ -405,7 +317,7 @@ struct DxilRemoveDeadBlocks : public FunctionPass {
   bool runOnFunction(Function &F) override {
     DxilValueCache *DVC = &getAnalysis<DxilValueCache>();
     bool Changed = false;
-    Changed |= DeleteDeadAllocas(F);
+    Changed |= hlsl::dxilutil::DeleteDeadAllocas(F);
     Changed |= DeleteDeadBlocks(F, DVC);
     Changed |= DeleteNonContributingValues(F, DVC);
     return Changed;

tools/clang/test/HLSLFileCheck/passes/dxil/dxil_remove_dead_pass/delete_dead_region_after_remove_dead_blocks.hlsl

@@ -0,0 +1,36 @@
+// RUN: %dxc %s -T ps_6_0 -Od | FileCheck %s
+
+// CBuffer member 'foo' is used for a condition to decide the value of 'cond'.
+// But 'cond' is eventually set to 0, under a compile time known condition 'my_cond'.
+//
+// The way the Od dead code removal passes were arranged was unable to clean up this pattern.
+// EraseDeadRegion would run first, but unable to delete 'if (foo)' because of outflowing
+// value 'cond' still being used.
+//
+// If we run RemoveDeadBlocks first, however, it would delete the branch `if (!my_cond)` and
+// relieve the use of 'cond', allowing EraseDeadRegion to delete `if (foo)`.
+//
+
+// CHECK: @main
+// CHECK-NOT: call %dx.types.CBufRet.f32 @dx.op.cbufferLoad
+
+cbuffer cb : register(b0) {
+  float foo;
+  float bar;
+}
+
+[RootSignature("")]
+float main(float i : I) : SV_Target {
+
+  float cond = 0;
+  if (foo) {
+    cond = sin(i);
+  }
+
+  float my_cond = 0;
+  if (!my_cond) {
+    cond = 0;
+  }
+
+  return cond ? 10 : 20;
+}