[PATCH] D92367: [SCEV] Fix incorrect exit count calculated in error scope

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

Please reduce the test or give an elaborate explanation of the problem. So far it's not clear to me what was the problem and why it fixed this way.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7828
+  if (!IdxExpr || !IdxExpr->getLoop()->contains(L) || !IdxExpr->isAffine() ||
+      isLoopInvariant(IdxExpr, L) ||
       !isa<SCEVConstant>(IdxExpr->getOperand(0)) ||
----------------
You require the `IdxExpr` be from a containing loop, which automatically makes  `IdxExpr` invariant w.r.t. `L`. So this whole condition is just trivially false. Is that really what you want to make?


================
Comment at: llvm/test/Analysis/ScalarEvolution/incorrect-exit-count.ll:1
+; RUN: opt -analyze -scalar-evolution < %s | FileCheck %s
+
----------------
Please commit this test separately using auto-generated checks from ` utils/update_analyze_test_checks.py`. If you are addressing some bug from bugs.llvm.com, better name the test after this bug (like prXXXXX.ll).

Then, apply your changes and re-run  utils/update_analyze_test_checks.py on it. The diff will show what your patch has changed.

This test is very big. You could also try to use bugpoint tool to reduce the test (see https://llvm.org/docs/Bugpoint.html), because now I cannot figure what's going on here. Both test reduction (if possible) and explanatory comments (which block/loop are causing the problem and why) would be extremely useful to understand your fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92367/new/

https://reviews.llvm.org/D92367