[llvm] 98a021f - [DebugInfo] Attempt to preserve more information during tail duplication
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 3 07:30:55 PST 2021
Author: Stephen Tozer
Date: 2021-12-03T15:30:05Z
New Revision: 98a021fcbfe104f98c7b67c35af4bbbccc3c1b8f
URL: https://github.com/llvm/llvm-project/commit/98a021fcbfe104f98c7b67c35af4bbbccc3c1b8f
DIFF: https://github.com/llvm/llvm-project/commit/98a021fcbfe104f98c7b67c35af4bbbccc3c1b8f.diff
LOG: [DebugInfo] Attempt to preserve more information during tail duplication
Prior to this patch, tail duplication handled debug info poorly -
specifically, debug instructions would be dropped instead of being set
undef, potentially extending the lifetimes of prior debug values that
should be killed. The pass was also very aggressive with dropping debug
info, dropping debug info even when the SSA value it referred to was
still present. This patch attempts to handle debug info more carefully,
checking to see whether each affected debug value can still be live,
setting it undef if not.
Reviewed By: jmorse
Differential Revision: https://reviews.llvm.org/D106875
Added:
Modified:
llvm/include/llvm/CodeGen/MachineSSAUpdater.h
llvm/lib/CodeGen/MachineSSAUpdater.cpp
llvm/lib/CodeGen/TailDuplicator.cpp
llvm/test/CodeGen/X86/tail-dup-debugvalue.mir
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineSSAUpdater.h b/llvm/include/llvm/CodeGen/MachineSSAUpdater.h
index 0af356e376ab8..3f0b55e0abb86 100644
--- a/llvm/include/llvm/CodeGen/MachineSSAUpdater.h
+++ b/llvm/include/llvm/CodeGen/MachineSSAUpdater.h
@@ -77,7 +77,9 @@ class MachineSSAUpdater {
Register GetValueAtEndOfBlock(MachineBasicBlock *BB);
/// GetValueInMiddleOfBlock - Construct SSA form, materializing a value that
- /// is live in the middle of the specified block.
+ /// is live in the middle of the specified block. If ExistingValueOnly is
+ /// true then this will only return an existing value or $noreg; otherwise new
+ /// instructions may be inserted to materialize a value.
///
/// GetValueInMiddleOfBlock is the same as GetValueAtEndOfBlock except in one
/// important case: if there is a definition of the rewritten value after the
@@ -94,7 +96,8 @@ class MachineSSAUpdater {
/// their respective blocks. However, the use of X happens in the *middle* of
/// a block. Because of this, we need to insert a new PHI node in SomeBB to
/// merge the appropriate values, and this value isn't live out of the block.
- Register GetValueInMiddleOfBlock(MachineBasicBlock *BB);
+ Register GetValueInMiddleOfBlock(MachineBasicBlock *BB,
+ bool ExistingValueOnly = false);
/// RewriteUse - Rewrite a use of the symbolic value. This handles PHI nodes,
/// which use their value in the corresponding predecessor. Note that this
@@ -104,7 +107,10 @@ class MachineSSAUpdater {
void RewriteUse(MachineOperand &U);
private:
- Register GetValueAtEndOfBlockInternal(MachineBasicBlock *BB);
+ // If ExistingValueOnly is true, will not create any new instructions. Used
+ // for debug values, which cannot modify Codegen.
+ Register GetValueAtEndOfBlockInternal(MachineBasicBlock *BB,
+ bool ExistingValueOnly = false);
};
} // end namespace llvm
diff --git a/llvm/lib/CodeGen/MachineSSAUpdater.cpp b/llvm/lib/CodeGen/MachineSSAUpdater.cpp
index 930677e4fd7d3..48076663ddf53 100644
--- a/llvm/lib/CodeGen/MachineSSAUpdater.cpp
+++ b/llvm/lib/CodeGen/MachineSSAUpdater.cpp
@@ -126,7 +126,9 @@ MachineInstrBuilder InsertNewDef(unsigned Opcode,
}
/// GetValueInMiddleOfBlock - Construct SSA form, materializing a value that
-/// is live in the middle of the specified block.
+/// is live in the middle of the specified block. If ExistingValueOnly is
+/// true then this will only return an existing value or $noreg; otherwise new
+/// instructions may be inserted to materialize a value.
///
/// GetValueInMiddleOfBlock is the same as GetValueAtEndOfBlock except in one
/// important case: if there is a definition of the rewritten value after the
@@ -143,14 +145,18 @@ MachineInstrBuilder InsertNewDef(unsigned Opcode,
/// their respective blocks. However, the use of X happens in the *middle* of
/// a block. Because of this, we need to insert a new PHI node in SomeBB to
/// merge the appropriate values, and this value isn't live out of the block.
-Register MachineSSAUpdater::GetValueInMiddleOfBlock(MachineBasicBlock *BB) {
+Register MachineSSAUpdater::GetValueInMiddleOfBlock(MachineBasicBlock *BB,
+ bool ExistingValueOnly) {
// If there is no definition of the renamed variable in this block, just use
// GetValueAtEndOfBlock to do our work.
if (!HasValueForBlock(BB))
- return GetValueAtEndOfBlockInternal(BB);
+ return GetValueAtEndOfBlockInternal(BB, ExistingValueOnly);
// If there are no predecessors, just return undef.
if (BB->pred_empty()) {
+ // If we cannot insert new instructions, just return $noreg.
+ if (ExistingValueOnly)
+ return Register();
// Insert an implicit_def to represent an undef value.
MachineInstr *NewDef = InsertNewDef(TargetOpcode::IMPLICIT_DEF,
BB, BB->getFirstTerminator(),
@@ -165,7 +171,7 @@ Register MachineSSAUpdater::GetValueInMiddleOfBlock(MachineBasicBlock *BB) {
bool isFirstPred = true;
for (MachineBasicBlock *PredBB : BB->predecessors()) {
- Register PredVal = GetValueAtEndOfBlockInternal(PredBB);
+ Register PredVal = GetValueAtEndOfBlockInternal(PredBB, ExistingValueOnly);
PredValues.push_back(std::make_pair(PredBB, PredVal));
// Compute SingularValue.
@@ -185,6 +191,10 @@ Register MachineSSAUpdater::GetValueInMiddleOfBlock(MachineBasicBlock *BB) {
if (DupPHI)
return DupPHI;
+ // If we cannot create new instructions, return $noreg now.
+ if (ExistingValueOnly)
+ return Register();
+
// Otherwise, we do need a PHI: insert one now.
MachineBasicBlock::iterator Loc = BB->empty() ? BB->end() : BB->begin();
MachineInstrBuilder InsertedPHI = InsertNewDef(TargetOpcode::PHI, BB,
@@ -350,10 +360,13 @@ class SSAUpdaterTraits<MachineSSAUpdater> {
/// for the specified BB and if so, return it. If not, construct SSA form by
/// first calculating the required placement of PHIs and then inserting new
/// PHIs where needed.
-Register MachineSSAUpdater::GetValueAtEndOfBlockInternal(MachineBasicBlock *BB){
+Register
+MachineSSAUpdater::GetValueAtEndOfBlockInternal(MachineBasicBlock *BB,
+ bool ExistingValueOnly) {
AvailableValsTy &AvailableVals = getAvailableVals(AV);
- if (Register V = AvailableVals[BB])
- return V;
+ Register ExistingVal = AvailableVals.lookup(BB);
+ if (ExistingVal || ExistingValueOnly)
+ return ExistingVal;
SSAUpdaterImpl<MachineSSAUpdater> Impl(this, &AvailableVals, InsertedPHIs);
return Impl.GetValue(BB);
diff --git a/llvm/lib/CodeGen/TailDuplicator.cpp b/llvm/lib/CodeGen/TailDuplicator.cpp
index 54fc6ee45d00d..22ee6b47c85f1 100644
--- a/llvm/lib/CodeGen/TailDuplicator.cpp
+++ b/llvm/lib/CodeGen/TailDuplicator.cpp
@@ -213,29 +213,30 @@ bool TailDuplicator::tailDuplicateAndUpdate(
SSAUpdate.AddAvailableValue(SrcBB, SrcReg);
}
+ SmallVector<MachineOperand *> DebugUses;
// Rewrite uses that are outside of the original def's block.
MachineRegisterInfo::use_iterator UI = MRI->use_begin(VReg);
- // Only remove instructions after loop, as DBG_VALUE_LISTs with multiple
- // uses of VReg may invalidate the use iterator when erased.
- SmallPtrSet<MachineInstr *, 4> InstrsToRemove;
while (UI != MRI->use_end()) {
MachineOperand &UseMO = *UI;
MachineInstr *UseMI = UseMO.getParent();
++UI;
+ // Rewrite debug uses last so that they can take advantage of any
+ // register mappings introduced by other users in its BB, since we
+ // cannot create new register definitions specifically for the debug
+ // instruction (as debug instructions should not affect CodeGen).
if (UseMI->isDebugValue()) {
- // SSAUpdate can replace the use with an undef. That creates
- // a debug instruction that is a kill.
- // FIXME: Should it SSAUpdate job to delete debug instructions
- // instead of replacing the use with undef?
- InstrsToRemove.insert(UseMI);
+ DebugUses.push_back(&UseMO);
continue;
}
if (UseMI->getParent() == DefBB && !UseMI->isPHI())
continue;
SSAUpdate.RewriteUse(UseMO);
}
- for (auto *MI : InstrsToRemove)
- MI->eraseFromParent();
+ for (auto *UseMO : DebugUses) {
+ MachineInstr *UseMI = UseMO->getParent();
+ UseMO->setReg(
+ SSAUpdate.GetValueInMiddleOfBlock(UseMI->getParent(), true));
+ }
}
SSAUpdateVRs.clear();
diff --git a/llvm/test/CodeGen/X86/tail-dup-debugvalue.mir b/llvm/test/CodeGen/X86/tail-dup-debugvalue.mir
index 835d276c99289..6fe93f04fbe69 100644
--- a/llvm/test/CodeGen/X86/tail-dup-debugvalue.mir
+++ b/llvm/test/CodeGen/X86/tail-dup-debugvalue.mir
@@ -1,10 +1,18 @@
# RUN: llc -run-pass=early-tailduplication -mtriple=x86_64-unknown-linux-gnu %s -o - | FileCheck %s
# Tail Duplication may update SSA values and invalidate any DBG_VALUEs that
-# use those values; those DBG_VALUEs should be deleted. This behaviour is tested
+# use those values; those DBG_VALUEs should be set undef. This is tested
# for DBG_VALUE users, and DBG_VALUE_LISTs that use the value multiple times.
-# CHECK-NOT: DBG_VALUE
+# CHECK: ![[VAR_J:[0-9]+]] = !DILocalVariable(name: "j"
+# CHECK: ![[VAR_K:[0-9]+]] = !DILocalVariable(name: "k"
+# CHECK-LABEL: bb.1.L:
+# CHECK: %[[REGISTER:[0-9]+]]:gr32 = PHI
+# CHECK-LABEL: bb.2.if.end4:
+# CHECK: DBG_VALUE_LIST ![[VAR_J]],
+# CHECK-SAME: %[[REGISTER]], 1, %[[REGISTER]]
+# CHECK: DBG_VALUE %[[REGISTER]], $noreg, ![[VAR_K]]
+
--- |
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
More information about the llvm-commits
mailing list