Skip to content

Commit 4bc4a6c

Browse files
SLTozerSLTozer
committed
[DebugInfo] Make DIArgList inherit from Metadata and always unique
1 parent f049395 commit 4bc4a6c

13 files changed

+119
-123
lines changed

‎llvm/include/llvm/AsmParser/LLParser.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,7 @@ namespace llvm {
548548
bool parseMetadataAsValue(Value *&V, PerFunctionState &PFS);
549549
bool parseValueAsMetadata(Metadata *&MD, const Twine &TypeMsg,
550550
PerFunctionState *PFS);
551+
bool parseDIArgList(Metadata *&MD, PerFunctionState *PFS);
551552
bool parseMetadata(Metadata *&MD, PerFunctionState *PFS);
552553
bool parseMDTuple(MDNode *&MD, bool IsDistinct = false);
553554
bool parseMDNode(MDNode *&N);
@@ -569,8 +570,6 @@ namespace llvm {
569570
#define HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS) \
570571
bool parse##CLASS(MDNode *&Result, bool IsDistinct);
571572
#include "llvm/IR/Metadata.def"
572-
bool parseDIArgList(MDNode *&Result, bool IsDistinct,
573-
PerFunctionState *PFS);
574573

575574
// Function Parsing.
576575
struct ArgInfo {

‎llvm/include/llvm/IR/DebugInfoMetadata.h

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3753,51 +3753,40 @@ class DIMacroFile : public DIMacroNode {
37533753

37543754
/// List of ValueAsMetadata, to be used as an argument to a dbg.value
37553755
/// intrinsic.
3756-
class DIArgList : public MDNode {
3756+
class DIArgList : public Metadata, ReplaceableMetadataImpl {
3757+
friend class ReplaceableMetadataImpl;
37573758
friend class LLVMContextImpl;
3758-
friend class MDNode;
37593759
using iterator = SmallVectorImpl<ValueAsMetadata *>::iterator;
37603760

37613761
SmallVector<ValueAsMetadata *, 4> Args;
37623762

3763-
DIArgList(LLVMContext &C, StorageType Storage,
3764-
ArrayRef<ValueAsMetadata *> Args)
3765-
: MDNode(C, DIArgListKind, Storage, std::nullopt),
3763+
DIArgList(LLVMContext &Context, ArrayRef<ValueAsMetadata *> Args)
3764+
: Metadata(DIArgListKind, Uniqued), ReplaceableMetadataImpl(Context),
37663765
Args(Args.begin(), Args.end()) {
37673766
track();
37683767
}
37693768
~DIArgList() { untrack(); }
37703769

3771-
static DIArgList *getImpl(LLVMContext &Context,
3772-
ArrayRef<ValueAsMetadata *> Args,
3773-
StorageType Storage, bool ShouldCreate = true);
3774-
3775-
TempDIArgList cloneImpl() const {
3776-
return getTemporary(getContext(), getArgs());
3777-
}
3778-
37793770
void track();
37803771
void untrack();
3781-
void dropAllReferences();
3772+
void dropAllReferences(bool Untrack);
37823773

37833774
public:
3784-
DEFINE_MDNODE_GET(DIArgList, (ArrayRef<ValueAsMetadata *> Args), (Args))
3785-
3786-
TempDIArgList clone() const { return cloneImpl(); }
3775+
static DIArgList *get(LLVMContext &Context, ArrayRef<ValueAsMetadata *> Args);
37873776

37883777
ArrayRef<ValueAsMetadata *> getArgs() const { return Args; }
37893778

37903779
iterator args_begin() { return Args.begin(); }
37913780
iterator args_end() { return Args.end(); }
37923781

3793-
ReplaceableMetadataImpl *getReplaceableUses() {
3794-
return Context.getReplaceableUses();
3795-
}
3796-
37973782
static bool classof(const Metadata *MD) {
37983783
return MD->getMetadataID() == DIArgListKind;
37993784
}
38003785

3786+
SmallVector<DPValue *> getAllDPValueUsers() {
3787+
return ReplaceableMetadataImpl::getAllDPValueUsers();
3788+
}
3789+
38013790
void handleChangedOperand(void *Ref, Metadata *New);
38023791
};
38033792

‎llvm/include/llvm/IR/Metadata.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ HANDLE_METADATA_BRANCH(ValueAsMetadata)
7777
HANDLE_METADATA_LEAF(ConstantAsMetadata)
7878
HANDLE_METADATA_LEAF(LocalAsMetadata)
7979
HANDLE_METADATA_LEAF(DistinctMDOperandPlaceholder)
80+
HANDLE_METADATA_LEAF(DIArgList)
8081
HANDLE_MDNODE_BRANCH(MDNode)
8182
HANDLE_MDNODE_LEAF_UNIQUABLE(MDTuple)
8283
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DILocation)
@@ -115,7 +116,6 @@ HANDLE_SPECIALIZED_MDNODE_BRANCH(DIMacroNode)
115116
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIMacro)
116117
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIMacroFile)
117118
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DICommonBlock)
118-
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIArgList)
119119
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIStringType)
120120
HANDLE_SPECIALIZED_MDNODE_LEAF_UNIQUABLE(DIGenericSubrange)
121121

