[llvm] r277409 - Tie the Verifier class to a Module; NFCI
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 1 18:34:51 PDT 2016
Author: sanjoy
Date: Mon Aug 1 20:34:50 2016
New Revision: 277409
URL: http://llvm.org/viewvc/llvm-project?rev=277409&view=rev
Log:
Tie the Verifier class to a Module; NFCI
Summary:
This commit changes the Verifier class to accept a Module via the
constructor to make it obvious that a specific instance of the class is
only intended to work with a specific module. The `updateModule` setter
(despite being private) was making this fact less transparent.
There are fields in the `Verifier` class like `DeoptimizeDeclarations`
and `GlobalValueVisited` which are module specific, so a given
Verifier instance will not in fact work across multiple modules today.
This change just makes that more obvious.
The motivation is to make it easy to get to the datalayout of the
module unambiguously. That is required to verify that `inttoptr` and
`ptrtoint` constant expressions are well typed in the face of
non-integral pointer types.
Reviewers: dexonsmith, bkramer, majnemer, chandlerc
Subscribers: mehdi_amini, mcrosier, llvm-commits
Differential Revision: https://reviews.llvm.org/D23040
Modified:
llvm/trunk/lib/IR/Verifier.cpp
Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=277409&r1=277408&r2=277409&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Mon Aug 1 20:34:50 2016
@@ -85,8 +85,10 @@ static cl::opt<bool> VerifyDebugInfo("ve
namespace {
struct VerifierSupport {
raw_ostream *OS;
- const Module *M = nullptr;
- Optional<ModuleSlotTracker> MST;
+ const Module &M;
+ ModuleSlotTracker MST;
+ const DataLayout &DL;
+ LLVMContext &Context;
/// Track the brokenness of the module while recursively visiting.
bool Broken = false;
@@ -95,7 +97,8 @@ struct VerifierSupport {
/// Whether to treat broken debug info as an error.
bool TreatBrokenDebugInfoAsError = true;
- explicit VerifierSupport(raw_ostream *OS) : OS(OS) {}
+ explicit VerifierSupport(raw_ostream *OS, const Module &M)
+ : OS(OS), M(M), MST(&M), DL(M.getDataLayout()), Context(M.getContext()) {}
private:
template <class NodeTy> void Write(const ilist_iterator<NodeTy> &I) {
@@ -103,8 +106,6 @@ private:
}
void Write(const Module *M) {
- if (!M)
- return;
*OS << "; ModuleID = '" << M->getModuleIdentifier() << "'\n";
}
@@ -112,10 +113,10 @@ private:
if (!V)
return;
if (isa<Instruction>(V)) {
- V->print(*OS, *MST);
+ V->print(*OS, MST);
*OS << '\n';
} else {
- V->printAsOperand(*OS, true, *MST);
+ V->printAsOperand(*OS, true, MST);
*OS << '\n';
}
}
@@ -126,7 +127,7 @@ private:
void Write(const Metadata *MD) {
if (!MD)
return;
- MD->print(*OS, *MST, M);
+ MD->print(*OS, MST, &M);
*OS << '\n';
}
@@ -137,7 +138,7 @@ private:
void Write(const NamedMDNode *NMD) {
if (!NMD)
return;
- NMD->print(*OS, *MST);
+ NMD->print(*OS, MST);
*OS << '\n';
}
@@ -209,7 +210,6 @@ public:
class Verifier : public InstVisitor<Verifier>, VerifierSupport {
friend class InstVisitor<Verifier>;
- LLVMContext *Context;
DominatorTree DT;
/// \brief When verifying a basic block, keep track of all of the
@@ -252,19 +252,12 @@ class Verifier : public InstVisitor<Veri
// constant expressions, we can arrive at a particular user many times.
SmallPtrSet<const Value *, 32> GlobalValueVisited;
- void checkAtomicMemAccessSize(const Module *M, Type *Ty,
- const Instruction *I);
-
- void updateModule(const Module *NewM) {
- if (M == NewM)
- return;
- MST.emplace(NewM);
- M = NewM;
- }
+ void checkAtomicMemAccessSize(Type *Ty, const Instruction *I);
public:
- explicit Verifier(raw_ostream *OS, bool ShouldTreatBrokenDebugInfoAsError)
- : VerifierSupport(OS), Context(nullptr), LandingPadResultTy(nullptr),
+ explicit Verifier(raw_ostream *OS, bool ShouldTreatBrokenDebugInfoAsError,
+ const Module &M)
+ : VerifierSupport(OS, M), LandingPadResultTy(nullptr),
SawFrameEscape(false) {
TreatBrokenDebugInfoAsError = ShouldTreatBrokenDebugInfoAsError;
}
@@ -272,8 +265,8 @@ public:
bool hasBrokenDebugInfo() const { return BrokenDebugInfo; }
bool verify(const Function &F) {
- updateModule(F.getParent());
- Context = &M->getContext();
+ assert(F.getParent() == &M &&
+ "An instance of this class only works with a specific module!");
// First ensure the function is well-enough formed to compute dominance
// information, and directly compute a dominance tree. We don't rely on the
@@ -291,7 +284,7 @@ public:
if (OS) {
*OS << "Basic Block in function '" << F.getName()
<< "' does not have terminator!\n";
- BB.printAsOperand(*OS, true, *MST);
+ BB.printAsOperand(*OS, true, MST);
*OS << "\n";
}
return false;
@@ -309,9 +302,8 @@ public:
return !Broken;
}
- bool verify(const Module &M) {
- updateModule(&M);
- Context = &M.getContext();
+ /// Verify the module that this instance of \c Verifier was initialized with.
+ bool verify() {
Broken = false;
// Collect all declarations of the llvm.experimental.deoptimize intrinsic.
@@ -518,17 +510,17 @@ void Verifier::visitGlobalValue(const Gl
forEachUser(&GV, GlobalValueVisited, [&](const Value *V) -> bool {
if (const Instruction *I = dyn_cast<Instruction>(V)) {
if (!I->getParent() || !I->getParent()->getParent())
- CheckFailed("Global is referenced by parentless instruction!", &GV,
- M, I);
- else if (I->getParent()->getParent()->getParent() != M)
- CheckFailed("Global is referenced in a different module!", &GV,
- M, I, I->getParent()->getParent(),
+ CheckFailed("Global is referenced by parentless instruction!", &GV, &M,
+ I);
+ else if (I->getParent()->getParent()->getParent() != &M)
+ CheckFailed("Global is referenced in a different module!", &GV, &M, I,
+ I->getParent()->getParent(),
I->getParent()->getParent()->getParent());
return false;
} else if (const Function *F = dyn_cast<Function>(V)) {
- if (F->getParent() != M)
- CheckFailed("Global is used by function in a different module", &GV,
- M, F, F->getParent());
+ if (F->getParent() != &M)
+ CheckFailed("Global is used by function in a different module", &GV, &M,
+ F, F->getParent());
return false;
}
return true;
@@ -562,7 +554,7 @@ void Verifier::visitGlobalVariable(const
if (ArrayType *ATy = dyn_cast<ArrayType>(GV.getValueType())) {
StructType *STy = dyn_cast<StructType>(ATy->getElementType());
PointerType *FuncPtrTy =
- FunctionType::get(Type::getVoidTy(*Context), false)->getPointerTo();
+ FunctionType::get(Type::getVoidTy(Context), false)->getPointerTo();
// FIXME: Reject the 2-field form in LLVM 4.0.
Assert(STy &&
(STy->getNumElements() == 2 || STy->getNumElements() == 3) &&
@@ -1135,7 +1127,7 @@ void Verifier::visitDIImportedEntity(con
void Verifier::visitComdat(const Comdat &C) {
// The Module is invalid if the GlobalValue has private linkage. Entities
// with private linkage don't have entries in the symbol table.
- if (const GlobalValue *GV = M->getNamedValue(C.getName()))
+ if (const GlobalValue *GV = M.getNamedValue(C.getName()))
Assert(!GV->hasPrivateLinkage(), "comdat global value has private linkage",
GV);
}
@@ -1402,12 +1394,12 @@ void Verifier::verifyParameterAttrs(Attr
"'noinline and alwaysinline' are incompatible!",
V);
- Assert(!AttrBuilder(Attrs, Idx)
- .overlaps(AttributeFuncs::typeIncompatible(Ty)),
- "Wrong types for attribute: " +
- AttributeSet::get(*Context, Idx,
- AttributeFuncs::typeIncompatible(Ty)).getAsString(Idx),
- V);
+ Assert(
+ !AttrBuilder(Attrs, Idx).overlaps(AttributeFuncs::typeIncompatible(Ty)),
+ "Wrong types for attribute: " +
+ AttributeSet::get(Context, Idx, AttributeFuncs::typeIncompatible(Ty))
+ .getAsString(Idx),
+ V);
if (PointerType *PTy = dyn_cast<PointerType>(Ty)) {
SmallPtrSet<Type*, 4> Visited;
@@ -1631,8 +1623,8 @@ void Verifier::visitConstantExprsRecursi
if (const auto *GV = dyn_cast<GlobalValue>(C)) {
// Global Values get visited separately, but we do need to make sure
// that the global value is in the correct module
- Assert(GV->getParent() == M, "Referencing global in another module!",
- EntryC, M, GV, GV->getParent());
+ Assert(GV->getParent() == &M, "Referencing global in another module!",
+ EntryC, &M, GV, GV->getParent());
continue;
}
@@ -1881,7 +1873,7 @@ void Verifier::visitFunction(const Funct
FunctionType *FT = F.getFunctionType();
unsigned NumArgs = F.arg_size();
- Assert(Context == &F.getContext(),
+ Assert(&Context == &F.getContext(),
"Function context does not match Module context!", &F);
Assert(!F.hasCommonLinkage(), "Functions may not have common linkage", &F);
@@ -2412,7 +2404,7 @@ void Verifier::visitPtrToIntInst(PtrToIn
"PtrToInt source must be pointer", &I);
if (auto *PTy = dyn_cast<PointerType>(SrcTy->getScalarType()))
- Assert(!M->getDataLayout().isNonIntegralPointerType(PTy),
+ Assert(!DL.isNonIntegralPointerType(PTy),
"ptrtoint not supported for non-integral pointers");
Assert(DestTy->getScalarType()->isIntegerTy(),
@@ -2441,7 +2433,7 @@ void Verifier::visitIntToPtrInst(IntToPt
"IntToPtr result must be a pointer", &I);
if (auto *PTy = dyn_cast<PointerType>(DestTy->getScalarType()))
- Assert(!M->getDataLayout().isNonIntegralPointerType(PTy),
+ Assert(!DL.isNonIntegralPointerType(PTy),
"inttoptr not supported for non-integral pointers");
Assert(SrcTy->isVectorTy() == DestTy->isVectorTy(), "IntToPtr type mismatch",
@@ -2976,9 +2968,8 @@ void Verifier::visitRangeMetadata(Instru
}
}
-void Verifier::checkAtomicMemAccessSize(const Module *M, Type *Ty,
- const Instruction *I) {
- unsigned Size = M->getDataLayout().getTypeSizeInBits(Ty);
+void Verifier::checkAtomicMemAccessSize(Type *Ty, const Instruction *I) {
+ unsigned Size = DL.getTypeSizeInBits(Ty);
Assert(Size >= 8, "atomic memory access' size must be byte-sized", Ty, I);
Assert(!(Size & (Size - 1)),
"atomic memory access' operand must have a power-of-two size", Ty, I);
@@ -3002,7 +2993,7 @@ void Verifier::visitLoadInst(LoadInst &L
"atomic load operand must have integer, pointer, or floating point "
"type!",
ElTy, &LI);
- checkAtomicMemAccessSize(M, ElTy, &LI);
+ checkAtomicMemAccessSize(ElTy, &LI);
} else {
Assert(LI.getSynchScope() == CrossThread,
"Non-atomic load cannot have SynchronizationScope specified", &LI);
@@ -3031,7 +3022,7 @@ void Verifier::visitStoreInst(StoreInst
"atomic store operand must have integer, pointer, or floating point "
"type!",
ElTy, &SI);
- checkAtomicMemAccessSize(M, ElTy, &SI);
+ checkAtomicMemAccessSize(ElTy, &SI);
} else {
Assert(SI.getSynchScope() == CrossThread,
"Non-atomic store cannot have SynchronizationScope specified", &SI);
@@ -3120,7 +3111,7 @@ void Verifier::visitAtomicCmpXchgInst(At
Assert(ElTy->isIntegerTy() || ElTy->isPointerTy(),
"cmpxchg operand must have integer or pointer type",
ElTy, &CXI);
- checkAtomicMemAccessSize(M, ElTy, &CXI);
+ checkAtomicMemAccessSize(ElTy, &CXI);
Assert(ElTy == CXI.getOperand(1)->getType(),
"Expected value type does not match pointer operand type!", &CXI,
ElTy);
@@ -3139,7 +3130,7 @@ void Verifier::visitAtomicRMWInst(Atomic
Type *ElTy = PTy->getElementType();
Assert(ElTy->isIntegerTy(), "atomicrmw operand must have integer type!",
&RMWI, ElTy);
- checkAtomicMemAccessSize(M, ElTy, &RMWI);
+ checkAtomicMemAccessSize(ElTy, &RMWI);
Assert(ElTy == RMWI.getOperand(1)->getType(),
"Argument value type does not match pointer operand type!", &RMWI,
ElTy);
@@ -3685,8 +3676,8 @@ void Verifier::visitInstruction(Instruct
"Cannot invoke an intrinsic other than donothing, patchpoint or "
"statepoint",
&I);
- Assert(F->getParent() == M, "Referencing function in another module!",
- &I, M, F, F->getParent());
+ Assert(F->getParent() == &M, "Referencing function in another module!",
+ &I, &M, F, F->getParent());
} else if (BasicBlock *OpBB = dyn_cast<BasicBlock>(I.getOperand(i))) {
Assert(OpBB->getParent() == BB->getParent(),
"Referring to a basic block in another function!", &I);
@@ -3694,7 +3685,8 @@ void Verifier::visitInstruction(Instruct
Assert(OpArg->getParent() == BB->getParent(),
"Referring to an argument in another function!", &I);
} else if (GlobalValue *GV = dyn_cast<GlobalValue>(I.getOperand(i))) {
- Assert(GV->getParent() == M, "Referencing global in another module!", &I, M, GV, GV->getParent());
+ Assert(GV->getParent() == &M, "Referencing global in another module!", &I,
+ &M, GV, GV->getParent());
} else if (isa<Instruction>(I.getOperand(i))) {
verifyDominatesUse(I, i);
} else if (isa<InlineAsm>(I.getOperand(i))) {
@@ -4279,7 +4271,7 @@ void Verifier::verifyBitPieceExpression(
}
void Verifier::verifyCompileUnits() {
- auto *CUs = M->getNamedMetadata("llvm.dbg.cu");
+ auto *CUs = M.getNamedMetadata("llvm.dbg.cu");
SmallPtrSet<const Metadata *, 2> Listed;
if (CUs)
Listed.insert(CUs->op_begin(), CUs->op_end());
@@ -4311,7 +4303,7 @@ bool llvm::verifyFunction(const Function
Function &F = const_cast<Function &>(f);
// Don't use a raw_null_ostream. Printing IR is expensive.
- Verifier V(OS, /*ShouldTreatBrokenDebugInfoAsError=*/true);
+ Verifier V(OS, /*ShouldTreatBrokenDebugInfoAsError=*/true, *f.getParent());
// Note that this function's return value is inverted from what you would
// expect of a function called "verify".
@@ -4321,13 +4313,13 @@ bool llvm::verifyFunction(const Function
bool llvm::verifyModule(const Module &M, raw_ostream *OS,
bool *BrokenDebugInfo) {
// Don't use a raw_null_ostream. Printing IR is expensive.
- Verifier V(OS, /*ShouldTreatBrokenDebugInfoAsError=*/!BrokenDebugInfo);
+ Verifier V(OS, /*ShouldTreatBrokenDebugInfoAsError=*/!BrokenDebugInfo, M);
bool Broken = false;
for (const Function &F : M)
Broken |= !V.verify(F);
- Broken |= !V.verify(M);
+ Broken |= !V.verify();
if (BrokenDebugInfo)
*BrokenDebugInfo = V.hasBrokenDebugInfo();
// Note that this function's return value is inverted from what you would
@@ -4339,23 +4331,26 @@ namespace {
struct VerifierLegacyPass : public FunctionPass {
static char ID;
- Verifier V;
+ std::unique_ptr<Verifier> V;
bool FatalErrors = true;
- VerifierLegacyPass()
- : FunctionPass(ID),
- V(&dbgs(), /*ShouldTreatBrokenDebugInfoAsError=*/false) {
+ VerifierLegacyPass() : FunctionPass(ID) {
initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
}
explicit VerifierLegacyPass(bool FatalErrors)
: FunctionPass(ID),
- V(&dbgs(), /*ShouldTreatBrokenDebugInfoAsError=*/false),
FatalErrors(FatalErrors) {
initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
}
+ bool doInitialization(Module &M) override {
+ V = llvm::make_unique<Verifier>(
+ &dbgs(), /*ShouldTreatBrokenDebugInfoAsError=*/false, M);
+ return false;
+ }
+
bool runOnFunction(Function &F) override {
- if (!V.verify(F) && FatalErrors)
+ if (!V->verify(F) && FatalErrors)
report_fatal_error("Broken function found, compilation aborted!");
return false;
@@ -4365,17 +4360,17 @@ struct VerifierLegacyPass : public Funct
bool HasErrors = false;
for (Function &F : M)
if (F.isDeclaration())
- HasErrors |= !V.verify(F);
+ HasErrors |= !V->verify(F);
- HasErrors |= !V.verify(M);
+ HasErrors |= !V->verify();
if (FatalErrors) {
if (HasErrors)
report_fatal_error("Broken module found, compilation aborted!");
- assert(!V.hasBrokenDebugInfo() && "Module contains invalid debug info");
+ assert(!V->hasBrokenDebugInfo() && "Module contains invalid debug info");
}
// Strip broken debug info.
- if (V.hasBrokenDebugInfo()) {
+ if (V->hasBrokenDebugInfo()) {
DiagnosticInfoIgnoringInvalidDebugMetadata DiagInvalid(M);
M.getContext().diagnose(DiagInvalid);
if (!StripDebugInfo(M))
More information about the llvm-commits
mailing list