[PATCH] D74810: [IndVarSimply] Fix assert/release build difference.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur created this revision.
Meinersbur added reviewers: reames, jeroen.dobbelaere.
Meinersbur added a project: LLVM.
Herald added subscribers: javed.absar, hiraditya.

In builds with assertions enabled (!NDEBUG), IndVarSimplify has an additional query to ScalarEvolution which may change future SCEV queries since it fills the internal cache differently. The result is actually only used when `-verify-indvars`. We fix the issue by only calling `SE->getBackedgeTakenCount(L)` if `-verify-indvars` is enabled such that only `-verify-indvars` shows the behavior, but not debug builds themselves. Also add a remark to the description of `-verify-indvars` about this behavior.

Fixes llvm.org/PR44815


https://reviews.llvm.org/D74810

Files:
  llvm/lib/Transforms/Scalar/IndVarSimplify.cpp


Index: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -102,8 +102,10 @@
 // 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"));
+    "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),
@@ -2663,7 +2665,11 @@
 
 #ifndef NDEBUG
   // Used below for a consistency check only
-  const SCEV *BackedgeTakenCount = SE->getBackedgeTakenCount(L);
+  // 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 =
+      VerifyIndvars ? SE->getBackedgeTakenCount(L) : nullptr;
 #endif
 
   bool Changed = false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74810.245309.patch
Type: text/x-patch
Size: 1399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200219/a92d534e/attachment.bin>