Skip to content

[DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values in findDbgValues #71952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values in findDbgValues #71952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47b21d0
[DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values
jmorse Jun 7, 2023
e37fe1f
Temporary fixes to make DIArgLists... temporary
jmorse Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions llvm/include/llvm/IR/DebugInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace llvm {
class DbgDeclareInst;
class DbgValueInst;
class DbgVariableIntrinsic;
class DPValue;
class Instruction;
class Module;

Expand All @@ -42,10 +43,12 @@ class Module;
TinyPtrVector<DbgDeclareInst *> FindDbgDeclareUses(Value *V);

/// Finds the llvm.dbg.value intrinsics describing a value.
void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V);
void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
Value *V, SmallVectorImpl<DPValue *> *DPValues = nullptr);

/// Finds the debug info intrinsics describing a value.
void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts, Value *V);
void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts,
Value *V, SmallVectorImpl<DPValue *> *DPValues = nullptr);

/// Find subprogram that is enclosing this scope.
DISubprogram *getDISubprogram(const MDNode *Scope);
Expand Down
38 changes: 31 additions & 7 deletions llvm/lib/IR/DebugInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/DebugProgramInstruction.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GVMaterializer.h"
#include "llvm/IR/Instruction.h"
Expand Down Expand Up @@ -65,7 +66,8 @@ TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(Value *V) {
}

template <typename IntrinsicT>
static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V) {
static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
Value *V, SmallVectorImpl<DPValue *> *DPValues) {
// This function is hot. Check whether the value has any metadata to avoid a
// DenseMap lookup.
if (!V->isUsedByMetadata())
Expand All @@ -78,31 +80,53 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V) {
// V will also appear twice in a dbg.assign if its used in the both the value
// and address components.
SmallPtrSet<IntrinsicT *, 4> EncounteredIntrinsics;
SmallPtrSet<DPValue *, 4> EncounteredDPValues;

/// Append IntrinsicT users of MetadataAsValue(MD).
auto AppendUsers = [&Ctx, &EncounteredIntrinsics, &Result](Metadata *MD) {
auto AppendUsers = [&Ctx, &EncounteredIntrinsics, &Result,
DPValues](Metadata *MD) {
if (auto *MDV = MetadataAsValue::getIfExists(Ctx, MD)) {
for (User *U : MDV->users())
if (IntrinsicT *DVI = dyn_cast<IntrinsicT>(U))
if (EncounteredIntrinsics.insert(DVI).second)
Result.push_back(DVI);
}
if (!DPValues)
return;
// Get DPValues that use this as a single value.
if (LocalAsMetadata *L = dyn_cast<LocalAsMetadata>(MD)) {
for (DPValue *DPV : L->getAllDPValueUsers()) {
if (DPV->getType() == DPValue::LocationType::Value)
DPValues->push_back(DPV);
}
}
};

if (auto *L = LocalAsMetadata::getIfExists(V)) {
AppendUsers(L);
for (Metadata *AL : L->getAllArgListUsers())
for (Metadata *AL : L->getAllArgListUsers()) {
AppendUsers(AL);
if (!DPValues)
continue;
DIArgList *DI = cast<DIArgList>(AL);
if (auto *Replaceable = DI->getReplaceableUses()) {
for (DPValue *DPV : Replaceable->getAllDPValueUsers())
if (DPV->getType() == DPValue::LocationType::Value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a template parameter for intrinsic type but not for DPValue type. What do you think of changing the signature to:

template <typename IntrinsicT, bool AnyDPVType, DPValue::LocationType SpecificDPVType = DPValue::LocationType(-1)>
static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
                              Value *V, SmallVectorImpl<DPValue *> *DPValues)

Filter DPValeus by SpecificDPVType unless AnyDPVType == true.

wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hhhrrmmm. This is awkward because the intrinsics have an inherent hierarchy due to the instruction hierachy, i.e. DbgInfoIntrinsic -> DbgValueInst -> DbgAssignInst. We haven't yet created that for DPValues because we're focusing only on dbg.value replacement for the moment... but conceptually we'll end up replicating that hierarchy in the future. I'd prefer to wait until we implement that as it should line up with the instruction hierarchy perfectly (TM).

if (EncounteredDPValues.insert(DPV).second)
DPValues->push_back(DPV);
}
}
}
}

void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
findDbgIntrinsics<DbgValueInst>(DbgValues, V);
void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
Value *V, SmallVectorImpl<DPValue *> *DPValues) {
findDbgIntrinsics<DbgValueInst>(DbgValues, V, DPValues);
}

