[SCEV] Use isKnownNonZero() for SCEV verification

[nikic]

Summary

Report all cases where we can prove that the expressions are not the same, rather than just the cases where we know the exact constant delta.

In part this is intended to clarify the difference with verify-scev-strict mode, which reports cases where we cannot prove that the expressions are the same.

[goldstein.w.n]

llvm/lib/Analysis/ScalarEvolution.cpp

14055

I don't quite understand the new comment for "In VerifySCEVStrict mode...". Before it was clear; constants only. I don't see how !Delta->isZero() implies we are unable prove the expressions are the same but isKnownNonZero(Delta) doesn't.

[nikic]

llvm/lib/Analysis/ScalarEvolution.cpp

14055

The answer to "Is Delta zero?" can be "yes", "no" and "unknown". By default only "no" answers are verification failures. In VerifySCEVStrict mode "unknown" answers are verification failures as well (which is why they are almost certainly false positives).

llvm/lib/Analysis/ScalarEvolution.cpp

const SCEV *visitCouldNotCompute(const SCEVCouldNotCompute *Expr) {
       return SE.getCouldNotCompute();
     }
   };
 
   SCEVMapper SCM(SE2);
   SmallPtrSet<BasicBlock *, 16> ReachableBlocks;
   SE2.getReachableBlocks(ReachableBlocks, F);
 
-  auto GetDelta = [&](const SCEV *Old, const SCEV *New) -> const SCEV * {
+  auto GetNonZeroDelta = [&](const SCEV *Old, const SCEV *New) -> const SCEV * {
     if (containsUndefs(Old) || containsUndefs(New)) {
       // SCEV treats "undef" as an unknown but consistent value (i.e. it does
       // not propagate undef aggressively).  This means we can (and do) fail
       // verification in cases where a transform makes a value go from "undef"
       // to "undef+1" (say).  The transform is fine, since in both cases the
       // result is "undef", but SCEV thinks the value increased by 1.
       return nullptr;
     }
 
-    // Unless VerifySCEVStrict is set, we only compare constant deltas.
     const SCEV *Delta = SE2.getMinusSCEV(Old, New);
-    if (!VerifySCEVStrict && !isa<SCEVConstant>(Delta))
-      return nullptr;
-
+    // By default, only report cases where we can prove the expressions are not
+    // the same. In VerifySCEVStrict mode, report cases where we cannot prove
+    // that the expressions are the same.
+    if (VerifySCEVStrict ? !Delta->isZero() : SE2.isKnownNonZero(Delta))
       return Delta;
+    return nullptr;
   };
 
   while (!LoopStack.empty()) {
     auto *L = LoopStack.pop_back_val();
     llvm::append_range(LoopStack, *L);
 
     // Only verify BECounts in reachable loops. For an unreachable loop,
     // any BECount is legal.
while (!LoopStack.empty()) {
 
     if (SE.getTypeSizeInBits(CurBECount->getType()) >
         SE.getTypeSizeInBits(NewBECount->getType()))
       NewBECount = SE2.getZeroExtendExpr(NewBECount, CurBECount->getType());
     else if (SE.getTypeSizeInBits(CurBECount->getType()) <
              SE.getTypeSizeInBits(NewBECount->getType()))
       CurBECount = SE2.getZeroExtendExpr(CurBECount, NewBECount->getType());
 
-    const SCEV *Delta = GetDelta(CurBECount, NewBECount);
-    if (Delta && !Delta->isZero()) {
+    const SCEV *Delta = GetNonZeroDelta(CurBECount, NewBECount);
+    if (Delta) {
       dbgs() << "Trip Count for " << *L << " Changed!\n";
       dbgs() << "Old: " << *CurBECount << "\n";
       dbgs() << "New: " << *NewBECount << "\n";
       dbgs() << "Delta: " << *Delta << "\n";
       std::abort();
     }
   }
 
if (It == ExprValueMap.end() || !It->second.contains(KV.first)) {
       std::abort();
     }
 
     if (auto *I = dyn_cast<Instruction>(&*KV.first)) {
       if (!ReachableBlocks.contains(I->getParent()))
         continue;
       const SCEV *OldSCEV = SCM.visit(KV.second);
       const SCEV *NewSCEV = SE2.getSCEV(I);
-      const SCEV *Delta = GetDelta(OldSCEV, NewSCEV);
-      if (Delta && !Delta->isZero()) {
+      const SCEV *Delta = GetNonZeroDelta(OldSCEV, NewSCEV);
+      if (Delta) {
         dbgs() << "SCEV for value " << *I << " changed!\n"
                << "Old: " << *OldSCEV << "\n"
                << "New: " << *NewSCEV << "\n"
                << "Delta: " << *Delta << "\n";
         std::abort();
       }
     }
   }