Skip to content

Commit c76b0eb

Browse files
committed
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 0aacd44.
1 parent 4e58f03 commit c76b0eb

File tree

2 files changed

+39
-53
lines changed

2 files changed

+39
-53
lines changed

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

+39-52
Original file line numberDiff line numberDiff line change
@@ -263,73 +263,60 @@ static InstructionCost ComputeSpeculationCost(const Instruction *I,
263263
bool SpeculativeExecutionPass::considerHoistingFromTo(
264264
BasicBlock &FromBlock, BasicBlock &ToBlock) {
265265
SmallPtrSet<const Instruction *, 8> NotHoisted;
266-
SmallDenseMap<const Instruction *, SmallVector<DPValue *>> DPValuesToHoist;
267-
auto HasNoUnhoistedInstr = [&NotHoisted](auto Values) {
268-
for (const Value *V : Values) {
269-
if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
270-
if (!NotHoisted.contains(I))
271-
return true;
266+
const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) {
267+
// Debug variable has special operand to check it's not hoisted.
268+
if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U)) {
269+
return all_of(DVI->location_ops(), [&NotHoisted](Value *V) {
270+
if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
271+
if (!NotHoisted.contains(I))
272+
return true;
273+
}
274+
return false;
275+
});
276+
}
277+
278+
// Usially debug label intrinsic corresponds to label in LLVM IR. In these
279+
// cases we should not move it here.
280+
// TODO: Possible special processing needed to detect it is related to a
281+
// hoisted instruction.
282+
if (isa<DbgLabelInst>(U))
283+
return false;
284+
285+
for (const Value *V : U->operand_values()) {
286+
if (const Instruction *I = dyn_cast<Instruction>(V)) {
287+
if (NotHoisted.contains(I))
288+
return false;
272289
}
273290
}
274-
return false;
291+
return true;
275292
};
276-
auto AllPrecedingUsesFromBlockHoisted =
277-
[&HasNoUnhoistedInstr](const User *U) {
278-
// Debug variable has special operand to check it's not hoisted.
279-
if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U))
280-
return HasNoUnhoistedInstr(DVI->location_ops());
281-
282-
// Usially debug label intrinsic corresponds to label in LLVM IR. In
283-
// these cases we should not move it here.
284-
// TODO: Possible special processing needed to detect it is related to a
285-
// hoisted instruction.
286-
if (isa<DbgLabelInst>(U))
287-
return false;
288-
289-
return HasNoUnhoistedInstr(U->operand_values());
290-
};
291293

292294
InstructionCost TotalSpeculationCost = 0;
293295
unsigned NotHoistedInstCount = 0;
294296
for (const auto &I : FromBlock) {
295-
// Make note of any DPValues that need hoisting.
296-
for (DPValue &DPV : I.getDbgValueRange()) {
297-
if (HasNoUnhoistedInstr(DPV.location_ops()))
298-
DPValuesToHoist[DPV.getInstruction()].push_back(&DPV);
299-
300-
const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI);
301-
if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) &&
302-
AllPrecedingUsesFromBlockHoisted(&I)) {
303-
TotalSpeculationCost += Cost;
304-
if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
305-
return false; // too much to hoist
306-
} else {
307-
// Debug info intrinsics should not be counted for threshold.
308-
if (!isa<DbgInfoIntrinsic>(I))
309-
NotHoistedInstCount++;
310-
if (NotHoistedInstCount > SpecExecMaxNotHoisted)
311-
return false; // too much left behind
312-
NotHoisted.insert(&I);
313-
}
297+
const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI);
298+
if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) &&
299+
AllPrecedingUsesFromBlockHoisted(&I)) {
300+
TotalSpeculationCost += Cost;
301+
if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
302+
return false; // too much to hoist
303+
} else {
304+
// Debug info intrinsics should not be counted for threshold.
305+
if (!isa<DbgInfoIntrinsic>(I))
306+
NotHoistedInstCount++;
307+
if (NotHoistedInstCount > SpecExecMaxNotHoisted)
308+
return false; // too much left behind
309+
NotHoisted.insert(&I);
314310
}
311+
}
315312

316313
for (auto I = FromBlock.begin(); I != FromBlock.end();) {
317-
// If any DPValues attached to this instruction should be hoisted, hoist
318-
// them now - they will end up attached to either the next hoisted
319-
// instruction or the ToBlock terminator.
320-
if (DPValuesToHoist.contains(&*I)) {
321-
for (auto *DPV : DPValuesToHoist[&*I]) {
322-
DPV->removeFromParent();
323-
ToBlock.insertDPValueBefore(DPV,
324-
ToBlock.getTerminator()->getIterator());
325-
}
326-
}
327314
// We have to increment I before moving Current as moving Current
328315
// changes the list that I is iterating through.
329316
auto Current = I;
330317
++I;
331318
if (!NotHoisted.count(&*Current)) {
332-
Current->moveBefore(ToBlock.getTerminator());
319+
Current->moveBeforePreserving(ToBlock.getTerminator());
333320
}
334321
}
335322
return true;

llvm/test/Transforms/SpeculativeExecution/PR46267.ll

-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
; RUN: opt < %s -S -passes='speculative-execution' | FileCheck %s
2-
; RUN: opt --try-experimental-debuginfo-iterators < %s -S -passes='speculative-execution' | FileCheck %s
32

43
%class.B = type { ptr }
54

0 commit comments

Comments
 (0)