[IndVars] Option verify-indvars is broken (and always has been), delete it

[mkazantsev]

Summary

This option is switched off by default, and it seems that it has never worked correctly.
What it basically does is: it remembers current BECount SCEV, and after all transforms
tries to validate some facts for it. However, between these two points this SCEV may
become invalid (e.g. because some SCEVUnknown it references may be deleted as dead
code). So basically it may work with broken pointers.

Besides, its implementation does strange things (e.g. forgetLoop) which are invasive and
may affect behavior in other parts of the system (specifically verification), concealing some
other problems. Another issue is that it may use SCEVCouldNotCompute object without
checking this.

The option is not used in any unit tests, and if switched on by default, the following tests
fail:

********************
Failed Tests (14):
  LLVM :: Transforms/IndVarSimplify/2005-06-15-InstMoveCrash.ll
  LLVM :: Transforms/IndVarSimplify/2007-06-06-DeleteDanglesPtr.ll
  LLVM :: Transforms/IndVarSimplify/2008-10-03-CouldNotCompute.ll
  LLVM :: Transforms/IndVarSimplify/2009-05-24-useafterfree.ll
  LLVM :: Transforms/IndVarSimplify/2011-10-27-lftrnull.ll
  LLVM :: Transforms/IndVarSimplify/ARM/code-size.ll
  LLVM :: Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
  LLVM :: Transforms/IndVarSimplify/X86/pr57187.ll
  LLVM :: Transforms/IndVarSimplify/X86/verify-scev.ll
  LLVM :: Transforms/IndVarSimplify/bbi-63564.ll
  LLVM :: Transforms/IndVarSimplify/invalidate-modified-lcssa-phi.ll
  LLVM :: Transforms/IndVarSimplify/loop-predication.ll
  LLVM :: Transforms/IndVarSimplify/post-inc-range.ll
  LLVM :: Transforms/IndVarSimplify/turn-to-invariant.ll

********************
Unexpectedly Passed Tests (1):
  LLVM :: Transforms/IndVarSimplify/pr55689.ll

None of these looks like real problems found by verification, these are
bugs in the verifying code itself (such as use of deleted SCEVs and
SCEVCouldNotCompute's).

I think it all gives enough justification for its removal.

[nikic]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

2203

Accidentally also dropped MSSA verification?

[mkazantsev]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

2203

Yeah, thanks for pointing out.

[nikic]

LGTM

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

2162

You can drop the ifndef -- we generally don't place VerifyMemorySSA behind NDEBUG, because you need to enable it via a flag anyway.

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

 #define DEBUG_TYPE "indvars"
 
 STATISTIC(NumWidened     , "Number of indvars widened");
 STATISTIC(NumReplaced    , "Number of exit values replaced");
 STATISTIC(NumLFTR        , "Number of loop exit tests replaced");
 STATISTIC(NumElimExt     , "Number of IV sign/zero extends eliminated");
 STATISTIC(NumElimIV      , "Number of congruent IVs eliminated");
 
-// Trip count verification can be enabled by default under NDEBUG if we
-// implement a strong expression equivalence checker in SCEV. Until then, we
-// use the verify-indvars flag, which may assert in some cases.
-static cl::opt<bool> VerifyIndvars(
-    "verify-indvars", cl::Hidden,
-    cl::desc("Verify the ScalarEvolution result after running indvars. Has no "
-             "effect in release builds. (Note: this adds additional SCEV "
-             "queries potentially changing the analysis result)"));
-
 static cl::opt<ReplaceExitVal> ReplaceExitValue(
     "replexitval", cl::Hidden, cl::init(OnlyCheapRepl),
     cl::desc("Choose the strategy to replace exit value in IndVarSimplify"),
     cl::values(
         clEnumValN(NeverRepl, "never", "never replace exit value"),
         clEnumValN(OnlyCheapRepl, "cheap",
                    "only replace exit value when the cost is cheap"),
         clEnumValN(
bool IndVarSimplify::run(Loop *L) {
   //  - We depend on having a preheader; in particular,
   //    Loop::getCanonicalInductionVariable only supports loops with preheaders,
   //    and we're in trouble if we can't find the induction variable even when
   //    we've manually inserted one.
   //  - LFTR relies on having a single backedge.
   if (!L->isLoopSimplifyForm())
     return false;
 
-#ifndef NDEBUG
-  // Used below for a consistency check only
-  // Note: Since the result returned by ScalarEvolution may depend on the order
-  // in which previous results are added to its cache, the call to
-  // getBackedgeTakenCount() may change following SCEV queries.
-  const SCEV *BackedgeTakenCount;
-  if (VerifyIndvars)
-    BackedgeTakenCount = SE->getBackedgeTakenCount(L);
-#endif
-
   bool Changed = false;
   // If there are any floating-point recurrences, attempt to
   // transform them to use integer recurrences.
   Changed |= rewriteNonIntegerIVs(L);
 
   // Create a rewriter object which we'll use to transform the code with.
   SCEVExpander Rewriter(*SE, DL, "indvars");
 #ifndef NDEBUG
#endif
   // rewriteFirstIterationLoopExitValues does not rely on the computation of
   // trip count and therefore can further simplify exit values in addition to
   // rewriteLoopExitValues.
   Changed |= rewriteFirstIterationLoopExitValues(L);
 
   // Clean up dead instructions.
   Changed |= DeleteDeadPHIs(L->getHeader(), TLI, MSSAU.get());
 
+#ifndef NDEBUG
   // Check a post-condition.
   assert(L->isRecursivelyLCSSAForm(*DT, *LI) &&
          "Indvars did not preserve LCSSA!");
-
-  // Verify that LFTR, and any other change have not interfered with SCEV's
-  // ability to compute trip count.  We may have *changed* the exit count, but
-  // only by reducing it.
-#ifndef NDEBUG
-  if (VerifyIndvars && !isa<SCEVCouldNotCompute>(BackedgeTakenCount)) {
-    SE->forgetLoop(L);
-    const SCEV *NewBECount = SE->getBackedgeTakenCount(L);
-    if (SE->getTypeSizeInBits(BackedgeTakenCount->getType()) <
-        SE->getTypeSizeInBits(NewBECount->getType()))
-      NewBECount = SE->getTruncateOrNoop(NewBECount,
-                                         BackedgeTakenCount->getType());
-    else
-      BackedgeTakenCount = SE->getTruncateOrNoop(BackedgeTakenCount,
-                                                 NewBECount->getType());
-    assert(!SE->isKnownPredicate(ICmpInst::ICMP_ULT, BackedgeTakenCount,
-                                 NewBECount) && "indvars must preserve SCEV");
-  }
   if (VerifyMemorySSA && MSSAU)
     MSSAU->getMemorySSA()->verifyMemorySSA();
 #endif
 
   return Changed;
 }
 
 PreservedAnalyses IndVarSimplifyPass::run(Loop &L, LoopAnalysisManager &AM,
                                           LoopStandardAnalysisResults &AR,
                                           LPMUpdater &) {