[PATCH] D66890: [IndVarSimplify] Do not use SCEV expander for IVCount in LFTR when possible.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 30 09:40:22 PDT 2019


reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

See inline review comment

If you decide to revise, please do the following:

1. Land your tests in a precommit, then rebase
2. Land the auto-generation of the one effected test, then rebase
3. Provide a clear comment (or pointer to particular test is fine) where SCEVExpander goes wrong.  I might be able to make better suggestions on approach.



================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2403
+        Limit = Cmp->getOperand(1);
+      if (SE->getSCEV(Limit) == IVLimit)
+        return Limit;
----------------
I don't believe this is sound.  

The problem is that SCEV can map multiple expressions to the same SCEV, but that does not imply that all of them are equivalent.  

Given:
%v1  = add nsw %a, %b -> SCEV A
%v2 = add nuw %a, %b -> SCEV B
(i.e pretend SCEV dropped *all* flags in it's construction)
This would replace all uses of %v2 with %v1 which is incorrect.


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

https://reviews.llvm.org/D66890





More information about the llvm-commits mailing list