[llvm] c76b0eb - Revert "[RemoveDIs][DebugInfo] Hoist DPValues in SpeculativeExecution (#80886)"

[Stephen Tozer via llvm-commits]

Author: Stephen Tozer
Date: 2024-02-07T14:10:57Z
New Revision: c76b0eb898d1e5edc416b658a6902163d64db1ce

URL: https://github.com/llvm/llvm-project/commit/c76b0eb898d1e5edc416b658a6902163d64db1ce
DIFF: https://github.com/llvm/llvm-project/commit/c76b0eb898d1e5edc416b658a6902163d64db1ce.diff

LOG: Revert "[RemoveDIs][DebugInfo] Hoist DPValues in SpeculativeExecution (#80886)"

Reverted due to buildbot failures resulting from failed compile due to a
missing brace error that got into the original commit.

This reverts commit 0aacd44a4698da012ee0704815123b28f2a06d0f.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
    llvm/test/Transforms/SpeculativeExecution/PR46267.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
index eebee87c2f514d..7a5318d4404ca4 100644
--- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
+++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
@@ -263,73 +263,60 @@ static InstructionCost ComputeSpeculationCost(const Instruction *I,
 bool SpeculativeExecutionPass::considerHoistingFromTo(
     BasicBlock &FromBlock, BasicBlock &ToBlock) {
   SmallPtrSet<const Instruction *, 8> NotHoisted;
-  SmallDenseMap<const Instruction *, SmallVector<DPValue *>> DPValuesToHoist;
-  auto HasNoUnhoistedInstr = [&NotHoisted](auto Values) {
-    for (const Value *V : Values) {
-      if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
-        if (!NotHoisted.contains(I))
-          return true;
+  const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) {
+    // Debug variable has special operand to check it's not hoisted.
+    if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U)) {
+      return all_of(DVI->location_ops(), [&NotHoisted](Value *V) {
+        if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
+          if (!NotHoisted.contains(I))
+            return true;
+        }
+        return false;
+      });
+    }
+
+    // Usially debug label intrinsic corresponds to label in LLVM IR. In these
+    // cases we should not move it here.
+    // TODO: Possible special processing needed to detect it is related to a
+    // hoisted instruction.
+    if (isa<DbgLabelInst>(U))
+      return false;
+
+    for (const Value *V : U->operand_values()) {
+      if (const Instruction *I = dyn_cast<Instruction>(V)) {
+        if (NotHoisted.contains(I))
+          return false;
       }
     }
-    return false;
+    return true;
   };
-  auto AllPrecedingUsesFromBlockHoisted =
-      [&HasNoUnhoistedInstr](const User *U) {
-        // Debug variable has special operand to check it's not hoisted.
-        if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U))
-          return HasNoUnhoistedInstr(DVI->location_ops());
-
-        // Usially debug label intrinsic corresponds to label in LLVM IR. In
-        // these cases we should not move it here.
-        // TODO: Possible special processing needed to detect it is related to a
-        // hoisted instruction.
-        if (isa<DbgLabelInst>(U))
-          return false;
-
-        return HasNoUnhoistedInstr(U->operand_values());
-      };
 
   InstructionCost TotalSpeculationCost = 0;
   unsigned NotHoistedInstCount = 0;
   for (const auto &I : FromBlock) {
-    // Make note of any DPValues that need hoisting.
-    for (DPValue &DPV : I.getDbgValueRange()) {
-      if (HasNoUnhoistedInstr(DPV.location_ops()))
-        DPValuesToHoist[DPV.getInstruction()].push_back(&DPV);
-
-      const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI);
-      if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) &&
-          AllPrecedingUsesFromBlockHoisted(&I)) {
-        TotalSpeculationCost += Cost;
-        if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
-          return false; // too much to hoist
-      } else {
-        // Debug info intrinsics should not be counted for threshold.
-        if (!isa<DbgInfoIntrinsic>(I))
-          NotHoistedInstCount++;
-        if (NotHoistedInstCount > SpecExecMaxNotHoisted)
-          return false; // too much left behind
-        NotHoisted.insert(&I);
-      }
+    const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI);
+    if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) &&
+        AllPrecedingUsesFromBlockHoisted(&I)) {
+      TotalSpeculationCost += Cost;
+      if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
+        return false;  // too much to hoist
+    } else {
+      // Debug info intrinsics should not be counted for threshold.
+      if (!isa<DbgInfoIntrinsic>(I))
+        NotHoistedInstCount++;
+      if (NotHoistedInstCount > SpecExecMaxNotHoisted)
+        return false; // too much left behind
+      NotHoisted.insert(&I);
     }
+  }
 
   for (auto I = FromBlock.begin(); I != FromBlock.end();) {
-    // If any DPValues attached to this instruction should be hoisted, hoist
-    // them now - they will end up attached to either the next hoisted
-    // instruction or the ToBlock terminator.
-    if (DPValuesToHoist.contains(&*I)) {
-      for (auto *DPV : DPValuesToHoist[&*I]) {
-        DPV->removeFromParent();
-        ToBlock.insertDPValueBefore(DPV,
-                                    ToBlock.getTerminator()->getIterator());
-      }
-    }
     // We have to increment I before moving Current as moving Current
     // changes the list that I is iterating through.
     auto Current = I;
     ++I;
     if (!NotHoisted.count(&*Current)) {
-      Current->moveBefore(ToBlock.getTerminator());
+      Current->moveBeforePreserving(ToBlock.getTerminator());
     }
   }
   return true;

diff  --git a/llvm/test/Transforms/SpeculativeExecution/PR46267.ll b/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
index c27b492b4b8765..00f80d3e33a6ca 100644
--- a/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
+++ b/llvm/test/Transforms/SpeculativeExecution/PR46267.ll
@@ -1,5 +1,4 @@
 ; RUN: opt < %s -S -passes='speculative-execution' | FileCheck %s
-; RUN: opt --try-experimental-debuginfo-iterators < %s -S -passes='speculative-execution' | FileCheck %s
 
 %class.B = type { ptr }