[llvm-commits] [llvm] r123131 - in /llvm/trunk: include/llvm/Analysis/ScalarEvolution.h lib/Analysis/ScalarEvolution.cpp

[Dan Gohman]

On Jan 15, 2011, at 6:44 PM, Chris Lattner wrote:

> On Jan 11, 2011, at 11:36 AM, Dan Gohman wrote:
> 
>> 
>> For the unsigned case, this is still really confusing. Take the most innocent-
>> looking expression like "X - 1" for example. This turns into X + UINT_MAX,
>> which has unsigned overflow.
>> 
>> For the signed case, setting HasNSW requires proving that the RHS is not
>> UINT_MIN.
>> 
>> A side note here is that the nsw and nuw concepts don't cover every
>> interesting kind of overflow. There are many situations where one operand
>> of an add is unsigned and the other operand is signed, but these are hard
>> to represent since they're asymmetric.
> 
> Hrm, I think I see what you mean.  The code in getMinusSCEVForExitTest ends up producing an AddRec from the getMinusExpr (it *knows* it will be an addrec) that counts down to (but doesn't pass) zero.  Do you think a better way to handle this is to just call getMinusSCEV, get the addrec, then ask for the same addrec in non-overflowing form?

There is currently no way in SCEV's expression language to express an
unsigned addrec that counts down and doesn't wrap.

[...]
> I don't think that matters for this code.  The code is carefully set up so that it has an AddRec that doesn't overflow "somehow", and it knows which direction it is stepping, and it knows that it gets to a specific integer value.  If it "strides past" this value, the only way for the program to be defined is if it wraps all the way back around to the value.  Just knowing that the addrec has cannot wrap *somewhere* should be enough, we're not doing an analysis in its original domain.

For example, {-3,+,1}<L>, and assume L's trip count is 6. It counts to a specific
known value. It doesn't wrap back to itself. It doesn't have signed overflow. It
does have unsigned overflow.

>>>>> +  // If the signs of the strides differ, then the negative stride is counting
>>>>> +  // down to the positive stride.
>>>>> +  if (LHSStride->getValue().isNegative() != RHSStride->getValue().isNegative()){
>>>>> +    if (RHSStride->getValue().isNegative())
>>>>> +      std::swap(LHS, RHS);
>>>>> +  } else {
>>>>> +    // If LHS's stride is smaller than RHS's stride, then "b" must be less than
>>>>> +    // "a" and "b" is RHS is counting up (catching up) to LHS.  This is true
>>>>> +    // whether the strides are positive or negative.
>>>>> +    if (RHSStride->getValue().slt(LHSStride->getValue()))
>>>>> +      std::swap(LHS, RHS);
>>>>> +  }
>>>>> +    
>>>>> +  return SE.getMinusSCEV(LHS, RHS, true /*HasNUW*/);
>>>> 
>>>> This isn't safe if the addrecs are nsw but not nuw (following the suspicion
>>>> above).
>>> 
>>> How so?
>> 
>> 
>> Suppose LHS is {X,+,-1}<L> and RHS is {-4,+,1}<L>. Both could legitimately have
>> NSW and not NUW. In the sixth iteration of the loop, LHS is X-6 and RHS is 1.
>> 
>> X-6 - 1 => X-6 + -1 => X-6 + UINT_MAX, which has unsigned overflow if X != 6.
> 
> Maybe (probably) I'm seriously misunderstanding something, but in this case, we'll end up getting a new SCEV {X+4,+,-2}.  Setting HasNUW on is resultant addrec tells the clients that the *result of the addrec* (not all the subcomputations that happen) will not wrap past zero. This allows the client to know that the crossover point happens at x/2+2.

HasNUW means that none of the intermediate computations overflow. The
result of the addrec may be evaluated each iteration of the loop, so it
can't have overflow at any point along the way.

Dan