[llvm] f5832ea - [UseListOrder] Fix use list order for function operands
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 7 11:59:21 PDT 2021
Author: Nikita Popov
Date: 2021-09-07T20:59:12+02:00
New Revision: f5832eaaadc5382a4385e63d25f5ce08bc7a516c
URL: https://github.com/llvm/llvm-project/commit/f5832eaaadc5382a4385e63d25f5ce08bc7a516c
DIFF: https://github.com/llvm/llvm-project/commit/f5832eaaadc5382a4385e63d25f5ce08bc7a516c.diff
LOG: [UseListOrder] Fix use list order for function operands
Functions can have a personality function, as well as prefix and
prologue data as additional operands. Unused operands are assigned
a dummy value of i1* null. This patch addresses multiple issues in
use-list order preservation for these:
* Fix verify-uselistorder to also enumerate the dummy values.
This means that now use-list order values of these values are
shuffled even if there is no other mention of i1* null in the
module. This results in failures of Assembler/call-arg-is-callee.ll,
Assembler/opaque-ptr.ll and Bitcode/use-list-order2.ll.
* The use-list order prediction in ValueEnumerator does not take
into account the fact that a global may use a value more than
once and leaves uses in the same global effectively unordered.
We should be comparing the operand number here, as we do for
the more general case.
* While we enumerate all operands of a function together (which
seems sensible to me), the bitcode reader would first resolve
prefix data for all function, then prologue data for all
functions, then personality functions for all functions. Change
this to resolve all operands for a given function together
instead.
Differential Revision: https://reviews.llvm.org/D109282
Added:
llvm/test/Assembler/function-operand-uselistorder.ll
Modified:
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
llvm/tools/verify-uselistorder/verify-uselistorder.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 49ba562d8ab7b..543cb9afce805 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -501,9 +501,14 @@ class BitcodeReader : public BitcodeReaderBase, public GVMaterializer {
std::vector<std::pair<GlobalVariable *, unsigned>> GlobalInits;
std::vector<std::pair<GlobalIndirectSymbol *, unsigned>> IndirectSymbolInits;
- std::vector<std::pair<Function *, unsigned>> FunctionPrefixes;
- std::vector<std::pair<Function *, unsigned>> FunctionPrologues;
- std::vector<std::pair<Function *, unsigned>> FunctionPersonalityFns;
+
+ struct FunctionOperandInfo {
+ Function *F;
+ unsigned PersonalityFn;
+ unsigned Prefix;
+ unsigned Prologue;
+ };
+ std::vector<FunctionOperandInfo> FunctionOperands;
/// The set of attributes by index. Index zero in the file is for null, and
/// is thus not represented here. As such all indices are off by one.
@@ -2244,15 +2249,11 @@ Error BitcodeReader::resolveGlobalAndIndirectSymbolInits() {
std::vector<std::pair<GlobalVariable *, unsigned>> GlobalInitWorklist;
std::vector<std::pair<GlobalIndirectSymbol *, unsigned>>
IndirectSymbolInitWorklist;
- std::vector<std::pair<Function *, unsigned>> FunctionPrefixWorklist;
- std::vector<std::pair<Function *, unsigned>> FunctionPrologueWorklist;
- std::vector<std::pair<Function *, unsigned>> FunctionPersonalityFnWorklist;
+ std::vector<FunctionOperandInfo> FunctionOperandWorklist;
GlobalInitWorklist.swap(GlobalInits);
IndirectSymbolInitWorklist.swap(IndirectSymbolInits);
- FunctionPrefixWorklist.swap(FunctionPrefixes);
- FunctionPrologueWorklist.swap(FunctionPrologues);
- FunctionPersonalityFnWorklist.swap(FunctionPersonalityFns);
+ FunctionOperandWorklist.swap(FunctionOperands);
while (!GlobalInitWorklist.empty()) {
unsigned ValID = GlobalInitWorklist.back().second;
@@ -2284,43 +2285,41 @@ Error BitcodeReader::resolveGlobalAndIndirectSymbolInits() {
IndirectSymbolInitWorklist.pop_back();
}
- while (!FunctionPrefixWorklist.empty()) {
- unsigned ValID = FunctionPrefixWorklist.back().second;
- if (ValID >= ValueList.size()) {
- FunctionPrefixes.push_back(FunctionPrefixWorklist.back());
- } else {
- if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
- FunctionPrefixWorklist.back().first->setPrefixData(C);
- else
- return error("Expected a constant");
+ while (!FunctionOperandWorklist.empty()) {
+ FunctionOperandInfo &Info = FunctionOperandWorklist.back();
+ if (Info.PersonalityFn) {
+ unsigned ValID = Info.PersonalityFn - 1;
+ if (ValID < ValueList.size()) {
+ if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
+ Info.F->setPersonalityFn(C);
+ else
+ return error("Expected a constant");
+ Info.PersonalityFn = 0;
+ }
}
- FunctionPrefixWorklist.pop_back();
- }
-
- while (!FunctionPrologueWorklist.empty()) {
- unsigned ValID = FunctionPrologueWorklist.back().second;
- if (ValID >= ValueList.size()) {
- FunctionPrologues.push_back(FunctionPrologueWorklist.back());
- } else {
- if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
- FunctionPrologueWorklist.back().first->setPrologueData(C);
- else
- return error("Expected a constant");
+ if (Info.Prefix) {
+ unsigned ValID = Info.Prefix - 1;
+ if (ValID < ValueList.size()) {
+ if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
+ Info.F->setPrefixData(C);
+ else
+ return error("Expected a constant");
+ Info.Prefix = 0;
+ }
}
- FunctionPrologueWorklist.pop_back();
- }
-
- while (!FunctionPersonalityFnWorklist.empty()) {
- unsigned ValID = FunctionPersonalityFnWorklist.back().second;
- if (ValID >= ValueList.size()) {
- FunctionPersonalityFns.push_back(FunctionPersonalityFnWorklist.back());
- } else {
- if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
- FunctionPersonalityFnWorklist.back().first->setPersonalityFn(C);
- else
- return error("Expected a constant");
+ if (Info.Prologue) {
+ unsigned ValID = Info.Prologue - 1;
+ if (ValID < ValueList.size()) {
+ if (Constant *C = dyn_cast_or_null<Constant>(ValueList[ValID]))
+ Info.F->setPrologueData(C);
+ else
+ return error("Expected a constant");
+ Info.Prologue = 0;
+ }
}
- FunctionPersonalityFnWorklist.pop_back();
+ if (Info.PersonalityFn || Info.Prefix || Info.Prologue)
+ FunctionOperands.push_back(Info);
+ FunctionOperandWorklist.pop_back();
}
return Error::success();
@@ -3385,8 +3384,10 @@ Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
if (Record.size() > 9)
UnnamedAddr = getDecodedUnnamedAddrType(Record[9]);
Func->setUnnamedAddr(UnnamedAddr);
- if (Record.size() > 10 && Record[10] != 0)
- FunctionPrologues.push_back(std::make_pair(Func, Record[10] - 1));
+
+ FunctionOperandInfo OperandInfo = {Func, 0, 0, 0};
+ if (Record.size() > 10)
+ OperandInfo.Prologue = Record[10];
if (Record.size() > 11)
Func->setDLLStorageClass(getDecodedDLLStorageClass(Record[11]));
@@ -3403,11 +3404,11 @@ Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
Func->setComdat(reinterpret_cast<Comdat *>(1));
}
- if (Record.size() > 13 && Record[13] != 0)
- FunctionPrefixes.push_back(std::make_pair(Func, Record[13] - 1));
+ if (Record.size() > 13)
+ OperandInfo.Prefix = Record[13];
- if (Record.size() > 14 && Record[14] != 0)
- FunctionPersonalityFns.push_back(std::make_pair(Func, Record[14] - 1));
+ if (Record.size() > 14)
+ OperandInfo.PersonalityFn = Record[14];
if (Record.size() > 15) {
Func->setDSOLocal(getDecodedDSOLocal(Record[15]));
@@ -3425,6 +3426,9 @@ Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
ValueList.push_back(Func);
+ if (OperandInfo.PersonalityFn || OperandInfo.Prefix || OperandInfo.Prologue)
+ FunctionOperands.push_back(OperandInfo);
+
// If this is a function with a body, remember the prototype we are
// creating now, so that we can match up the body with them later.
if (!isProto) {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index d86db61ee1f46..0c6e7462e1242 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -229,8 +229,11 @@ static void predictValueUseListOrderImpl(const Value *V, const Function *F,
// have been read (despite having earlier IDs). Rather than awkwardly
// modeling this behaviour here, orderModule() has assigned IDs to
// initializers of GlobalValues before GlobalValues themselves.
- if (OM.isGlobalValue(LID) && OM.isGlobalValue(RID))
+ if (OM.isGlobalValue(LID) && OM.isGlobalValue(RID)) {
+ if (LID == RID)
+ return LU->getOperandNo() > RU->getOperandNo();
return LID < RID;
+ }
// If ID is 4, then expect: 7 6 5 1 2 3.
if (LID < RID) {
diff --git a/llvm/test/Assembler/function-operand-uselistorder.ll b/llvm/test/Assembler/function-operand-uselistorder.ll
new file mode 100644
index 0000000000000..e17475f172d20
--- /dev/null
+++ b/llvm/test/Assembler/function-operand-uselistorder.ll
@@ -0,0 +1,11 @@
+; RUN: verify-uselistorder %s
+
+ at g = global i8 0
+
+define void @f1() prefix i8* @g prologue i8* @g personality i8* @g {
+ ret void
+}
+
+define void @f2() prefix i8* @g prologue i8* @g personality i8* @g {
+ ret void
+}
diff --git a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
index bc82385c4c95a..dd45209a134a3 100644
--- a/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ b/llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -202,14 +202,9 @@ ValueMapping::ValueMapping(const Module &M) {
map(A.getAliasee());
for (const GlobalIFunc &IF : M.ifuncs())
map(IF.getResolver());
- for (const Function &F : M) {
- if (F.hasPrefixData())
- map(F.getPrefixData());
- if (F.hasPrologueData())
- map(F.getPrologueData());
- if (F.hasPersonalityFn())
- map(F.getPersonalityFn());
- }
+ for (const Function &F : M)
+ for (Value *Op : F.operands())
+ map(Op);
// Function bodies.
for (const Function &F : M) {
@@ -484,14 +479,9 @@ static void changeUseLists(Module &M, Changer changeValueUseList) {
changeValueUseList(A.getAliasee());
for (GlobalIFunc &IF : M.ifuncs())
changeValueUseList(IF.getResolver());
- for (Function &F : M) {
- if (F.hasPrefixData())
- changeValueUseList(F.getPrefixData());
- if (F.hasPrologueData())
- changeValueUseList(F.getPrologueData());
- if (F.hasPersonalityFn())
- changeValueUseList(F.getPersonalityFn());
- }
+ for (Function &F : M)
+ for (Value *Op : F.operands())
+ changeValueUseList(Op);
// Function bodies.
for (Function &F : M) {
More information about the llvm-commits
mailing list