Skip to content

Commit 8325d46

Browse files
dingxiangfei2009nikic
authored andcommitted
[MergeFuncs] Compare load instruction metadata
MergeFuncs currently merges load instructions with differing semantically-relevant metadata, e.g. a load that has !nonnull with one that does not. Update FunctionComparator to make sure that metadata of both loads is the same. Alternatively, it would be possilbe to ignore the metadata during comparison, and then drop it during merging. Differential Revision: https://reviews.llvm.org/D144682
1 parent d9b3a94 commit 8325d46

File tree

2 files changed

+40
-10
lines changed

2 files changed

+40
-10
lines changed

llvm/include/llvm/Transforms/Utils/FunctionComparator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ class FunctionComparator {
332332
int cmpOrderings(AtomicOrdering L, AtomicOrdering R) const;
333333
int cmpInlineAsm(const InlineAsm *L, const InlineAsm *R) const;
334334
int cmpAttrs(const AttributeList L, const AttributeList R) const;
335-
int cmpRangeMetadata(const MDNode *L, const MDNode *R) const;
335+
int cmpMetadata(const MDNode *L, const MDNode *R) const;
336+
int cmpInstMetadata(Instruction const *L, Instruction const *R) const;
336337
int cmpOperandBundlesSchema(const CallBase &LCS, const CallBase &RCS) const;
337338

338339
/// Compare two GEPs for equivalent pointer arithmetic.

llvm/lib/Transforms/Utils/FunctionComparator.cpp

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,13 @@ int FunctionComparator::cmpAttrs(const AttributeList L,
157157
return 0;
158158
}
159159

160-
int FunctionComparator::cmpRangeMetadata(const MDNode *L,
161-
const MDNode *R) const {
160+
int FunctionComparator::cmpMetadata(const MDNode *L, const MDNode *R) const {
162161
if (L == R)
163162
return 0;
164163
if (!L)
165164
return -1;
166165
if (!R)
167166
return 1;
168-
// Range metadata is a sequence of numbers. Make sure they are the same
169-
// sequence.
170167
// TODO: Note that as this is metadata, it is possible to drop and/or merge
171168
// this data when considering functions to merge. Thus this comparison would
172169
// return 0 (i.e. equivalent), but merging would become more complicated
@@ -176,14 +173,48 @@ int FunctionComparator::cmpRangeMetadata(const MDNode *L,
176173
if (int Res = cmpNumbers(L->getNumOperands(), R->getNumOperands()))
177174
return Res;
178175
for (size_t I = 0; I < L->getNumOperands(); ++I) {
176+
// TODO: the following routine coerce the metadata contents into numbers
177+
// before comparison.
178+
// It ignores any other cases, so that the metadata nodes are considered
179+
// equal even though this is not correct.
180+
// We should structurally compare the metadata nodes to be perfect here.
179181
ConstantInt *LLow = mdconst::extract<ConstantInt>(L->getOperand(I));
180182
ConstantInt *RLow = mdconst::extract<ConstantInt>(R->getOperand(I));
183+
if (LLow == RLow)
184+
continue;
185+
if (!LLow)
186+
return -1;
187+
if (!RLow)
188+
return 1;
181189
if (int Res = cmpAPInts(LLow->getValue(), RLow->getValue()))
182190
return Res;
183191
}
184192
return 0;
185193
}
186194

195+
int FunctionComparator::cmpInstMetadata(Instruction const *L,
196+
Instruction const *R) const {
197+
/// These metadata affects the other optimization passes by making assertions
198+
/// or constraints.
199+
/// Values that carry different expectations should be considered different.
200+
SmallVector<std::pair<unsigned, MDNode *>> MDL, MDR;
201+
L->getAllMetadataOtherThanDebugLoc(MDL);
202+
R->getAllMetadataOtherThanDebugLoc(MDR);
203+
if (MDL.size() > MDR.size())
204+
return 1;
205+
else if (MDL.size() < MDR.size())
206+
return -1;
207+
for (size_t I = 0, N = MDL.size(); I < N; ++I) {
208+
auto const [KeyL, ML] = MDL[I];
209+
auto const [KeyR, MR] = MDR[I];
210+
if (int Res = cmpNumbers(KeyL, KeyR))
211+
return Res;
212+
if (int Res = cmpMetadata(ML, MR))
213+
return Res;
214+
}
215+
return 0;
216+
}
217+
187218
int FunctionComparator::cmpOperandBundlesSchema(const CallBase &LCS,
188219
const CallBase &RCS) const {
189220
assert(LCS.getOpcode() == RCS.getOpcode() && "Can't compare otherwise!");
@@ -586,9 +617,7 @@ int FunctionComparator::cmpOperations(const Instruction *L,
586617
if (int Res = cmpNumbers(LI->getSyncScopeID(),
587618
cast<LoadInst>(R)->getSyncScopeID()))
588619
return Res;
589-
return cmpRangeMetadata(
590-
LI->getMetadata(LLVMContext::MD_range),
591-
cast<LoadInst>(R)->getMetadata(LLVMContext::MD_range));
620+
return cmpInstMetadata(L, R);
592621
}
593622
if (const StoreInst *SI = dyn_cast<StoreInst>(L)) {
594623
if (int Res =
@@ -616,8 +645,8 @@ int FunctionComparator::cmpOperations(const Instruction *L,
616645
if (int Res = cmpNumbers(CI->getTailCallKind(),
617646
cast<CallInst>(R)->getTailCallKind()))
618647
return Res;
619-
return cmpRangeMetadata(L->getMetadata(LLVMContext::MD_range),
620-
R->getMetadata(LLVMContext::MD_range));
648+
return cmpMetadata(L->getMetadata(LLVMContext::MD_range),
649+
R->getMetadata(LLVMContext::MD_range));
621650
}
622651
if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(L)) {
623652
ArrayRef<unsigned> LIndices = IVI->getIndices();

0 commit comments

Comments
 (0)