SafepointIRVerifier port to new Pass Manager

[skatkov]

Summary

Straightforward port of StatepointIRVerifier pass to new Pass Manager framework.

[fedor.sergeev]

Please, update verify-safepoint tests to use new-pass-manager flavor.

include/llvm/IR/SafepointIRVerifier.h

41 ↗

(On Diff #192287)

Why do we need Module wrapper for this?
Seems like function pass is enough.

lib/Passes/PassRegistry.def

220 ↗

(On Diff #192287)

Just a few lines below there are a handful of verifiers.
They seem to be using a text pipeline format of "verify<something>".

Lets do this as "verify<safepoint-ir>"

(and move it down there)

[reames]

Mark to reflect Fedor's comments

[skatkov]

include/llvm/IR/SafepointIRVerifier.h

41 ↗

(On Diff #192287)

This is done to similarity to VerifierPass which implements both Module and Function pass functionality. I agree that this pass does not do any verification on Module level but I do not see any issues with keeping both wrappers.

lib/Passes/PassRegistry.def

220 ↗

(On Diff #192287)

I preferred to keep the same name as Legacy pass manager used.

[skatkov]

Fedor's comments handled.

[fedor.sergeev]

include/llvm/IR/SafepointIRVerifier.h

41 ↗

(On Diff #192287)

There is no need for additional wrapper.
We already have module->function adaptors and it applies automatically when you specify

-passes='verify<safepoint-ir>'

So from usability point of view there is no utility in adding this wrapper.

VerifierPass calls verifyModule for Module version and verifyFunction for Function version, so its not just a wrapper.

[fedor.sergeev]

LGTM

[aprantl]

Unfortunately this has introduced a cyclic dependency in the module build graph. Could you please revert the patch to get the bots running again until we figured out a solution?

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22548/consoleFull#147888646349ba4694-19c4-4d7e-bec5-911270d8a58c

In file included from <module-includes>:21:
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/IR/SafepointIRVerifier.h:21:10: fatal error: cyclic dependency in module 'LLVM_intrinsic_gen': LLVM_intrinsic_gen -> LLVM_IR -> LLVM_intrinsic_gen
#include "llvm/IR/PassManager.h"
         ^
While building module 'LLVM_intrinsic_gen' imported from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/lib/IR/Attributes.cpp:15:
In file included from <module-includes>:1:
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/IR/Argument.h:19:10: fatal error: could not build module 'LLVM_IR'
#include "llvm/IR/Value.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/lib/IR/Attributes.cpp:15:10: fatal error: could not build module 'LLVM_intrinsic_gen'
#include "llvm/IR/Attributes.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
3 errors generated.

[aprantl]

NB: not the patch in this review, but the follow-up patch:

Author: Serguei Katkov <[e-mail]>
Date:   Thu Mar 28 07:02:00 2019 +0000

    SafepointIRVerifier port to new Pass Manager
    
    Add missed include.
    
    llvm-svn: 357148

diff --git a/llvm/include/llvm/IR/SafepointIRVerifier.h b/llvm/include/llvm/IR/SafepointIRVerifier.h
index 2a13bcc92a7..ec5527954ad 100644
--- a/llvm/include/llvm/IR/SafepointIRVerifier.h
+++ b/llvm/include/llvm/IR/SafepointIRVerifier.h
@@ -18,6 +18,8 @@
 #ifndef LLVM_IR_SAFEPOINT_IR_VERIFIER
 #define LLVM_IR_SAFEPOINT_IR_VERIFIER
 
+#include "llvm/IR/PassManager.h"
+
 namespace llvm {
 
 class Function;

[aprantl]

I reverted the commit in r357201. Let me know if you need any help resolving the cyclic dependency. (You can reproduce it by cmake-ing with -DLLVM_ENABLE_MODULES=1.)

[skatkov]

Thank you for reverting the patch, I'll take a look into this dependency. I'll ping you if I have a trouble in reproducing the crash.

[fedor.sergeev]

Landed the change back, with fixes to module.modulemap. rL357361.

[aprantl]

Thanks, and sorry for the extra inconvenience!

llvm/trunk/include/llvm/IR/SafepointIRVerifier.h

 class FunctionPass;
 
 /// Run the safepoint verifier over a single function.  Crashes on failure.
 void verifySafepointIR(Function &F);
 
 /// Create an instance of the safepoint verifier pass which can be added to
 /// a pass pipeline to check for relocation bugs.
 FunctionPass *createSafepointIRVerifierPass();
+
+/// Create an instance of the safepoint verifier pass which can be added to
+/// a pass pipeline to check for relocation bugs.
+class SafepointIRVerifierPass : public PassInfoMixin<SafepointIRVerifierPass> {
+
+public:
+  explicit SafepointIRVerifierPass() {}
+
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
 }
 
 #endif // LLVM_IR_SAFEPOINT_IR_VERIFIER

llvm/trunk/lib/IR/SafepointIRVerifier.cpp

void addDeadEdge(const Use &DeadEdge) {
     addDeadBlock(BB);
   }
 };
 } // namespace
 
 static void Verify(const Function &F, const DominatorTree &DT,
                    const CFGDeadness &CD);
 
+namespace llvm {
+PreservedAnalyses SafepointIRVerifierPass::run(Function &F,
+                                               FunctionAnalysisManager &AM) {
+  const auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
+  CFGDeadness CD;
+  CD.processFunction(F, DT);
+  Verify(F, DT, CD);
+  return PreservedAnalyses::all();
+}
+}
+
 namespace {
 
 struct SafepointIRVerifier : public FunctionPass {
   static char ID; // Pass identification, replacement for typeid
   SafepointIRVerifier() : FunctionPass(ID) {
     initializeSafepointIRVerifierPass(*PassRegistry::getPassRegistry());
   }
 

llvm/trunk/lib/Passes/PassBuilder.cpp

 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/CodeGen/PreISelIntrinsicLowering.h"
 #include "llvm/CodeGen/UnreachableBlockElim.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/IR/SafepointIRVerifier.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/AggressiveInstCombine/AggressiveInstCombine.h"
 #include "llvm/Transforms/IPO/AlwaysInliner.h"
 #include "llvm/Transforms/IPO/ArgumentPromotion.h"

llvm/trunk/lib/Passes/PassRegistry.def

 FUNCTION_PASS("sroa", SROA())
 FUNCTION_PASS("tailcallelim", TailCallElimPass())
 FUNCTION_PASS("unreachableblockelim", UnreachableBlockElimPass())
 FUNCTION_PASS("verify", VerifierPass())
 FUNCTION_PASS("verify<domtree>", DominatorTreeVerifierPass())
 FUNCTION_PASS("verify<loops>", LoopVerifierPass())
 FUNCTION_PASS("verify<memoryssa>", MemorySSAVerifierPass())
 FUNCTION_PASS("verify<regions>", RegionInfoVerifierPass())
+FUNCTION_PASS("verify<safepoint-ir>", SafepointIRVerifierPass())
 FUNCTION_PASS("view-cfg", CFGViewerPass())
 FUNCTION_PASS("view-cfg-only", CFGOnlyViewerPass())
 FUNCTION_PASS("transform-warning", WarnMissedTransformationsPass())
 FUNCTION_PASS("asan", AddressSanitizerPass(false, false, false))
 FUNCTION_PASS("msan", MemorySanitizerPass({}))
 FUNCTION_PASS("tsan", ThreadSanitizerPass())
 #undef FUNCTION_PASS