[PATCH] D18867: [IndVarSimplify] Eliminate zext of a signed IV when the IV is known to be non-negative

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

Hi!

Firstly, apologies for letting this set for so long.

In http://reviews.llvm.org/D18867#458868, @lihuang wrote:

> I looked at the threads on llvm-dev discussing the no-wrap flag
>  propagation issues. It seems like this has been a problem for a while
>  and there is not a clear agreement on when and how to propagate
>  no-wrap flags from instructions to corresponding SCEVs.
>
> The problem in this case could be easily solved by propagating "nsw"
>  flag of "%add = add nsw i32 %i.0, 2" to it's SCEV. It's not possible
>  to infer the "nsw" flag for this SCEV. As %i.0 has range
>  [0,-2147483648), "%i.0 + 2" could cause signed-overflow.
>
> Sanjoy, I see you are actively contributing to SCEV. Do you know the
>  community's plan on strengthening the no-wrap flag propagation? The
>  tests I added here might not serve as a very good motivation for the
>  effort but I think this will generally benefit SCEV and some
>  optimizations depending on SCEV.


You're on the right track -- in many cases SCEV cannot propagate
no-wrap flags from the instruction to the SCEV expression.  However,
the test cases as written don't work with `isKnownPredicate` because
the loops are not in canonical form, and not because of some
fundamental reason.  If I run `-loop-rotate` on them (possibly
followed by `-loop-simplifycfg`) then your patch minus the
`isKnownNonNegative` bit does the right thing.

So I'd suggest checking if (in your pass pipeline) you're
canonicalizing the loops before passing them to `-indvars`.  If you
are and are still facing issues, then it is fine to use
`isKnownNonNegative` here (since there _is_ a shortcoming in
`ScalarEvolution::isKnownPredicate`) with test cases that
fundamentally require `isKnownNonNegative` (and don't work with
`ScalarEvolution::isKnownPredicate` even on rotated loops).


http://reviews.llvm.org/D18867