[llvm] ddd95b1 - [RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors (#79476)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 02:58:50 PST 2024
Author: Orlando Cazalet-Hyams
Date: 2024-02-05T10:58:46Z
New Revision: ddd95b15d102331ecb7ce94dcf3a7280e0a133fa
URL: https://github.com/llvm/llvm-project/commit/ddd95b15d102331ecb7ce94dcf3a7280e0a133fa
DIFF: https://github.com/llvm/llvm-project/commit/ddd95b15d102331ecb7ce94dcf3a7280e0a133fa.diff
LOG: [RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors (#79476)
Hoist DPValues attached to each instruction being considered for hoisting if
they are identical in lock-step. This includes the final instructions which
are considered but not hoisted, because the corresponding dbg.values would
appear before those instruction and thus hoisted if identical.
Identical debug records hoisted:
llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
Non-identical debug records not hoisted:
llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
Debug records attached to first not-hoisted instructions are hoisted:
llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 254795ec244534..f5b398cae04ed6 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1526,6 +1526,62 @@ static bool shouldHoistCommonInstructions(Instruction *I1, Instruction *I2,
return true;
}
+/// Hoists DPValues from \p I1 and \p OtherInstrs that are identical in
+/// lock-step to \p TI. This matches how dbg.* intrinsics are hoisting in
+/// hoistCommonCodeFromSuccessors. e.g. The input:
+/// I1 DPVs: { x, z },
+/// OtherInsts: { I2 DPVs: { x, y, z } }
+/// would result in hoisting only DPValue x.
+static void
+hoistLockstepIdenticalDPValues(Instruction *TI, Instruction *I1,
+ SmallVectorImpl<Instruction *> &OtherInsts) {
+ if (!I1->hasDbgValues())
+ return;
+ using CurrentAndEndIt =
+ std::pair<DPValue::self_iterator, DPValue::self_iterator>;
+ // Vector of {Current, End} iterators.
+ SmallVector<CurrentAndEndIt> Itrs;
+ Itrs.reserve(OtherInsts.size() + 1);
+ // Helper lambdas for lock-step checks:
+ // Return true if this Current == End.
+ auto atEnd = [](const CurrentAndEndIt &Pair) {
+ return Pair.first == Pair.second;
+ };
+ // Return true if all Current are identical.
+ auto allIdentical = [](const SmallVector<CurrentAndEndIt> &Itrs) {
+ return all_of(make_first_range(ArrayRef(Itrs).drop_front()),
+ [&](DPValue::self_iterator I) {
+ return Itrs[0].first->isIdenticalToWhenDefined(*I);
+ });
+ };
+
+ // Collect the iterators.
+ Itrs.push_back(
+ {I1->getDbgValueRange().begin(), I1->getDbgValueRange().end()});
+ for (Instruction *Other : OtherInsts) {
+ if (!Other->hasDbgValues())
+ return;
+ Itrs.push_back(
+ {Other->getDbgValueRange().begin(), Other->getDbgValueRange().end()});
+ }
+
+ // Iterate in lock-step until any of the DPValue lists are exausted. If
+ // the lock-step DPValues are identical, hoist all of them to TI.
+ // This replicates the dbg.* intrinsic behaviour in
+ // hoistCommonCodeFromSuccessors.
+ while (none_of(Itrs, atEnd)) {
+ bool HoistDPVs = allIdentical(Itrs);
+ for (CurrentAndEndIt &Pair : Itrs) {
+ // Increment Current iterator now as we may be about to move the DPValue.
+ DPValue &DPV = *Pair.first++;
+ if (HoistDPVs) {
+ DPV.removeFromParent();
+ TI->getParent()->insertDPValueBefore(&DPV, TI->getIterator());
+ }
+ }
+ }
+}
+
/// Hoist any common code in the successor blocks up into the block. This
/// function guarantees that BB dominates all successors. If EqTermsOnly is
/// given, only perform hoisting in case both blocks only contain a terminator.
@@ -1598,7 +1654,6 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
auto OtherSuccIterRange = make_first_range(OtherSuccIterPairRange);
Instruction *I1 = &*BB1ItrPair.first;
- auto *BB1 = I1->getParent();
// Skip debug info if it is not identical.
bool AllDbgInstsAreIdentical = all_of(OtherSuccIterRange, [I1](auto &Iter) {
@@ -1624,18 +1679,23 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
AllInstsAreIdentical = false;
}
+ SmallVector<Instruction *, 8> OtherInsts;
+ for (auto &SuccIter : OtherSuccIterRange)
+ OtherInsts.push_back(&*SuccIter);
+
// If we are hoisting the terminator instruction, don't move one (making a
// broken BB), instead clone it, and remove BI.
if (HasTerminator) {
// Even if BB, which contains only one unreachable instruction, is ignored
// at the beginning of the loop, we can hoist the terminator instruction.
// If any instructions remain in the block, we cannot hoist terminators.
- if (NumSkipped || !AllInstsAreIdentical)
+ if (NumSkipped || !AllInstsAreIdentical) {
+ hoistLockstepIdenticalDPValues(TI, I1, OtherInsts);
return Changed;
- SmallVector<Instruction *, 8> Insts;
- for (auto &SuccIter : OtherSuccIterRange)
- Insts.push_back(&*SuccIter);
- return hoistSuccIdenticalTerminatorToSwitchOrIf(TI, I1, Insts) || Changed;
+ }
+
+ return hoistSuccIdenticalTerminatorToSwitchOrIf(TI, I1, OtherInsts) ||
+ Changed;
}
if (AllInstsAreIdentical) {
@@ -1660,18 +1720,25 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
// The debug location is an integral part of a debug info intrinsic
// and can't be separated from it or replaced. Instead of attempting
// to merge locations, simply hoist both copies of the intrinsic.
- I1->moveBeforePreserving(TI);
+ hoistLockstepIdenticalDPValues(TI, I1, OtherInsts);
+ // We've just hoisted DPValues; move I1 after them (before TI) and
+ // leave any that were not hoisted behind (by calling moveBefore
+ // rather than moveBeforePreserving).
+ I1->moveBefore(TI);
for (auto &SuccIter : OtherSuccIterRange) {
auto *I2 = &*SuccIter++;
assert(isa<DbgInfoIntrinsic>(I2));
- I2->moveBeforePreserving(TI);
+ I2->moveBefore(TI);
}
} else {
// For a normal instruction, we just move one to right before the
// branch, then replace all uses of the other with the first. Finally,
// we remove the now redundant second instruction.
- I1->moveBeforePreserving(TI);
- BB->splice(TI->getIterator(), BB1, I1->getIterator());
+ hoistLockstepIdenticalDPValues(TI, I1, OtherInsts);
+ // We've just hoisted DPValues; move I1 after them (before TI) and
+ // leave any that were not hoisted behind (by calling moveBefore
+ // rather than moveBeforePreserving).
+ I1->moveBefore(TI);
for (auto &SuccIter : OtherSuccIterRange) {
Instruction *I2 = &*SuccIter++;
assert(I2 != I1);
@@ -1690,8 +1757,10 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
Changed = true;
NumHoistCommonInstrs += SuccIterPairs.size();
} else {
- if (NumSkipped >= HoistCommonSkipLimit)
+ if (NumSkipped >= HoistCommonSkipLimit) {
+ hoistLockstepIdenticalDPValues(TI, I1, OtherInsts);
return Changed;
+ }
// We are about to skip over a pair of non-identical instructions. Record
// if any have characteristics that would prevent reordering instructions
// across them.
@@ -1752,7 +1821,10 @@ bool SimplifyCFGOpt::hoistSuccIdenticalTerminatorToSwitchOrIf(
}
}
- // Okay, it is safe to hoist the terminator.
+ // Hoist DPValues attached to the terminator to match dbg.* intrinsic hoisting
+ // behaviour in hoistCommonCodeFromSuccessors.
+ hoistLockstepIdenticalDPValues(TI, I1, OtherSuccTIs);
+ // Clone the terminator and hoist it into the pred, without any debug info.
Instruction *NT = I1->clone();
NT->insertInto(TIParent, TI->getIterator());
if (!NT->getType()->isVoidTy()) {
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll b/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
index 52dc284c795948..8cc466ff82e5d9 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
@@ -1,4 +1,5 @@
; RUN: opt < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -hoist-common-insts=true | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -hoist-common-insts=true | FileCheck %s
; SimplifyCFG can hoist any common code in the 'then' and 'else' blocks to
; the 'if' basic block.
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
index bdf4802480abf1..e00d1daf71de58 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
@@ -1,4 +1,5 @@
; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -hoist-common-insts=true -S < %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -hoist-common-insts=true -S < %s | FileCheck %s
; Verify that we don't crash due an invalid !dbg location on the hoisted llvm.dbg.value
define i64 @caller(ptr %ptr, i64 %flag) !dbg !10 {
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
index 1979c5096ab629..af7da45ec089cc 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s
+; RUN: opt -try-experimental-debuginfo-iterators -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s
define i32 @foo(i32 %i) nounwind ssp !dbg !0 {
; CHECK-LABEL: @foo(
More information about the llvm-commits
mailing list