[llvm-commits] [llvm] r127591 - /llvm/trunk/lib/Analysis/ScalarEvolution.cpp

[Eli Friedman]

On Mon, Mar 14, 2011 at 6:35 PM, Andrew Trick <atrick at apple.com> wrote:
> On Mar 14, 2011, at 2:09 PM, Eli Friedman wrote:
>> On Mon, Mar 14, 2011 at 1:28 PM, Andrew Trick <atrick at apple.com> wrote:
>>> Author: atrick
>>> Date: Mon Mar 14 12:28:02 2011
>>> New Revision: 127591
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=127591&view=rev
>>> Log:
>>> HowFarToZero can compute a trip count as long as the recurrence has no-self-wrap.
>>
>> Have you read http://llvm.org/bugs/show_bug.cgi?id=8942#c3 ?  That
>> goes a bit into why depending on no-self-wrap requires some
>> infrastructure changes.
>>
>> -Eli
>
> If I understand your comments in PR8942, you have the same concern
> that I expressed in the FIXME with this commit. I cannot take credit for the
> comment above the FIXME, "If the recurrence is known not to wraparound".
> I copied that text from getMinusSCEVForExitTest() in preparation for
> removing that function.
>
> The problem you brought up needs to be addressed, and I don't have the
> answer yet. In the meantime, I'm working to clarify the underlying
> logic. That will make issues like this more obvious and help me fix
> existing bugs.
>
> As to the safety of this change, there are two things to consider:
>
> (1) NUW was already being used as an alias for no-self-wrap in some
> cases. I'm only making the logic explicit and avoiding the use of NUW
> unless it stricly applies.
>
> (2) The theoretical issue that you raise is still a problem for NUW
> recurences with non-unit stride, which could "miss" the exit test
> before "unnatural loop termination".
>
> I believe the resulting BECount satisfies the intention of
> ComputeBackedgeTakenCountFromExit:
> /// Compute the number of times the backedge of the specified loop will execute
> /// *if it exits via the specified block*.
>
> But I also realize other clients of SCEV may have a different
> interpretation. I could really use help identifying specific
> situations that could fail. Without that, I don't think I'll be able
> to come up with an infrastructure change that makes the analysis more
> robust.

Hmm... getBackedgeTakenCount and getMaxBackedgeTakenCount have
surprisingly few users.  One user of getBackedgeTakenCount is IndVars,
which will try to rewrite the exit condition in terms of the backedge
taken count.  It assumes that the count is both the exit count and the
maximum loop count of the loop.  On the other hand, LoopIdiomRecognize
doesn't really care what the max iteration count is (I'm not
completely sure it's safe for other reasons, but that isn't really
related).  I'm not sure I really follow what LSR is doing with the
backedge taken count.  LoopDeletion uses the MaxBackedgeTakenCount to
make sure a loop isn't infinite.  And I think that actually
exhaustively covers all the uses of the backedge counts.  So
basically, if we can't actually do an indvars transform, very little
actually looks at the backedge count directly.  Some places that use
LoopInfo::getTripCount() and friends could probably be changed to be
more aggressive, though.

-Eli