[llvm] [RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors (PR #79476)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 25 09:45:35 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Orlando Cazalet-Hyams (OCHyams)
<details>
<summary>Changes</summary>
[RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors
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
---
Full diff: https://github.com/llvm/llvm-project/pull/79476.diff
4 Files Affected:
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+87-12)
- (modified) llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll (+1)
- (modified) llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll (+1)
- (modified) llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll (+1)
``````````diff
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index f3994b6cc39fefb..45bf4483f0d62e7 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1526,6 +1526,63 @@ 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 any Current == End.
+ auto atEnd = [&]() {
+ return any_of(Itrs,
+ [](const CurrentAndEndIt &P) { return P.first == P.second; });
+ };
+ // Return true if all Current are identical.
+ auto allIdentical = [&]() {
+ 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 (!atEnd()) {
+ bool HoistDPVs = allIdentical();
+ 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.
@@ -1624,18 +1681,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 +1722,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());
+ // we remove the now redundant second instruction.s
+ 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 +1759,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,9 +1823,13 @@ 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()) {
I1->replaceAllUsesWith(NT);
for (Instruction *OtherSuccTI : OtherSuccTIs)
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll b/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
index 52dc284c7959486..8cc466ff82e5d90 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 bdf4802480abf12..e00d1daf71de58c 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 1979c5096ab629e..af7da45ec089ccc 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(
``````````
</details>
https://github.com/llvm/llvm-project/pull/79476
More information about the llvm-commits
mailing list