[llvm] d1e880a - [SCEV] Enable verification in LoopPM

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 7 00:46:30 PST 2022


Author: Nikita Popov
Date: 2022-03-07T09:46:20+01:00
New Revision: d1e880acaa6f096618fa09d18daf17b9300558fc

URL: https://github.com/llvm/llvm-project/commit/d1e880acaa6f096618fa09d18daf17b9300558fc
DIFF: https://github.com/llvm/llvm-project/commit/d1e880acaa6f096618fa09d18daf17b9300558fc.diff

LOG: [SCEV] Enable verification in LoopPM

Currently, we hardly ever actually run SCEV verification, even in
tests with -verify-scev. This is because the NewPM LPM does not
verify SCEV. The reason for this is that SCEV verification can
actually change the result of subsequent SCEV queries, which means
that you see different transformations depending on whether
verification is enabled or not.

To allow verification in the LPM, this limits verification to
BECounts that have actually been cached. It will not calculate
new BECounts.

BackedgeTakenInfo::getExact() is still not entirely readonly,
it still calls getUMinFromMismatchedTypes(). But I hope that this
is not problematic in the same way. (This could be avoided by
performing the umin in the other SCEV instance, but this would
require duplicating some of the code.)

Differential Revision: https://reviews.llvm.org/D120551

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ScalarEvolution.h
    llvm/lib/Analysis/ScalarEvolution.cpp
    llvm/lib/Transforms/Scalar/LoopPassManager.cpp
    llvm/test/Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
    llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 2b5105ae90e82..cea8f1a756a79 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -67,6 +67,8 @@ class Type;
 class Value;
 enum SCEVTypes : unsigned short;
 
+extern bool VerifySCEV;
+
 /// This class represents an analyzed expression in the program.  These are
 /// opaque objects that the client is not allowed to do much with directly.
 ///

diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 7d2d3381230ba..338117348b0a1 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -142,6 +142,9 @@ STATISTIC(NumTripCountsNotComputed,
 STATISTIC(NumBruteForceTripCountsComputed,
           "Number of loops with trip counts computed by force");
 
+// FIXME: Enable this with EXPENSIVE_CHECKS when the test suite is clean.
+bool llvm::VerifySCEV = false;
+
 static cl::opt<unsigned>
 MaxBruteForceIterations("scalar-evolution-max-iterations", cl::ReallyHidden,
                         cl::ZeroOrMore,
@@ -150,9 +153,8 @@ MaxBruteForceIterations("scalar-evolution-max-iterations", cl::ReallyHidden,
                                  "derived loop"),
                         cl::init(100));
 
-// FIXME: Enable this with EXPENSIVE_CHECKS when the test suite is clean.
-static cl::opt<bool> VerifySCEV(
-    "verify-scev", cl::Hidden,
+static cl::opt<bool, true> VerifySCEVOpt(
+    "verify-scev", cl::Hidden, cl::location(VerifySCEV),
     cl::desc("Verify ScalarEvolution's backedge taken counts (slow)"));
 static cl::opt<bool> VerifySCEVStrict(
     "verify-scev-strict", cl::Hidden,
@@ -13359,8 +13361,14 @@ void ScalarEvolution::verify() const {
     if (!ReachableBlocks.contains(L->getHeader()))
       continue;
 
-    auto *CurBECount = SCM.visit(
-        const_cast<ScalarEvolution *>(this)->getBackedgeTakenCount(L));
+    // Only verify cached BECounts. Computing new BECounts may change the
+    // results of subsequent SCEV uses.
+    auto It = BackedgeTakenCounts.find(L);
+    if (It == BackedgeTakenCounts.end())
+      continue;
+
+    auto *CurBECount =
+        SCM.visit(It->second.getExact(L, const_cast<ScalarEvolution *>(this)));
     auto *NewBECount = SE2.getBackedgeTakenCount(L);
 
     if (CurBECount == SE2.getCouldNotCompute() ||

diff  --git a/llvm/lib/Transforms/Scalar/LoopPassManager.cpp b/llvm/lib/Transforms/Scalar/LoopPassManager.cpp
index 3238b88f59ec3..d20d275ea60c4 100644
--- a/llvm/lib/Transforms/Scalar/LoopPassManager.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopPassManager.cpp
@@ -309,12 +309,12 @@ PreservedAnalyses FunctionToLoopPassAdaptor::run(Function &F,
 
 #ifndef NDEBUG
     // LoopAnalysisResults should always be valid.
-    // Note that we don't LAR.SE.verify() because that can change observed SE
-    // queries. See PR44815.
     if (VerifyDomInfo)
       LAR.DT.verify();
     if (VerifyLoopInfo)
       LAR.LI.verify(LAR.DT);
+    if (VerifySCEV)
+      LAR.SE.verify();
     if (LAR.MSSA && VerifyMemorySSA)
       LAR.MSSA->verifyMemorySSA();
 #endif

diff  --git a/llvm/test/Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll b/llvm/test/Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
index b497dd0d974f3..589caa765ada4 100644
--- a/llvm/test/Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
+++ b/llvm/test/Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
@@ -1,18 +1,13 @@
-; RUN: opt -indvars -stats -disable-output < %s 2>&1 | FileCheck %s --check-prefix=STATS
-; RUN: opt -indvars -S < %s | FileCheck %s --check-prefix=IR
-; REQUIRES: asserts
+; RUN: opt -indvars -S < %s | FileCheck %s
 
 ; Check that IndVarSimplify's result is not influenced by stray calls to
 ; ScalarEvolution in debug builds. However, -verify-indvars may still do
 ; such calls.
 ; llvm.org/PR44815
 
-; STATS: 1 scalar-evolution - Number of loops with trip counts computed by force
-; STATS: 2 scalar-evolution - Number of loops with predictable loop counts
-
 ; In this test, adding -verify-indvars causes %tmp13 to not be optimized away.
-; IR-LABEL: @foo
-; IR-NOT:   phi i32
+; CHECK-LABEL: @foo
+; CHECK-NOT:   phi i32
 
 target triple = "x86_64-unknown-linux-gnu"
 

diff  --git a/llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll b/llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll
index 45555323d7617..ab649f11472fd 100644
--- a/llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll
+++ b/llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -passes='loop(indvars),verify<scalar-evolution>' %s | FileCheck %s
+; RUN: opt -S -indvars -verify-scev %s | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"
 target triple = "x86_64-unknown-linux-gnu"
 


        


More information about the llvm-commits mailing list