[PATCH] D141850: [SCEV] Preserve divisibility and min/max information in applyLoopGuards
Alon Kom via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 20 06:00:22 PST 2023
alonkom marked 4 inline comments as done.
alonkom added a comment.
@mkazantsev Let me know if you have any other comments
================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:15031
+ RHS = MinMax->getOperand(1);
+ return isa<SCEVConstant>(RHS);
+ }
----------------
mkazantsev wrote:
> 1. If I remember correctly, SCEV operands are always sorted by kind. It means that constants always go first. `RHS` can't be a constant because then `LHS` is also a constant and the whole thing should be folded away. So it should be trivially `false`, and I guess this code part isn't covered by tests if it wasn't caught.
> 2. Usually matcher methods aren't supposed to change operands if they return `false`. In your case it doesn't matter, but generally better to follow the standard practice and do all checks before modifying the operands.
>
# Nice catch! It didn't hurt the divisibility info, but some min/max info was lost. I Fixed this and added a unit-test since the lit only checks the divisibility.
# Done.
================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:15191
+ case CmpInst::ICMP_SLT: {
+ auto ModifiedRHS = getMinusSCEV(RHS, getOne(RHS->getType()));
+ ModifiedRHS =
----------------
mkazantsev wrote:
> alonkom wrote:
> > mkazantsev wrote:
> > > How do you account for `RHS` being `SINT_MIN` and similar cases here?
> > I wonder how this worked before my patch.
> > if we have assume (N < SINT_MIN) then the following SCEV was generated:
> > smin(N, SINT_MIN - 1) which would overflow.
> >
> >
> Well, if there is a check somewhere that `RHS` is non-negative, then it makes sense. Otherwise it's possibly broken. :)
>
Anyway, the change I've made only applies to non-negative values
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141850/new/
https://reviews.llvm.org/D141850
More information about the llvm-commits
mailing list