‎llvm/include/llvm/IR/Metadata.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,7 +1037,6 @@ struct TempMDNodeDeleter {
10371037
class MDNode : public Metadata {
10381038
friend class ReplaceableMetadataImpl;
10391039
friend class LLVMContextImpl;
1040-
friend class DIArgList;
10411040

10421041
/// The header that is coallocated with an MDNode along with its "small"
10431042
/// operands. It is located immediately before the main body of the node.
@@ -1220,9 +1219,7 @@ class MDNode : public Metadata {
12201219
bool isDistinct() const { return Storage == Distinct; }
12211220
bool isTemporary() const { return Storage == Temporary; }
12221221

1223-
bool isReplaceable() const {
1224-
return isTemporary() || getMetadataID() == DIArgListKind;
1225-
}
1222+
bool isReplaceable() const { return isTemporary(); }
12261223

12271224
/// RAUW a temporary.
12281225
///

‎llvm/lib/AsmParser/LLParser.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5486,13 +5486,9 @@ bool LLParser::parseDIExpression(MDNode *&Result, bool IsDistinct) {
54865486
return false;
54875487
}
54885488

5489-
bool LLParser::parseDIArgList(MDNode *&Result, bool IsDistinct) {
5490-
return parseDIArgList(Result, IsDistinct, nullptr);
5491-
}
54925489
/// ParseDIArgList:
54935490
/// ::= !DIArgList(i32 7, i64 %0)
5494-
bool LLParser::parseDIArgList(MDNode *&Result, bool IsDistinct,
5495-
PerFunctionState *PFS) {
5491+
bool LLParser::parseDIArgList(Metadata *&MD, PerFunctionState *PFS) {
54965492
assert(PFS && "Expected valid function state");
54975493
assert(Lex.getKind() == lltok::MetadataVar && "Expected metadata type name");
54985494
Lex.Lex();
@@ -5512,7 +5508,7 @@ bool LLParser::parseDIArgList(MDNode *&Result, bool IsDistinct,
55125508
if (parseToken(lltok::rparen, "expected ')' here"))
55135509
return true;
55145510

5515-
Result = GET_OR_DISTINCT(DIArgList, (Context, Args));
5511+
MD = DIArgList::get(Context, Args);
55165512
return false;
55175513
}
55185514

@@ -5626,13 +5622,17 @@ bool LLParser::parseValueAsMetadata(Metadata *&MD, const Twine &TypeMsg,
56265622
/// ::= !DILocation(...)
56275623
bool LLParser::parseMetadata(Metadata *&MD, PerFunctionState *PFS) {
56285624
if (Lex.getKind() == lltok::MetadataVar) {
5629-
MDNode *N;
56305625
// DIArgLists are a special case, as they are a list of ValueAsMetadata and
56315626
// so parsing this requires a Function State.
56325627
if (Lex.getStrVal() == "DIArgList") {
5633-
if (parseDIArgList(N, false, PFS))
5628+
Metadata *AL;
5629+
if (parseDIArgList(AL, PFS))
56345630
return true;
5635-
} else if (parseSpecializedMDNode(N)) {
5631+
MD = AL;
5632+
return false;
5633+
}
5634+
MDNode *N;
5635+
if (parseSpecializedMDNode(N)) {
56365636
return true;
56375637
}
56385638
MD = N;

‎llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,7 @@ class ModuleBitcodeWriter : public ModuleBitcodeWriterBase {
336336
unsigned Abbrev);
337337
void writeDIMacroFile(const DIMacroFile *N, SmallVectorImpl<uint64_t> &Record,
338338
unsigned Abbrev);
339-
void writeDIArgList(const DIArgList *N, SmallVectorImpl<uint64_t> &Record,
340-
unsigned Abbrev);
339+
void writeDIArgList(const DIArgList *N, SmallVectorImpl<uint64_t> &Record);
341340
void writeDIModule(const DIModule *N, SmallVectorImpl<uint64_t> &Record,
342341
unsigned Abbrev);
343342
void writeDIAssignID(const DIAssignID *N, SmallVectorImpl<uint64_t> &Record,
@@ -1975,13 +1974,12 @@ void ModuleBitcodeWriter::writeDIMacroFile(const DIMacroFile *N,
19751974
}
19761975

19771976
void ModuleBitcodeWriter::writeDIArgList(const DIArgList *N,
1978-
SmallVectorImpl<uint64_t> &Record,
1979-
unsigned Abbrev) {
1977+
SmallVectorImpl<uint64_t> &Record) {
19801978
Record.reserve(N->getArgs().size());
19811979
for (ValueAsMetadata *MD : N->getArgs())
19821980
Record.push_back(VE.getMetadataID(MD));
19831981

1984-
Stream.EmitRecord(bitc::METADATA_ARG_LIST, Record, Abbrev);
1982+
Stream.EmitRecord(bitc::METADATA_ARG_LIST, Record);
19851983
Record.clear();
19861984
}
19871985

@@ -2264,6 +2262,10 @@ void ModuleBitcodeWriter::writeMetadataRecords(
22642262
#include "llvm/IR/Metadata.def"
22652263
}
22662264
}
2265+
if (auto *AL = dyn_cast<DIArgList>(MD)) {
2266+
writeDIArgList(AL, Record);
2267+
continue;
2268+
}
22672269
writeValueAsMetadata(cast<ValueAsMetadata>(MD), Record);
22682270
}
22692271
}

‎llvm/lib/IR/AsmWriter.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,9 +1265,8 @@ void SlotTracker::CreateFunctionSlot(const Value *V) {
12651265
void SlotTracker::CreateMetadataSlot(const MDNode *N) {
12661266
assert(N && "Can't insert a null Value into SlotTracker!");
12671267

1268-
// Don't make slots for DIExpressions or DIArgLists. We just print them inline
1269-
// everywhere.
1270-
if (isa<DIExpression>(N) || isa<DIArgList>(N))
1268+
// Don't make slots for DIExpressions. We just print them inline everywhere.
1269+
if (isa<DIExpression>(N))
12711270
return;
12721271

12731272
unsigned DestSlot = mdnNext;
@@ -3516,8 +3515,6 @@ void AssemblyWriter::printNamedMDNode(const NamedMDNode *NMD) {
35163515
// Write DIExpressions inline.
35173516
// FIXME: Ban DIExpressions in NamedMDNodes, they will serve no purpose.
35183517
MDNode *Op = NMD->getOperand(i);
3519-
assert(!isa<DIArgList>(Op) &&
3520-
"DIArgLists should not appear in NamedMDNodes");
35213518
if (auto *Expr = dyn_cast<DIExpression>(Op)) {
35223519
writeDIExpression(Out, Expr, AsmWriterContext::getEmpty());
35233520
continue;
@@ -4918,7 +4915,7 @@ static void printMetadataImplRec(raw_ostream &ROS, const Metadata &MD,
49184915
WriteAsOperandInternal(OS, &MD, WriterCtx, /* FromValue */ true);
49194916

49204917
auto *N = dyn_cast<MDNode>(&MD);
4921-
if (!N || isa<DIExpression>(MD) || isa<DIArgList>(MD))
4918+
if (!N || isa<DIExpression>(MD))
49224919
return;
49234920

49244921
OS << " = ";
@@ -4986,7 +4983,7 @@ static void printMetadataImpl(raw_ostream &ROS, const Metadata &MD,
49864983
WriteAsOperandInternal(OS, &MD, *WriterCtx, /* FromValue */ true);
49874984

49884985
auto *N = dyn_cast<MDNode>(&MD);
4989-
if (OnlyAsOperand || !N || isa<DIExpression>(MD) || isa<DIArgList>(MD))
4986+
if (OnlyAsOperand || !N || isa<DIExpression>(MD))
49904987
return;
49914988

49924989
OS << " = ";

‎llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 27 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2115,24 +2115,24 @@ DIMacroFile *DIMacroFile::getImpl(LLVMContext &Context, unsigned MIType,
21152115
DEFINE_GETIMPL_STORE(DIMacroFile, (MIType, Line), Ops);
21162116
}
21172117

2118-
DIArgList *DIArgList::getImpl(LLVMContext &Context,
2119-
ArrayRef<ValueAsMetadata *> Args,
2120-
StorageType Storage, bool ShouldCreate) {
2121-
DEFINE_GETIMPL_LOOKUP(DIArgList, (Args));
2122-
DEFINE_GETIMPL_STORE_NO_OPS(DIArgList, (Args));
2118+
DIArgList *DIArgList::get(LLVMContext &Context,
2119+
ArrayRef<ValueAsMetadata *> Args) {
2120+
auto ExistingIt = Context.pImpl->DIArgLists.find_as(DIArgListKeyInfo(Args));
2121+
if (ExistingIt != Context.pImpl->DIArgLists.end())
2122+
return *ExistingIt;
2123+
DIArgList *NewArgList = new DIArgList(Context, Args);
2124+
Context.pImpl->DIArgLists.insert(NewArgList);
2125+
return NewArgList;
21232126
}
21242127

21252128
void DIArgList::handleChangedOperand(void *Ref, Metadata *New) {
21262129
ValueAsMetadata **OldVMPtr = static_cast<ValueAsMetadata **>(Ref);
21272130
assert((!New || isa<ValueAsMetadata>(New)) &&
21282131
"DIArgList must be passed a ValueAsMetadata");
21292132
untrack();
2130-
bool Uniq = isUniqued();
2131-
if (Uniq) {
2132-
// We need to update the uniqueness once the Args are updated since they
2133-
// form the key to the DIArgLists store.
2134-
eraseFromStore();
2135-
}
2133+
// We need to update the set storage once the Args are updated since they
2134+
// form the key to the DIArgLists store.
2135+
getContext().pImpl->DIArgLists.erase(this);
21362136
ValueAsMetadata *NewVM = cast_or_null<ValueAsMetadata>(New);
21372137
for (ValueAsMetadata *&VM : Args) {
21382138
if (&VM == OldVMPtr) {
@@ -2142,28 +2142,19 @@ void DIArgList::handleChangedOperand(void *Ref, Metadata *New) {
21422142
VM = ValueAsMetadata::get(PoisonValue::get(VM->getValue()->getType()));
21432143
}
21442144
}
2145-
if (Uniq) {
2146-
// In the RemoveDIs project (eliminating debug-info-intrinsics), DIArgLists
2147-
// can be referred to by DebugValueUser objects, which necessitates them
2148-
// being unique and replaceable metadata. This causes a slight
2149-
// performance regression that's to be avoided during the early stages of
2150-
// the RemoveDIs prototype, see D154080.
2151-
#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
2152-
MDNode *UniqueArgList = uniquify();
2153-
if (UniqueArgList != this) {
2154-
replaceAllUsesWith(UniqueArgList);
2155-
// Clear this here so we don't try to untrack in the destructor.
2156-
Args.clear();
2157-
delete this;
2158-
return;
2159-
}
2160-
#else
2161-
// Otherwise, don't fully unique, become distinct instead. See D108968,
2162-
// there's a latent bug that presents here as nondeterminism otherwise.
2163-
if (uniquify() != this)
2164-
storeDistinctInContext();
2165-
#endif
2145+
// We've changed the contents of this DIArgList, and the set storage may
2146+
// already contain a DIArgList with our new set of args; if it does, then we
2147+
// must RAUW this with the existing DIArgList, otherwise we simply insert this
2148+
// back into the set storage.
2149+
DIArgList *ExistingArgList = getUniqued(getContext().pImpl->DIArgLists, this);
2150+
if (ExistingArgList) {
2151+
replaceAllUsesWith(ExistingArgList);
2152+
// Clear this here so we don't try to untrack in the destructor.
2153+
Args.clear();
2154+
delete this;
2155+
return;
21662156
}
2157+
getContext().pImpl->DIArgLists.insert(this);
21672158
track();
21682159
}
21692160
void DIArgList::track() {
@@ -2176,8 +2167,9 @@ void DIArgList::untrack() {
21762167
if (VAM)
21772168
MetadataTracking::untrack(&VAM, *VAM);
21782169
}
2179-
void DIArgList::dropAllReferences() {
2180-
untrack();
2170+
void DIArgList::dropAllReferences(bool Untrack) {
2171+
if (Untrack)
2172+
untrack();
21812173
Args.clear();
2182-
MDNode::dropAllReferences();
2174+
ReplaceableMetadataImpl::resolveAllUses(/* ResolveUsers */ false);
21832175
}

‎llvm/lib/IR/LLVMContextImpl.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,8 @@ LLVMContextImpl::~LLVMContextImpl() {
6868

6969
// Drop references for MDNodes. Do this before Values get deleted to avoid
7070
// unnecessary RAUW when nodes are still unresolved.
71-
for (auto *I : DistinctMDNodes) {
72-
// We may have DIArgList that were uniqued, and as it has a custom
73-
// implementation of dropAllReferences, it needs to be explicitly invoked.
74-
if (auto *AL = dyn_cast<DIArgList>(I)) {
75-
AL->dropAllReferences();
76-
continue;
77-
}
71+
for (auto *I : DistinctMDNodes)
7872
I->dropAllReferences();
79-
}
8073
#define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS) \
8174
for (auto *I : CLASS##s) \
8275
I->dropAllReferences();
@@ -87,6 +80,10 @@ LLVMContextImpl::~LLVMContextImpl() {
8780
Pair.second->dropUsers();
8881
for (auto &Pair : MetadataAsValues)
8982
Pair.second->dropUse();
83+
// Do not untrack ValueAsMetadata references for DIArgLists, as they have
84+
// already been more efficiently untracked above.
85+
for (DIArgList *AL : DIArgLists)
86+
AL->dropAllReferences(/* Untrack */ false);
9087

9188
// Destroy MDNodes.
9289
for (MDNode *I : DistinctMDNodes)

0 commit comments

Comments
 (0)