Standardize LLVM module verification (#46387)

* hide away the llvm verification steps behind JL_VERIFY_PASSES in our llvm passes
* verify modules before and after optimization runs
* Verify passes when compiling with assertions

Make.inc

@@ -452,8 +452,8 @@ JULIACODEGEN := LLVM
 ifeq ($(FORCE_ASSERTIONS), 1)
 # C++ code needs to include LLVM header with the same assertion flag as LLVM
 # Use this flag to re-enable assertion in our code after all the LLVM headers are included
-CXX_DISABLE_ASSERTION :=
-DISABLE_ASSERTIONS :=
+CXX_DISABLE_ASSERTION := -DJL_VERIFY_PASSES
+DISABLE_ASSERTIONS := -DJL_VERIFY_PASSES
 else
 CXX_DISABLE_ASSERTION := -DJL_NDEBUG
 DISABLE_ASSERTIONS := -DNDEBUG -DJL_NDEBUG

src/aotcompile.cpp

@@ -590,7 +590,11 @@ void jl_dump_native_impl(void *native_code,
  // do the actual work
  auto add_output = [&] (Module &M, StringRef unopt_bc_Name, StringRef bc_Name, StringRef obj_Name, StringRef asm_Name) {
  preopt.run(M, empty.MAM);
- if (bc_fname || obj_fname || asm_fname) optimizer.run(M);
+ if (bc_fname || obj_fname || asm_fname) {
+ assert(!verifyModule(M, &errs()));
+ optimizer.run(M);
+ assert(!verifyModule(M, &errs()));
+ }
 
  // We would like to emit an alias or an weakref alias to redirect these symbols
  // but LLVM doesn't let us emit a GlobalAlias to a declaration...
@@ -1031,6 +1035,7 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz
  // and will better match what's actually in sysimg.
  for (auto &global : output.globals)
  global.second->setLinkage(GlobalValue::ExternalLinkage);
+ assert(!verifyModule(*m.getModuleUnlocked(), &errs()));
  if (optimize) {
 #ifndef JL_USE_NEW_PM
  legacy::PassManager PM;
@@ -1042,6 +1047,7 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz
 #endif
  //Safe b/c context lock is held by output
  PM.run(*m.getModuleUnlocked());
+ assert(!verifyModule(*m.getModuleUnlocked(), &errs()));
  }
  const std::string *fname;
  if (decls.functionObject == "jl_fptr_args" || decls.functionObject == "jl_fptr_sparam")

src/jitlayers.cpp

@@ -14,6 +14,7 @@
 #if JL_LLVM_VERSION >= 130000
 #include <llvm/ExecutionEngine/Orc/ExecutorProcessControl.h>
 #endif
+#include <llvm/IR/Verifier.h>
 #include <llvm/Support/DynamicLibrary.h>
 #include <llvm/Support/FormattedStream.h>
 #include <llvm/Support/SmallVectorMemoryBuffer.h>
@@ -1106,7 +1107,9 @@ namespace {
  JL_TIMING(LLVM_OPT);
 
  //Run the optimization
+ assert(!verifyModule(M, &errs()));
  (***PMs).run(M);
+ assert(!verifyModule(M, &errs()));
 
  uint64_t end_time = 0;
  {

src/llvm-alloc-opt.cpp

@@ -1184,7 +1184,9 @@ bool AllocOpt::runOnFunction(Function &F, function_ref<DominatorTree&()> GetDT)
  optimizer.initialize();
  optimizer.optimizeAll();
  bool modified = optimizer.finalize();
+#ifdef JL_VERIFY_PASSES
  assert(!verifyFunction(F, &errs()));
+#endif
  return modified;
 }
 

src/llvm-cpufeatures.cpp

@@ -110,7 +110,9 @@ bool lowerCPUFeatures(Module &M)
  for (auto I: Materialized) {
  I->eraseFromParent();
  }
+#ifdef JL_VERIFY_PASSES
  assert(!verifyModule(M, &errs()));
+#endif
  return true;
  } else {
  return false;

src/llvm-demote-float16.cpp

@@ -153,7 +153,9 @@ static bool demoteFloat16(Function &F)
  if (erase.size() > 0) {
  for (auto V : erase)
  V->eraseFromParent();
+#ifdef JL_VERIFY_PASSES
  assert(!verifyFunction(F, &errs()));
+#endif
  return true;
  }
  else

src/llvm-final-gc-lowering.cpp

@@ -9,6 +9,7 @@
 #include <llvm/IR/IntrinsicInst.h>
 #include <llvm/IR/Module.h>
 #include <llvm/IR/IRBuilder.h>
+#include <llvm/IR/Verifier.h>
 #include <llvm/Pass.h>
 #include <llvm/Support/Debug.h>
 #include <llvm/Transforms/Utils/ModuleUtils.h>
@@ -390,7 +391,11 @@ bool FinalLowerGCLegacy::doInitialization(Module &M) {
 }
 
 bool FinalLowerGCLegacy::doFinalization(Module &M) {
- return finalLowerGC.doFinalization(M);
+ auto ret = finalLowerGC.doFinalization(M);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
+ return ret;
 }
 
 
@@ -405,6 +410,9 @@ PreservedAnalyses FinalLowerGCPass::run(Module &M, ModuleAnalysisManager &AM)
  modified |= finalLowerGC.runOnFunction(F);
  }
  modified |= finalLowerGC.doFinalization(M);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
  if (modified) {
  return PreservedAnalyses::allInSet<CFGAnalyses>();
  }

src/llvm-julia-licm.cpp

@@ -284,7 +284,9 @@ struct JuliaLICM : public JuliaPassContext {
  if (changed && SE) {
  SE->forgetLoopDispositions(L);
  }
+#ifdef JL_VERIFY_PASSES
  assert(!verifyFunction(*L->getHeader()->getParent(), &errs()));
+#endif
  return changed;
  }
 };

src/llvm-late-gc-lowering.cpp

@@ -2741,7 +2741,11 @@ bool LateLowerGCFrameLegacy::runOnFunction(Function &F) {
  return getAnalysis<DominatorTreeWrapperPass>().getDomTree();
  };
  auto lateLowerGCFrame = LateLowerGCFrame(GetDT);
- return lateLowerGCFrame.runOnFunction(F);
+ bool modified = lateLowerGCFrame.runOnFunction(F);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ return modified;
 }
 
 PreservedAnalyses LateLowerGC::run(Function &F, FunctionAnalysisManager &AM)
@@ -2751,7 +2755,11 @@ PreservedAnalyses LateLowerGC::run(Function &F, FunctionAnalysisManager &AM)
  };
  auto lateLowerGCFrame = LateLowerGCFrame(GetDT);
  bool CFGModified = false;
