[PATCH] D21042: Verifier: Simplify and fix issue where we were not verifying unmaterialized functions.
Peter Collingbourne via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 6 14:21:03 PDT 2016
pcc created this revision.
pcc added a reviewer: aprantl.
pcc added a subscriber: llvm-commits.
- Arrange to call verify(Function &) on each function, followed by
verify(Module &), whether the verifier is being used from the pass or from
verifyModule(). As a side effect, this fixes an issue that caused us not
to call verify(Function &) on unmaterialized functions from verifyModule().
- Remove previously unreachable code that verifies that a function definition
has an entry block. By definition, a function definition has at least one block.
http://reviews.llvm.org/D21042
Files:
lib/IR/Verifier.cpp
Index: lib/IR/Verifier.cpp
===================================================================
--- lib/IR/Verifier.cpp
+++ lib/IR/Verifier.cpp
@@ -276,13 +276,14 @@
Context = &M->getContext();
// First ensure the function is well-enough formed to compute dominance
- // information.
- if (F.empty()) {
- if (OS)
- *OS << "Function '" << F.getName()
- << "' does not contain an entry block!\n";
- return false;
- }
+ // information, and directly compute a dominance tree. We don't rely on the
+ // pass manager to provide this as it isolates us from a potentially
+ // out-of-date dominator tree and makes it significantly more complex to run
+ // this code outside of a pass manager.
+ // FIXME: It's really gross that we have to cast away constness here.
+ if (!F.empty())
+ DT.recalculate(const_cast<Function &>(F));
+
for (const BasicBlock &BB : F) {
if (!BB.empty() && BB.back().isTerminator())
continue;
@@ -296,13 +297,6 @@
return false;
}
- // Now directly compute a dominance tree. We don't rely on the pass
- // manager to provide this as it isolates us from a potentially
- // out-of-date dominator tree and makes it significantly more complex to
- // run this code outside of a pass manager.
- // FIXME: It's really gross that we have to cast away constness here.
- DT.recalculate(const_cast<Function &>(F));
-
Broken = false;
// FIXME: We strip const here because the inst visitor strips const.
visit(const_cast<Function &>(F));
@@ -320,17 +314,10 @@
Context = &M.getContext();
Broken = false;
- // Scan through, checking all of the external function's linkage now...
- for (const Function &F : M) {
- visitGlobalValue(F);
-
- // Check to make sure function prototypes are okay.
- if (F.isDeclaration()) {
- visitFunction(F);
- if (F.getIntrinsicID() == Intrinsic::experimental_deoptimize)
- DeoptimizeDeclarations.push_back(&F);
- }
- }
+ // Collect all declarations of the llvm.experimental.deoptimize intrinsic.
+ for (const Function &F : M)
+ if (F.getIntrinsicID() == Intrinsic::experimental_deoptimize)
+ DeoptimizeDeclarations.push_back(&F);
// Now that we've visited every function, verify that we never asked to
// recover a frame index that wasn't escaped.
@@ -1872,6 +1859,8 @@
// visitFunction - Verify that a function is ok.
//
void Verifier::visitFunction(const Function &F) {
+ visitGlobalValue(F);
+
// Check function arguments.
FunctionType *FT = F.getFunctionType();
unsigned NumArgs = F.arg_size();
@@ -4450,8 +4439,7 @@
bool Broken = false;
for (const Function &F : M)
- if (!F.isDeclaration() && !F.isMaterializable())
- Broken |= !V.verify(F);
+ Broken |= !V.verify(F);
Broken |= !V.verify(M);
if (BrokenDebugInfo)
@@ -4488,7 +4476,12 @@
}
bool doFinalization(Module &M) override {
- bool HasErrors = !V.verify(M);
+ bool HasErrors = false;
+ for (Function &F : M)
+ if (F.isDeclaration())
+ HasErrors |= !V.verify(F);
+
+ HasErrors |= !V.verify(M);
if (FatalErrors) {
if (HasErrors)
report_fatal_error("Broken module found, compilation aborted!");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21042.59786.patch
Type: text/x-patch
Size: 3310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160606/b927cbda/attachment.bin>
More information about the llvm-commits
mailing list