[DebugInfo][RemoveDIs] Add local-utility plumbing for DPValues #72276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch re-implements a variety of debug-info maintenence functions to use DPValues instead of DbgValueInst's: supporting the "new" non-intrinsic representation of debug-info. As per [0], we need to have parallel implementations of various utilities for a time, and these are the most fundamental utilities used throughout the compiler. I've added --try-experimental-debuginfo-iterators to a variety of RUN lines: this is a flag that turns on "new debug-info" if it's built into LLVM, and not otherwise. This should ensure that we have the same behaviour for the same IR inputs, but using a different internal representation. For the most part these changes affect SROA/Mem2Reg promotion of dbg.declares into dbg.value intrinsics (now DPValues), we're leaving dbg.declares as instructions until later in the day. There's also some salvaging changes made. I believe the tests that I've added cover almost all the code being updated here. The only thing I'm not confident about is SimplifyCFG, which calls rewriteDebugUsers down a variety of code paths. Those changes can't immediately get full coverage as an additional patch is needed that updates handling of Unreachable instructions, will upload that shortly. [0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939/9
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) ChangesThis patch re-implements a variety of debug-info maintenence functions to use DPValues instead of DbgValueInst's: supporting the "new" non-intrinsic representation of debug-info. As per [0], we need to have parallel implementations of various utilities for a time, and these are the most fundamental utilities used throughout the compiler. I've added --try-experimental-debuginfo-iterators to a variety of RUN lines: this is a flag that turns on "new debug-info" if it's built into LLVM, and not otherwise. This should ensure that we have the same behaviour for the same IR inputs, but using a different internal representation. For the most part these changes affect SROA/Mem2Reg promotion of dbg.declares into dbg.value intrinsics (now DPValues), we're leaving dbg.declares as instructions until later in the day. There's also some salvaging changes made. I believe the tests that I've added cover almost all the code being updated here. The only thing I'm not confident about is SimplifyCFG, which calls rewriteDebugUsers down a variety of code paths. Those changes can't immediately get full coverage as an additional patch is needed that updates handling of Unreachable instructions, will upload that shortly. Patch is 41.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72276.diff 23 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index fa8405a6191eba8..9547677397f8021 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -262,16 +262,22 @@ CallInst *changeToCall(InvokeInst *II, DomTreeUpdater *DTU = nullptr);
/// that has an associated llvm.dbg.declare intrinsic.
void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
StoreInst *SI, DIBuilder &Builder);
+void ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ StoreInst *SI, DIBuilder &Builder);
/// Inserts a llvm.dbg.value intrinsic before a load of an alloca'd value
/// that has an associated llvm.dbg.declare intrinsic.
void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
LoadInst *LI, DIBuilder &Builder);
+void ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ LoadInst *LI, DIBuilder &Builder);
/// Inserts a llvm.dbg.value intrinsic after a phi that has an associated
/// llvm.dbg.declare intrinsic.
void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
PHINode *LI, DIBuilder &Builder);
+void ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ PHINode *LI, DIBuilder &Builder);
/// Lowers llvm.dbg.declare intrinsics into appropriate set of
/// llvm.dbg.value intrinsics.
@@ -307,7 +313,8 @@ void salvageDebugInfo(Instruction &I);
/// \p Insns, rather than all debug users from findDbgUsers( \p I).
/// Mark undef if salvaging cannot be completed.
void salvageDebugInfoForDbgValues(Instruction &I,
- ArrayRef<DbgVariableIntrinsic *> Insns);
+ ArrayRef<DbgVariableIntrinsic *> Insns,
+ ArrayRef<DPValue *> DPInsns);
/// Given an instruction \p I and DIExpression \p DIExpr operating on
/// it, append the effects of \p I to the DIExpression operand list
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 2e0b185769041cc..5940ab735e74756 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3921,6 +3921,7 @@ bool InstCombinerImpl::tryToSinkInstruction(Instruction *I,
// For all debug values in the destination block, the sunk instruction
// will still be available, so they do not need to be dropped.
SmallVector<DbgVariableIntrinsic *, 2> DbgUsersToSalvage;
+ SmallVector<DPValue *, 2> DPValuesToSalvage;
for (auto &DbgUser : DbgUsers)
if (DbgUser->getParent() != DestBlock)
DbgUsersToSalvage.push_back(DbgUser);
@@ -3964,7 +3965,10 @@ bool InstCombinerImpl::tryToSinkInstruction(Instruction *I,
// Perform salvaging without the clones, then sink the clones.
if (!DIIClones.empty()) {
- salvageDebugInfoForDbgValues(*I, DbgUsersToSalvage);
+ // RemoveDIs: pass in empty vector of DPValues until we get to instrumenting
+ // this pass.
+ SmallVector<DPValue *, 1> DummyDPValues;
+ salvageDebugInfoForDbgValues(*I, DbgUsersToSalvage, DummyDPValues);
// The clones are in reverse order of original appearance, reverse again to
// maintain the original order.
for (auto &DIIClone : llvm::reverse(DIIClones)) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index aacf66bfe38eb91..ba97cdf8f3c4b4c 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -69,6 +69,7 @@
#include "llvm/IR/Value.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/KnownBits.h"
@@ -86,6 +87,8 @@
using namespace llvm;
using namespace llvm::PatternMatch;
+extern cl::opt<bool> UseNewDbgInfoFormat;
+
#define DEBUG_TYPE "local"
STATISTIC(NumRemoved, "Number of unreachable basic blocks removed");
@@ -608,10 +611,13 @@ void llvm::RecursivelyDeleteTriviallyDeadInstructions(
bool llvm::replaceDbgUsesWithUndef(Instruction *I) {
SmallVector<DbgVariableIntrinsic *, 1> DbgUsers;
- findDbgUsers(DbgUsers, I);
+ SmallVector<DPValue *, 1> DPUsers;
+ findDbgUsers(DbgUsers, I, &DPUsers);
for (auto *DII : DbgUsers)
DII->setKillLocation();
- return !DbgUsers.empty();
+ for (auto *DPV : DPUsers)
+ DPV->setKillLocation();
+ return !DbgUsers.empty() || !DPUsers.empty();
}
/// areAllUsesEqual - Check whether the uses of a value are all the same.
@@ -1566,12 +1572,18 @@ static bool PhiHasDebugValue(DILocalVariable *DIVar,
// is removed by LowerDbgDeclare(), we need to make sure that we are
// not inserting the same dbg.value intrinsic over and over.
SmallVector<DbgValueInst *, 1> DbgValues;
- findDbgValues(DbgValues, APN);
+ SmallVector<DPValue *, 1> DPValues;
+ findDbgValues(DbgValues, APN, &DPValues);
for (auto *DVI : DbgValues) {
assert(is_contained(DVI->getValues(), APN));
if ((DVI->getVariable() == DIVar) && (DVI->getExpression() == DIExpr))
return true;
}
+ for (auto *DPV : DPValues) {
+ assert(is_contained(DPV->location_ops(), APN));
+ if ((DPV->getVariable() == DIVar) && (DPV->getExpression() == DIExpr))
+ return true;
+ }
return false;
}
@@ -1607,6 +1619,57 @@ static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
// Could not determine size of variable. Conservatively return false.
return false;
}
+// RemoveDIs: duplicate implementation of the above, using DPValues, the
+// replacement for dbg.values.
+static bool valueCoversEntireFragment(Type *ValTy, DPValue *DPV) {
+ const DataLayout &DL = DPV->getModule()->getDataLayout();
+ TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+ if (std::optional<uint64_t> FragmentSize = DPV->getFragmentSizeInBits())
+ return TypeSize::isKnownGE(ValueSize, TypeSize::Fixed(*FragmentSize));
+
+ // We can't always calculate the size of the DI variable (e.g. if it is a
+ // VLA). Try to use the size of the alloca that the dbg intrinsic describes
+ // intead.
+ if (DPV->isAddressOfVariable()) {
+ // DPV should have exactly 1 location when it is an address.
+ assert(DPV->getNumVariableLocationOps() == 1 &&
+ "address of variable must have exactly 1 location operand.");
+ if (auto *AI =
+ dyn_cast_or_null<AllocaInst>(DPV->getVariableLocationOp(0))) {
+ if (std::optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) {
+ return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+ }
+ }
+ }
+ // Could not determine size of variable. Conservatively return false.
+ return false;
+}
+
+static void insertDbgValueOrDPValue(DIBuilder &Builder, Value *DV, DILocalVariable *DIVar, DIExpression *DIExpr, const DebugLoc &NewLoc, BasicBlock::iterator Instr) {
+ if (!UseNewDbgInfoFormat) {
+ auto *DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, (Instruction*)nullptr);
+ DbgVal->insertBefore(Instr);
+ } else {
+ // RemoveDIs: if we're using the new debug-info format, allocate a
+ // DPValue directly instead of a dbg.value intrinsic.
+ ValueAsMetadata *DVAM = ValueAsMetadata::get(DV);
+ DPValue *DV = new DPValue(DVAM, DIVar, DIExpr, NewLoc.get());
+ Instr->getParent()->insertDPValueBefore(DV, Instr);
+ }
+}
+
+static void insertDbgValueOrDPValueAfter(DIBuilder &Builder, Value *DV, DILocalVariable *DIVar, DIExpression *DIExpr, const DebugLoc &NewLoc, BasicBlock::iterator Instr) {
+ if (!UseNewDbgInfoFormat) {
+ auto *DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, (Instruction*)nullptr);
+ DbgVal->insertAfter(&*Instr);
+ } else {
+ // RemoveDIs: if we're using the new debug-info format, allocate a
+ // DPValue directly instead of a dbg.value intrinsic.
+ ValueAsMetadata *DVAM = ValueAsMetadata::get(DV);
+ DPValue *DV = new DPValue(DVAM, DIVar, DIExpr, NewLoc.get());
+ Instr->getParent()->insertDPValueAfter(DV, &*Instr);
+ }
+}
/// Inserts a llvm.dbg.value intrinsic before a store to an alloca'd value
/// that has an associated llvm.dbg.declare intrinsic.
@@ -1636,7 +1699,7 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
DIExpr->isDeref() || (!DIExpr->startsWithDeref() &&
valueCoversEntireFragment(DV->getType(), DII));
if (CanConvert) {
- Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
+ insertDbgValueOrDPValue(Builder, DV, DIVar, DIExpr, NewLoc, SI->getIterator());
return;
}
@@ -1648,7 +1711,18 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
// know which part) we insert an dbg.value intrinsic to indicate that we
// know nothing about the variable's content.
DV = UndefValue::get(DV->getType());
- Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
+ insertDbgValueOrDPValue(Builder, DV, DIVar, DIExpr, NewLoc, SI->getIterator());
+}
+
+// RemoveDIs: duplicate the getDebugValueLoc method using DPValues instead of
+// dbg.value intrinsics.
+static DebugLoc getDebugValueLocDPV(DPValue *DPV, Instruction *Src) {
+ // Original dbg.declare must have a location.
+ const DebugLoc &DeclareLoc = DPV->getDebugLoc();
+ MDNode *Scope = DeclareLoc.getScope();
+ DILocation *InlinedAt = DeclareLoc.getInlinedAt();
+ // Produce an unknown location with the correct scope / inlinedAt fields.
+ return DILocation::get(DPV->getContext(), 0, 0, Scope, InlinedAt);
}
/// Inserts a llvm.dbg.value intrinsic before a load of an alloca'd value
@@ -1674,9 +1748,39 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
// future if multi-location support is added to the IR, it might be
// preferable to keep tracking both the loaded value and the original
// address in case the alloca can not be elided.
- Instruction *DbgValue = Builder.insertDbgValueIntrinsic(
- LI, DIVar, DIExpr, NewLoc, (Instruction *)nullptr);
- DbgValue->insertAfter(LI);
+ insertDbgValueOrDPValueAfter(Builder, LI, DIVar, DIExpr, NewLoc, LI->getIterator());
+}
+
+void llvm::ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ StoreInst *SI, DIBuilder &Builder) {
+ assert(DPV->isAddressOfVariable());
+ auto *DIVar = DPV->getVariable();
+ assert(DIVar && "Missing variable");
+ auto *DIExpr = DPV->getExpression();
+ Value *DV = SI->getValueOperand();
+
+ DebugLoc NewLoc = getDebugValueLocDPV(DPV, SI);
+
+ if (!valueCoversEntireFragment(DV->getType(), DPV)) {
+ // FIXME: If storing to a part of the variable described by the dbg.declare,
+ // then we want to insert a DPValue.value for the corresponding fragment.
+ LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to DPValue: "
+ << *DPV << '\n');
+ // For now, when there is a store to parts of the variable (but we do not
+ // know which part) we insert an DPValue record to indicate that we know
+ // nothing about the variable's content.
+ DV = UndefValue::get(DV->getType());
+ ValueAsMetadata *DVAM = ValueAsMetadata::get(DV);
+ DPValue *NewDPV = new DPValue(DVAM, DIVar, DIExpr, NewLoc.get());
+ SI->getParent()->insertDPValueBefore(NewDPV, SI->getIterator());
+ return;
+ }
+
+ assert(UseNewDbgInfoFormat);
+ // Create a DPValue directly and insert.
+ ValueAsMetadata *DVAM = ValueAsMetadata::get(DV);
+ DPValue *NewDPV = new DPValue(DVAM, DIVar, DIExpr, NewLoc.get());
+ SI->getParent()->insertDPValueBefore(NewDPV, SI->getIterator());
}
/// Inserts a llvm.dbg.value intrinsic after a phi that has an associated
@@ -1707,8 +1811,38 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
// The block may be a catchswitch block, which does not have a valid
// insertion point.
// FIXME: Insert dbg.value markers in the successors when appropriate.
- if (InsertionPt != BB->end())
- Builder.insertDbgValueIntrinsic(APN, DIVar, DIExpr, NewLoc, &*InsertionPt);
+ if (InsertionPt != BB->end()) {
+ insertDbgValueOrDPValue(Builder, APN, DIVar, DIExpr, NewLoc, InsertionPt);
+ }
+}
+
+void llvm::ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ LoadInst *LI, DIBuilder &Builder) {
+ auto *DIVar = DPV->getVariable();
+ auto *DIExpr = DPV->getExpression();
+ assert(DIVar && "Missing variable");
+
+ if (!valueCoversEntireFragment(LI->getType(), DPV)) {
+ // FIXME: If only referring to a part of the variable described by the
+ // dbg.declare, then we want to insert a DPValue for the corresponding
+ // fragment.
+ LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to DPValue: "
+ << *DPV << '\n');
+ return;
+ }
+
+ DebugLoc NewLoc = getDebugValueLocDPV(DPV, nullptr);
+
+ // We are now tracking the loaded value instead of the address. In the
+ // future if multi-location support is added to the IR, it might be
+ // preferable to keep tracking both the loaded value and the original
+ // address in case the alloca can not be elided.
+ assert(UseNewDbgInfoFormat);
+
+ // Create a DPValue directly and insert.
+ ValueAsMetadata *LIVAM = ValueAsMetadata::get(LI);
+ DPValue *DV = new DPValue(LIVAM, DIVar, DIExpr, NewLoc.get());
+ LI->getParent()->insertDPValueAfter(DV, LI);
}
/// Determine whether this alloca is either a VLA or an array.
@@ -1721,6 +1855,36 @@ static bool isArray(AllocaInst *AI) {
static bool isStructure(AllocaInst *AI) {
return AI->getAllocatedType() && AI->getAllocatedType()->isStructTy();
}
+void llvm::ConvertDebugDeclareToDebugValue(DPValue *DPV,
+ PHINode *APN, DIBuilder &Builder) {
+ auto *DIVar = DPV->getVariable();
+ auto *DIExpr = DPV->getExpression();
+ assert(DIVar && "Missing variable");
+
+ if (PhiHasDebugValue(DIVar, DIExpr, APN))
+ return;
+
+ if (!valueCoversEntireFragment(APN->getType(), DPV)) {
+ // FIXME: If only referring to a part of the variable described by the
+ // dbg.declare, then we want to insert a DPValue for the corresponding
+ // fragment.
+ LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to DPValue: "
+ << *DPV << '\n');
+ return;
+ }
+
+ BasicBlock *BB = APN->getParent();
+ auto InsertionPt = BB->getFirstInsertionPt();
+
+ DebugLoc NewLoc = getDebugValueLocDPV(DPV, nullptr);
+
+ // The block may be a catchswitch block, which does not have a valid
+ // insertion point.
+ // FIXME: Insert DPValue markers in the successors when appropriate.
+ if (InsertionPt != BB->end()) {
+ insertDbgValueOrDPValue(Builder, APN, DIVar, DIExpr, NewLoc, InsertionPt);
+ }
+}
/// LowerDbgDeclare - Lowers llvm.dbg.declare intrinsics into appropriate set
/// of llvm.dbg.value intrinsics.
@@ -1777,8 +1941,7 @@ bool llvm::LowerDbgDeclare(Function &F) {
DebugLoc NewLoc = getDebugValueLoc(DDI);
auto *DerefExpr =
DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
- DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
- NewLoc, CI);
+ insertDbgValueOrDPValue(DIB, AI, DDI->getVariable(), DerefExpr, NewLoc, CI->getIterator());
}
} else if (BitCastInst *BI = dyn_cast<BitCastInst>(U)) {
if (BI->getType()->isPointerTy())
@@ -1878,14 +2041,12 @@ bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
return !DbgDeclares.empty();
}
-static void replaceOneDbgValueForAlloca(DbgValueInst *DVI, Value *NewAddress,
+static void updateOneDbgValueForAlloca(const DebugLoc &Loc, DILocalVariable *DIVar, DIExpression *DIExpr,
+ Value *NewAddress, DbgValueInst *DVI, DPValue *DPV,
DIBuilder &Builder, int Offset) {
- const DebugLoc &Loc = DVI->getDebugLoc();
- auto *DIVar = DVI->getVariable();
- auto *DIExpr = DVI->getExpression();
assert(DIVar && "Missing variable");
- // This is an alloca-based llvm.dbg.value. The first thing it should do with
+ // This is an alloca-based dbg.value/DPValue. The first thing it should do with
// the alloca pointer is dereference it. Otherwise we don't know how to handle
// it and give up.
if (!DIExpr || DIExpr->getNumElements() < 1 ||
@@ -1893,29 +2054,46 @@ static void replaceOneDbgValueForAlloca(DbgValueInst *DVI, Value *NewAddress,
return;
// Insert the offset before the first deref.
- // We could just change the offset argument of dbg.value, but it's unsigned...
if (Offset)
DIExpr = DIExpression::prepend(DIExpr, 0, Offset);
- Builder.insertDbgValueIntrinsic(NewAddress, DIVar, DIExpr, Loc, DVI);
- DVI->eraseFromParent();
+ if (DVI) {
+ DVI->setExpression(DIExpr);
+ DVI->replaceVariableLocationOp(0u, NewAddress);
+ } else {
+ assert(DPV);
+ DPV->setExpression(DIExpr);
+ DPV->replaceVariableLocationOp(0u, NewAddress);
+ }
}
void llvm::replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
DIBuilder &Builder, int Offset) {
- if (auto *L = LocalAsMetadata::getIfExists(AI))
- if (auto *MDV = MetadataAsValue::getIfExists(AI->getContext(), L))
- for (Use &U : llvm::make_early_inc_range(MDV->uses()))
- if (auto *DVI = dyn_cast<DbgValueInst>(U.getUser()))
- replaceOneDbgValueForAlloca(DVI, NewAllocaAddress, Builder, Offset);
+ SmallVector<DbgVariableIntrinsic *, 1> DbgUsers;
+ SmallVector<DPValue *, 1> DPUsers;
+ findDbgUsers(DbgUsers, AI, &DPUsers);
+
+ // Attempt to replace dbg.values that use this alloca.
+ for (auto *DV : DbgUsers) {
+ auto *DVI = dyn_cast<DbgValueInst>(DV);
+ if (!DVI)
+ continue;
+ updateOneDbgValueForAlloca(DVI->getDebugLoc(), DVI->getVariable(), DVI->getExpression(), NewAllocaAddress, DVI, nullptr, Builder, Offset);
+ }
+
+ // Replace any DPValues that use this alloca.
+ for (DPValue *DPV : DPUsers) {
+ updateOneDbgValueForAlloca(DPV->getDebugLoc(), DPV->getVariable(), DPV->getExpression(), NewAllocaAddress, nullptr, DPV, Builder, Offset);
+ }
}
/// Where possible to salvage debug information for \p I do so.
/// If not possible mark undef.
void llvm::salvageDebugInfo(Instruction &I) {
SmallVector<DbgVariableIntrinsic *, 1> DbgUsers;
- findDbgUsers(DbgUsers, &I);
- salvageDebugInfoForDbgValues(I, DbgUsers);
+ SmallVector<DPValue *, 1> DPUsers;
+ findDbgUsers(DbgUsers, &I, &DPUsers);
+ salvageDebugInfoForDbgValues(I, DbgUsers, DPUsers);
}
/// Salvage the address component of \p DAI.
@@ -1953,7 +2131,7 @@ static void salvageDbgAssignAddress(DbgAssignIntrinsic *DAI) {
}
void llvm::salvageDebugInfoForDbgValues(
- Instruction &I, ArrayRef<DbgVariableIntrinsic *> DbgUsers) {
+ Instruction &I, ArrayRef<DbgVariableIntrinsic *> DbgUsers, ArrayRef<DPValue *> DPUsers) {
// These are arbitrary chosen limits on the maximum number of values and the
// maximum size of a debug expression we can salvage up to, used for
// performance reasons.
@@ -2019,12 +2197,68 @@ void llvm::salvageDebugInfoForDbgValues(
LLVM_DEBUG(dbgs() << "SALVAGE: " << *DII << '\n');
Salvaged = true;
}
+ // Duplicate of above block for DPValues.
+ for (auto *DPV : DPUsers) {
+ // Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they
+ // are implicitly pointing out the value as a DWARF memory location
+ // description.
+ bool StackValue = DPV->getType() == DPValue::LocationType::Value;
+ auto DPVLocation = DPV->location_ops();
+ assert(
+ is_contained(DPVLocation, &I) &&
+ "DbgVariableIntrinsic must use salvaged instruction as its location");
+ SmallVector<Value *, 4> AdditionalValues;
+ // 'I' may appear more than once in DPV's location ops, and each use of 'I'
+ // must be updated in the DIExpression and potentially have additional
+ // values added; thus we call salvageDebugInfoImpl for each 'I' instance in
+ // DPVLocation.
+ Value *Op0 = nullptr;
+ DIExpression *SalvagedExpr = DPV->getExpression();
+ auto LocItr = find(DPVLocation, &I);
+ while (SalvagedExpr && LocI...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff c8b11091e832503f5440e9128fbd61ffa9ff86fd 784dfc3e895874845d8e8e21507d1c5e960006dd -- llvm/include/llvm/Transforms/Utils/Local.h llvm/lib/IR/BasicBlock.cpp llvm/lib/Transforms/InstCombine/InstructionCombining.cpp llvm/lib/Transforms/Utils/Local.cppView the diff from clang-format here.diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index db63b74f78..162837c051 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1636,7 +1636,8 @@ static bool valueCoversEntireFragment(Type *ValTy, DPValue *DPV) {
"address of variable must have exactly 1 location operand.");
if (auto *AI =
dyn_cast_or_null<AllocaInst>(DPV->getVariableLocationOp(0))) {
- if (std::optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) {
+ if (std::optional<TypeSize> FragmentSize =
+ AI->getAllocationSizeInBits(DL)) {
return TypeSize::isKnownGE(ValueSize, *FragmentSize);
}
}
|
OCHyams
reviewed
Nov 15, 2023
| @@ -2019,12 +2197,68 @@ void llvm::salvageDebugInfoForDbgValues( | |||
| LLVM_DEBUG(dbgs() << "SALVAGE: " << *DII << '\n'); | |||
| Salvaged = true; | |||
| } | |||
| // Duplicate of above block for DPValues. | |||
| for (auto *DPV : DPUsers) { | |||
There was a problem hiding this comment.
If not for the isa<dbg.assign> check in the original I'd be tempted to say this function might be better templated rather than copy/pasted due to its size. Casts make it ugly to do so, alas. The size of the duplication makes me nervous, but I'm not sure what else we can do without introducing a load of utilities (something like isaDbgValue(DbgVariableIntrinsic *) + isaDbgValue(DPValue *) etc).
* get insertDPValueAfter to create markers * remove redundant param from getDebugValueLocDPV * Have replaceDbgValueForAlloca search for DbgValue only * Switch dropDebugUsers back to the original behaviour
OCHyams
approved these changes
Nov 16, 2023
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this pull request
Nov 27, 2023
That patch was missing a recent change to the non-DPValue StoreInst overload. Add it to the DPValue version. This codepath will be tested after SROA is updated to handle DPValues.
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this pull request
Dec 1, 2023
That patch was missing a recent change to the non-DPValue StoreInst overload. Add it to the DPValue version. This codepath will be tested after SROA is updated to handle DPValues. (cherry picked from commit cbe6239a1827e99ac13733a71152199f5fc9fb8b) Signed-off-by: OCHyams <orlando.hyams@sony.com>
OCHyams
added a commit
that referenced
this pull request
Dec 12, 2023
That patch was missing a recent change to the non-DPValue StoreInst overload. Add it to the DPValue version. This is tested by `llvm/tests/Transforms/Mem2Reg/dbg_declare_to_value_conversions.ll`, which already has `--try-experimental-debuginfo-iterators`. It doesn't fail currently because until #74090 lands the declares are not converted to DPValues. The tests will become "live" once #74090 lands (see for more info).
ichaer
added a commit
to ichaer/llvm-project-onesided_lower_bound
that referenced
this pull request
Jan 29, 2024
* main: (5908 commits) [readtapi] Cleanup printing command line options (llvm#75106) [flang] Fix compilation error due to variable no being used (llvm#75210) [C API] Add getters and setters for fast-math flags on relevant instructions (llvm#75123) [libc++][CI] Tests the no RTTI configuration. (llvm#65518) [RemoveDIs] Fold variable into assert, it's only used once. NFC [RemoveDI] Handle DPValues in SROA (llvm#74089) [AArch64][GlobalISel] Test Pre-Commit for Look into array's element [mlir][tensor] Fix bug in `tensor.extract(tensor.from_elements)` folder (llvm#75109) [analyzer] Move alpha checker EnumCastOutOfRange to optin (llvm#67157) [RemoveDIs] Handle DPValues in replaceDbgDeclare (llvm#73507) [SHT_LLVM_BB_ADDR_MAP] Implements PGOAnalysisMap in Object and ObjectYAML with tests. [X86][GlobalISel] Add instruction selection for G_SELECT (llvm#70753) [AMDGPU] Remove unused function splitScalar64BitAddSub [LLVM][DWARF] Add compilation directory and dwo name to TU in dwo section (llvm#74909) [clang][Interp] Implement __builtin_ffs (llvm#72988) [RemoveDIs] Update ConvertDebugDeclareToDebugValue after llvm#72276 (llvm#73508) [libc][NFC] Reuse `FloatProperties` constant instead of creating new ones (llvm#75187) [RemoveDIs] Fix removeRedundantDdbgInstrs utils for dbg.declares (llvm#74102) Reapply "[RemoveDIs][NFC] Find DPValues using findDbgDeclares (llvm#73500)" [GitHub] Remove author association print from new-prs workflow ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch re-implements a variety of debug-info maintenence functions to use DPValues instead of DbgValueInst's: supporting the "new" non-intrinsic representation of debug-info. As per [0], we need to have parallel implementations of various utilities for a time, and these are the most fundamental utilities used throughout the compiler.
I've added --try-experimental-debuginfo-iterators to a variety of RUN lines: this is a flag that turns on "new debug-info" if it's built into LLVM, and not otherwise. This should ensure that we have the same behaviour for the same IR inputs, but using a different internal representation. For the most part these changes affect SROA/Mem2Reg promotion of dbg.declares into dbg.value intrinsics (now DPValues), we're leaving dbg.declares as instructions until later in the day. There's also some salvaging changes made.
I believe the tests that I've added cover almost all the code being updated here. The only thing I'm not confident about is SimplifyCFG, which calls rewriteDebugUsers down a variety of code paths. Those changes can't immediately get full coverage as an additional patch is needed that updates handling of Unreachable instructions, will upload that shortly.
[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939/9