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

[Andrew Trick]

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.

Thanks for reviewing!

-Andy

>> Modified:
>>    llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>> 
>> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=127591&r1=127590&r2=127591&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
>> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Mon Mar 14 12:28:02 2011
>> @@ -4978,15 +4978,6 @@
>>   const SCEV *Start = getSCEVAtScope(AddRec->getStart(), L->getParentLoop());
>>   const SCEV *Step = getSCEVAtScope(AddRec->getOperand(1), L->getParentLoop());
>> 
>> -  // If the AddRec is NUW, then (in an unsigned sense) it cannot be counting up
>> -  // to wrap to 0, it must be counting down to equal 0.  Also, while counting
>> -  // down, it cannot "miss" 0 (which would cause it to wrap), regardless of what
>> -  // the stride is.  As such, NUW addrec's will always become zero in
>> -  // "start / -stride" steps, and we know that the division is exact.
>> -  if (AddRec->getNoWrapFlags(SCEV::FlagNUW))
>> -    // FIXME: We really want an "isexact" bit for udiv.
>> -    return getUDivExpr(Start, getNegativeSCEV(Step));
>> -
>>   // For now we handle only constant steps.
>>   //
>>   // TODO: Handle a nonconstant Step given AddRec<NUW>. If the
>> @@ -5002,15 +4993,28 @@
>>   // For negative steps (counting down to zero):
>>   //   N = Start/-Step
>>   // First compute the unsigned distance from zero in the direction of Step.
>> -  const SCEV *Distance = StepC->getValue()->getValue().isNonNegative() ?
>> -    getNegativeSCEV(Start) : Start;
>> +  bool CountDown = StepC->getValue()->getValue().isNegative();
>> +  const SCEV *Distance = CountDown ? Start : getNegativeSCEV(Start);
>> 
>>   // Handle unitary steps, which cannot wraparound.
>> -  if (StepC->getValue()->equalsInt(1))      // 1*N = -Start (mod 2^BW), so:
>> -    return Distance;                        //   N = -Start (as unsigned)
>> -
>> -  if (StepC->getValue()->isAllOnesValue())  // -1*N = -Start (mod 2^BW), so:
>> -    return Distance;                        //    N = Start (as unsigned)
>> +  // 1*N = -Start; -1*N = Start (mod 2^BW), so:
>> +  //   N = Distance (as unsigned)
>> +  if (StepC->getValue()->equalsInt(1) || StepC->getValue()->isAllOnesValue())
>> +    return Distance;
>> +
>> +  // If the recurrence is known not to wraparound, unsigned divide computes the
>> +  // back edge count. We know that the value will either become zero (and thus
>> +  // the loop terminates), that the loop will terminate through some other exit
>> +  // condition first, or that the loop has undefined behavior.  This means
>> +  // we can't "miss" the exit value, even with nonunit stride.
>> +  //
>> +  // FIXME: Prove that loops always exhibits *acceptable* undefined
>> +  // behavior. Loops must exhibit defined behavior until a wrapped value is
>> +  // actually used. So the trip count computed by udiv could be smaller than the
>> +  // number of well-defined iterations.
>> +  if (AddRec->getNoWrapFlags(SCEV::FlagNW))
>> +    // FIXME: We really want an "isexact" bit for udiv.
>> +    return getUDivExpr(Distance, CountDown ? getNegativeSCEV(Step) : Step);
>> 
>>   // Then, try to solve the above equation provided that Start is constant.
>>   if (const SCEVConstant *StartC = dyn_cast<SCEVConstant>(Start))
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>