void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
Value *V) {
findDbgIntrinsics<DbgVariableIntrinsic>(DbgUsers, V);
Value *V, SmallVectorImpl<DPValue *> *DPValues) {
findDbgIntrinsics<DbgVariableIntrinsic>(DbgUsers, V, DPValues);
}

DISubprogram *llvm::getDISubprogram(const MDNode *Scope) {
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/IR/DebugInfoMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2110,6 +2110,8 @@ DIMacroFile *DIMacroFile::getImpl(LLVMContext &Context, unsigned MIType,
DIArgList *DIArgList::getImpl(LLVMContext &Context,
ArrayRef<ValueAsMetadata *> Args,
StorageType Storage, bool ShouldCreate) {
if (Storage == Uniqued)
Storage = Temporary;
DEFINE_GETIMPL_LOOKUP(DIArgList, (Args));
DEFINE_GETIMPL_STORE_NO_OPS(DIArgList, (Args));
}
Expand Down
8 changes: 7 additions & 1 deletion llvm/lib/IR/Value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,11 +574,17 @@ void Value::replaceUsesWithIf(Value *New,
/// with New.
static void replaceDbgUsesOutsideBlock(Value *V, Value *New, BasicBlock *BB) {
SmallVector<DbgVariableIntrinsic *> DbgUsers;
findDbgUsers(DbgUsers, V);
SmallVector<DPValue *> DPUsers;
findDbgUsers(DbgUsers, V, &DPUsers);
for (auto *DVI : DbgUsers) {
if (DVI->getParent() != BB)
DVI->replaceVariableLocationOp(V, New);
}
for (auto *DPV : DPUsers) {
DPMarker *Marker = DPV->getMarker();
if (Marker->getParent() != BB)
DPV->replaceVariableLocationOp(V, New);
}
}

// Like replaceAllUsesWith except it does not handle constants or basic blocks.
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,9 @@ void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
}

// Check these last, so we diagnose problems in operands first.
if (isa<DIArgList>(&MD))
// These remain tracked throughout compilation.
return;
Check(!MD.isTemporary(), "Expected no forward declarations!", &MD);
Check(MD.isResolved(), "All nodes should be resolved!", &MD);
}
Expand Down
48 changes: 48 additions & 0 deletions llvm/unittests/IR/DebugInfoTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,54 @@ TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) {
EXPECT_TRUE(DbgDeclare->isKillLocation());
}

// Duplicate of above test, but in DPValue representation.
TEST(MetadataTest, DeleteInstUsedByDPValue) {
LLVMContext C;
std::unique_ptr<Module> M = parseIR(C, R"(
define i16 @f(i16 %a) !dbg !6 {
%b = add i16 %a, 1, !dbg !11
call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
call void @llvm.dbg.value(metadata !DIArgList(i16 %a, i16 %b), metadata !9, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg !11
ret i16 0, !dbg !11
}
declare void @llvm.dbg.value(metadata, metadata, metadata) #0
attributes #0 = { nounwind readnone speculatable willreturn }

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!5}

!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: "t.ll", directory: "/")
!2 = !{}
!5 = !{i32 2, !"Debug Info Version", i32 3}
!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
!7 = !DISubroutineType(types: !2)
!8 = !{!9}
!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
!10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
!11 = !DILocation(line: 1, column: 1, scope: !6)
)");

bool OldDbgValueMode = UseNewDbgInfoFormat;
UseNewDbgInfoFormat = true;
Instruction &I = *M->getFunction("f")->getEntryBlock().getFirstNonPHI();
M->convertToNewDbgValues();

// Find the DPValues using %b.
SmallVector<DbgValueInst *, 2> DVIs;
SmallVector<DPValue *, 2> DPVs;
findDbgValues(DVIs, &I, &DPVs);
ASSERT_EQ(DPVs.size(), 2u);

