[llvm] [RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors (PR #79476)
Orlando Cazalet-Hyams via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 25 09:45:00 PST 2024
https://github.com/OCHyams created https://github.com/llvm/llvm-project/pull/79476
[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
>From ff5deddfa2f6345f29c6d35c07aa85906ff20b3a Mon Sep 17 00:00:00 2001
From: OCHyams <orlando.hyamso at sony.com>
Date: Thu, 25 Jan 2024 10:34:26 +0000
Subject: [PATCH] [RemoveDIs] Handle DPValues in hoistCommonCodeFromSuccessors
This involves hoisting DPValues that are identical in lock-step attached
to each instruction being considered for hoisting (i.e., we also need
to do it for the final instructions which are not hoisted, because
dbg.values after the last "real" instruction would be hoisted).
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
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 99 ++++++++++++++++---
.../Transforms/SimplifyCFG/X86/pr39187-g.ll | 1 +
.../SimplifyCFG/hoist-dbgvalue-inlined.ll | 1 +
.../Transforms/SimplifyCFG/hoist-dbgvalue.ll | 1 +
4 files changed, 90 insertions(+), 12 deletions(-)
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(
More information about the llvm-commits
mailing list