Skip to content

Commit f08c12f

Browse files
teresajohnsonnikic
authored andcommitted
[DIArgList] Re-unique after changing operands to fix non-determinism
We have a large compile showing occasional non-deterministic behavior that is due to DIArgList not being properly uniqued in some cases. I tracked this down to handleChangedOperands, for which there is a custom implementation for DIArgList, that does not take care of re-uniquing after updating the DIArgList Args, unlike the default version of handleChangedOperands for MDNode. Since the Args in the DIArgList form the key for the store, this seems to be occasionally breaking the lookup in that DenseSet. Specifically, when invoking DIArgList::get() from replaceVariableLocationOp, very occasionally it returns a new DIArgList object, when one already exists having the same exact Args pointers. This in turn causes a subsequent call to Instruction::isIdenticalToWhenDefined on those two otherwise identical DIArgList objects during a later pass to return false, leading to different IR in those rare cases. I modified DIArgList::handleChangedOperands to perform similar re-uniquing as the MDNode version used by other metadata node types. This also necessitated a change to the context destructor, since in some cases we end up with DIArgList as distinct nodes: DIArgList is the only metadata node type to have a custom dropAllReferences, so we need to invoke that version on DIArgList in the DistinctMDNodes store to clean it up properly. Differential Revision: https://reviews.llvm.org/D108968 (cherry picked from commit badcd58)
1 parent e048e97 commit f08c12f

File tree

3 files changed

+19
-1
lines changed

3 files changed

+19
-1
lines changed

llvm/include/llvm/IR/Metadata.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,7 @@ struct TempMDNodeDeleter {
897897
class MDNode : public Metadata {
898898
friend class ReplaceableMetadataImpl;
899899
friend class LLVMContextImpl;
900+
friend class DIArgList;
900901

901902
unsigned NumOperands;
902903
unsigned NumUnresolved;

llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,6 +1592,12 @@ void DIArgList::handleChangedOperand(void *Ref, Metadata *New) {
15921592
assert((!New || isa<ValueAsMetadata>(New)) &&
15931593
"DIArgList must be passed a ValueAsMetadata");
15941594
untrack();
1595+
bool Uniq = isUniqued();
1596+
if (Uniq) {
1597+
// We need to update the uniqueness once the Args are updated since they
1598+
// form the key to the DIArgLists store.
1599+
eraseFromStore();
1600+
}
15951601
ValueAsMetadata *NewVM = cast_or_null<ValueAsMetadata>(New);
15961602
for (ValueAsMetadata *&VM : Args) {
15971603
if (&VM == OldVMPtr) {
@@ -1601,6 +1607,10 @@ void DIArgList::handleChangedOperand(void *Ref, Metadata *New) {
16011607
VM = ValueAsMetadata::get(UndefValue::get(VM->getValue()->getType()));
16021608
}
16031609
}
1610+
if (Uniq) {
1611+
if (uniquify() != this)
1612+
storeDistinctInContext();
1613+
}
16041614
track();
16051615
}
16061616
void DIArgList::track() {

llvm/lib/IR/LLVMContextImpl.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,15 @@ LLVMContextImpl::~LLVMContextImpl() {
5555

5656
// Drop references for MDNodes. Do this before Values get deleted to avoid
5757
// unnecessary RAUW when nodes are still unresolved.
58-
for (auto *I : DistinctMDNodes)
58+
for (auto *I : DistinctMDNodes) {
59+
// We may have DIArgList that were uniqued, and as it has a custom
60+
// implementation of dropAllReferences, it needs to be explicitly invoked.
61+
if (auto *AL = dyn_cast<DIArgList>(I)) {
62+
AL->dropAllReferences();
63+
continue;
64+
}
5965
I->dropAllReferences();
66+
}
6067
#define HANDLE_MDNODE_LEAF_UNIQUABLE(CLASS) \
6168
for (auto *I : CLASS##s) \
6269
I->dropAllReferences();

0 commit comments

Comments
 (0)