// Delete %b. The DPValue should now point to undef.
I.eraseFromParent();
EXPECT_EQ(DPVs[0]->getNumVariableLocationOps(), 1u);
EXPECT_TRUE(isa<UndefValue>(DPVs[0]->getVariableLocationOp(0)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth also checking DPV[x]->isKillLocation() for completeness?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

EXPECT_EQ(DPVs[1]->getNumVariableLocationOps(), 2u);
EXPECT_TRUE(isa<UndefValue>(DPVs[1]->getVariableLocationOp(1)));
UseNewDbgInfoFormat = OldDbgValueMode;
}

TEST(DIBuiler, CreateFile) {
LLVMContext Ctx;
std::unique_ptr<Module> M(new Module("MyModule", Ctx));
Expand Down
76 changes: 76 additions & 0 deletions llvm/unittests/IR/ValueTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "gtest/gtest.h"
using namespace llvm;

extern cl::opt<bool> UseNewDbgInfoFormat;

namespace {

TEST(ValueTest, UsedInBasicBlock) {
Expand Down Expand Up @@ -314,4 +316,78 @@ TEST(ValueTest, replaceUsesOutsideBlock) {
ASSERT_TRUE(ExitDbg->getValue(0) == cast<Value>(B));
ASSERT_TRUE(Ret->getOperand(0) == cast<Value>(B));
}

TEST(ValueTest, replaceUsesOutsideBlockDPValue) {
// Check that Value::replaceUsesOutsideBlock(New, BB) replaces uses outside
// BB, including DPValues.
const auto *IR = R"(
define i32 @f() !dbg !6 {
entry:
%a = add i32 0, 1, !dbg !15
%b = add i32 0, 2, !dbg !15
%c = add i32 %a, 2, !dbg !15
call void @llvm.dbg.value(metadata i32 %a, metadata !9, metadata !DIExpression()), !dbg !15
br label %exit, !dbg !15

exit:
call void @llvm.dbg.value(metadata i32 %a, metadata !11, metadata !DIExpression()), !dbg !16
ret i32 %a, !dbg !16
}

declare void @llvm.dbg.value(metadata, metadata, metadata)

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!5}

!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: "test.ll", directory: "/")
!2 = !{}
!5 = !{i32 2, !"Debug Info Version", i32 3}
!6 = distinct !DISubprogram(name: "f", linkageName: "f", scope: null, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !8)
!7 = !DISubroutineType(types: !2)
!8 = !{!9, !11}
!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_signed)
!11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 2, type: !12)
!12 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_signed)
!15 = !DILocation(line: 1, column: 1, scope: !6)
!16 = !DILocation(line: 5, column: 1, scope: !6)
)";
LLVMContext Ctx;
SMDiagnostic Err;
std::unique_ptr<Module> M = parseAssemblyString(IR, Err, Ctx);
if (!M)
Err.print("ValueTest", errs());

bool OldDbgValueMode = UseNewDbgInfoFormat;
UseNewDbgInfoFormat = true;
M->convertToNewDbgValues();

auto GetNext = [](auto *I) { return &*++I->getIterator(); };

Function *F = M->getFunction("f");
// Entry.
BasicBlock *Entry = &F->front();
Instruction *A = &Entry->front();
Instruction *B = GetNext(A);
Instruction *C = GetNext(B);
Instruction *Branch = GetNext(C);
// Exit.
BasicBlock *Exit = GetNext(Entry);
Instruction *Ret = &Exit->front();

EXPECT_TRUE(Branch->hasDbgValues());
EXPECT_TRUE(Ret->hasDbgValues());

DPValue *DPV1 = &*Branch->getDbgValueRange().begin();
DPValue *DPV2 = &*Ret->getDbgValueRange().begin();

A->replaceUsesOutsideBlock(B, Entry);
// These users are in Entry so shouldn't be changed.
EXPECT_TRUE(DPV1->getVariableLocationOp(0) == cast<Value>(A));
// These users are outside Entry so should be changed.
EXPECT_TRUE(DPV2->getVariableLocationOp(0) == cast<Value>(B));
UseNewDbgInfoFormat = OldDbgValueMode;
}

} // end anonymous namespace