- if (lateLowerGCFrame.runOnFunction(F, &CFGModified)) {
+ bool modified = lateLowerGCFrame.runOnFunction(F, &CFGModified);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ if (modified) {
  if (CFGModified) {
  return PreservedAnalyses::none();
  } else {

src/llvm-lower-handlers.cpp

@@ -17,6 +17,7 @@
 #include <llvm/IR/Module.h>
 #include <llvm/IR/Value.h>
 #include <llvm/IR/LegacyPassManager.h>
+#include <llvm/IR/Verifier.h>
 #include <llvm/Pass.h>
 #include <llvm/Support/Debug.h>
 #include <llvm/Transforms/Utils/BasicBlockUtils.h>
@@ -234,7 +235,11 @@ static bool lowerExcHandlers(Function &F) {
 
 PreservedAnalyses LowerExcHandlers::run(Function &F, FunctionAnalysisManager &AM)
 {
- if (lowerExcHandlers(F)) {
+ bool modified = lowerExcHandlers(F);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ if (modified) {
  return PreservedAnalyses::allInSet<CFGAnalyses>();
  }
  return PreservedAnalyses::all();
@@ -246,7 +251,11 @@ struct LowerExcHandlersLegacy : public FunctionPass {
  LowerExcHandlersLegacy() : FunctionPass(ID)
  {}
  bool runOnFunction(Function &F) {
- return lowerExcHandlers(F);
+ bool modified = lowerExcHandlers(F);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ return modified;
  }
 };
 

src/llvm-muladd.cpp

@@ -84,7 +84,9 @@ static bool combineMulAdd(Function &F)
  }
  }
  }
+#ifdef JL_VERIFY_PASSES
  assert(!verifyFunction(F, &errs()));
+#endif
  return modified;
 }
 

src/llvm-multiversioning.cpp

@@ -1134,8 +1134,9 @@ static bool runMultiVersioning(Module &M, function_ref<LoopInfo&(Function&)> Get
  // At this point, we should have fixed up all the uses of the cloned functions
  // and collected all the shared/target-specific relocations.
  clone.emit_metadata();
-
+#ifdef JL_VERIFY_PASSES
  assert(!verifyModule(M, &errs()));
+#endif
 
  return true;
 }

src/llvm-propagate-addrspaces.cpp

@@ -302,7 +302,11 @@ struct PropagateJuliaAddrspacesLegacy : FunctionPass {
 
  PropagateJuliaAddrspacesLegacy() : FunctionPass(ID) {}
  bool runOnFunction(Function &F) override {
- return propagateJuliaAddrspaces(F);
+ bool modified = propagateJuliaAddrspaces(F);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ return modified;
  }
 };
 
@@ -314,7 +318,12 @@ Pass *createPropagateJuliaAddrspaces() {
 }
 
 PreservedAnalyses PropagateJuliaAddrspacesPass::run(Function &F, FunctionAnalysisManager &AM) {
- if (propagateJuliaAddrspaces(F)) {
+ bool modified = propagateJuliaAddrspaces(F);
+
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyFunction(F, &errs()));
+#endif
+ if (modified) {
  return PreservedAnalyses::allInSet<CFGAnalyses>();
  } else {
  return PreservedAnalyses::all();

src/llvm-ptls.cpp

@@ -16,6 +16,7 @@
 #include <llvm/IR/Constants.h>
 #include <llvm/IR/LLVMContext.h>
 #include <llvm/IR/MDBuilder.h>
+#include <llvm/IR/Verifier.h>
 
 #include <llvm/IR/InlineAsm.h>
 #include <llvm/Transforms/Utils/BasicBlockUtils.h>
@@ -356,7 +357,11 @@ struct LowerPTLSLegacy: public ModulePass {
  bool imaging_mode;
  bool runOnModule(Module &M) override {
  LowerPTLS lower(M, imaging_mode);
- return lower.run(nullptr);
+ bool modified = lower.run(nullptr);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
+ return modified;
  }
 };
 
@@ -371,7 +376,11 @@ static RegisterPass<LowerPTLSLegacy> X("LowerPTLS", "LowerPTLS Pass",
 PreservedAnalyses LowerPTLSPass::run(Module &M, ModuleAnalysisManager &AM) {
  LowerPTLS lower(M, imaging_mode);
  bool CFGModified = false;
- if (lower.run(&CFGModified)) {
+ bool modified = lower.run(&CFGModified);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
+ if (modified) {
  if (CFGModified) {
  return PreservedAnalyses::none();
  } else {

src/llvm-remove-addrspaces.cpp

@@ -8,6 +8,7 @@
 #include <llvm/IR/Instructions.h>
 #include <llvm/IR/InstIterator.h>
 #include <llvm/IR/LegacyPassManager.h>
+#include <llvm/IR/Verifier.h>
 #include <llvm/Support/Debug.h>
 #include <llvm/Transforms/Utils/Cloning.h>
 #include <llvm/Transforms/Utils/ValueMapper.h>
@@ -464,7 +465,11 @@ struct RemoveAddrspacesPassLegacy : public ModulePass {
 
 public:
  bool runOnModule(Module &M) override {
- return removeAddrspaces(M, ASRemapper);
+ bool modified = removeAddrspaces(M, ASRemapper);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
+ return modified;
  }
 };
 
@@ -484,7 +489,11 @@ Pass *createRemoveAddrspacesPass(
 RemoveAddrspacesPass::RemoveAddrspacesPass() : RemoveAddrspacesPass(removeAllAddrspaces) {}
 
 PreservedAnalyses RemoveAddrspacesPass::run(Module &M, ModuleAnalysisManager &AM) {
- if (removeAddrspaces(M, ASRemapper)) {
+ bool modified = removeAddrspaces(M, ASRemapper);
+#ifdef JL_VERIFY_PASSES
+ assert(!verifyModule(M, &errs()));
+#endif
+ if (modified) {
  return PreservedAnalyses::allInSet<CFGAnalyses>();
  } else {
  return PreservedAnalyses::all();

src/llvm-simdloop.cpp

@@ -232,8 +232,9 @@ static bool markLoopInfo(Module &M, Function *marker, function_ref<LoopInfo &(Fu
  for (Instruction *I : ToDelete)
  I->deleteValue();
  marker->eraseFromParent();
-
+#ifdef JL_VERIFY_PASSES
  assert(!verifyModule(M, &errs()));
+#endif
  return Changed;
 }