[PATCH] D18867: [IndVarSimplify] Eliminate zext of a signed IV when the IV is known to be non-negative
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 16 00:06:59 PDT 2016
sanjoy added a comment.
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).
More information about the llvm-commits