[DomTree] Add API forceVerifyDomTree

[anemet]

Summary

This API bypasses the -verify-dom-info flag to always check DT
consistency. This is useful to stress-test a single pass without
slowing down other passes.

Looks like the LoopVectorizer was trying to do something similar when it
calls verifyDomTree directly. That's a no-op unless -verify-dom-info is
used. But if -verify-dom-info is passed the verification would be run
anyway. We should probably also fix this in LV.

[reames]

LGTM. I would mildly prefer a boolean parameter to verifyDomTree with a default value, but I'm okay with either version.

You might also consider making the force version declaration under a #ifndef NDEBUG to ensure it doesn't accidentally get left on in a release build.

[anemet]

In D8818#153879, @reames wrote:

LGTM. I would mildly prefer a boolean parameter to verifyDomTree with a default value, but I'm okay with either version.

Thanks. I'll make that change -- fewer functions to remember.

[anemet]

Philip,

Rather than introducing another API flag besides the -verify-dom-info
command-line flag, I've decided to clean this up.

The new commit log should explain the situation:

[DomTree] verifyDomTree to unconditionally perform DT verification

I folded the check for the flag -verify-dom-info into the only caller
where I think it is supposed to be checked: verifyAnalysis.  (The idea
of the flag is to enable this expensive verification in
verifyPreservedAnalysis.)

I'm assuming that when manually scheduling the verification pass with
-passes=verify<domtree>, we do want to perform the verification, so I
don't check for the flag there.

Please let me know if you're OK with this.

Thanks,
Adam

[reames]

LGTM.

[anemet]

It's r236575, thanks Philip.

lib/IR/Dominators.cpp

bool DominatorTree::isReachableFromEntry(const Use &U) const {
   if (PHINode *PN = dyn_cast<PHINode>(I))
     return isReachableFromEntry(PN->getIncomingBlock(U));
 
   // Everything else uses their operands in their own block.
   return isReachableFromEntry(I->getParent());
 }
 
 void DominatorTree::verifyDomTree() const {
-  if (!VerifyDomInfo)
-    return;
-
   Function &F = *getRoot()->getParent();
 
   DominatorTree OtherDT;
   OtherDT.recalculate(F);
   if (compare(OtherDT)) {
     errs() << "DominatorTree is not up to date!\nComputed:\n";
     print(errs());
     errs() << "\nActual:\n";

 INITIALIZE_PASS(DominatorTreeWrapperPass, "domtree",
                 "Dominator Tree Construction", true, true)
 
 bool DominatorTreeWrapperPass::runOnFunction(Function &F) {
   DT.recalculate(F);
   return false;
 }
 
-void DominatorTreeWrapperPass::verifyAnalysis() const { DT.verifyDomTree(); }
+void DominatorTreeWrapperPass::verifyAnalysis() const {
+    if (VerifyDomInfo)
+      DT.verifyDomTree();
+}
 
 void DominatorTreeWrapperPass::print(raw_ostream &OS, const Module *) const {
   DT.print(OS);
 }
 

lib/Transforms/Scalar/PlaceSafepoints.cpp

static Instruction *findLocationForEntrySafepoint(Function &F,
   }
 
   BasicBlock *BB = cursor->getParent();
   SplitBlock(BB, cursor, nullptr);
 
   // Note: SplitBlock modifies the DT.  Simply passing a Pass (which is a
   // module pass) is not enough.
   DT.recalculate(F);
-#ifndef NDEBUG
+
   // SplitBlock updates the DT
-  DT.verifyDomTree();
-#endif
+  DEBUG(DT.verifyDomTree());
 
   return BB->getTerminator();
 }
 
 /// Identify the list of call sites which need to be have parseable state
 static void findCallSafepoints(Function &F,
                                std::vector<CallSite> &Found /*rval*/) {
   assert(Found.empty() && "must be empty!");