[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