[llvm] r222093 - ScalarEvolution: HowFarToZero was wrongly using signed division

[Tobias Grosser]

On 16.11.2014 17:02, Sebastian Pop wrote:
> On Sun, Nov 16, 2014 at 4:19 AM, Tobias Grosser <tobias at grosser.es> wrote:
>> On 16.11.2014 08:30, David Majnemer wrote:
>>>
>>> Author: majnemer
>>> Date: Sun Nov 16 01:30:35 2014
>>> New Revision: 222093
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=222093&view=rev
>>> Log:
>>> ScalarEvolution: HowFarToZero was wrongly using signed division
>>>
>>> HowFarToZero was supposed to use unsigned division in order to calculate
>>> the backedge taken count.  However, SCEVDivision::divide performs signed
>>> division.  Unless I am mistaken, no users of SCEVDivision actually want
>>> signed arithmetic: switch to udiv and urem.
>
> findArrayDimensionsRec requires signed division in case Step is negative:
>
>      // Normalize the terms before the next call to findArrayDimensionsRec.
>      const SCEV *Q, *R;
>      SCEVDivision::divide(SE, Term, Step, &Q, &R);
>
>>>
>>> This fixes PR21578.
>>
>>
>> This commit breaks a delinearization test case in polly.
>>
>
> Tobi, could you say which testcase broke?
FAIL: Polly :: Isl/Ast/simple-run-time-condition.ll (160 of 391)


; CHECK: ({{(q == 100 && o <= 0|o <= 0 && q == 100)}})
          ^
<stdin>:8:5: note: scanning from here
if ((o >= 1 && q == 0) || (o <= 0 && q == -2305843009213693852) ? 1 : 0)

>> I copied Sebastian who originally wrote the divide operation exactly as part
>> of our delinearization analysis. He is possibly the best person to comment
>> on this.
>>
>
> Most likely we will need to duplicate the code of SCEVDivision into a
> SCEVUDivide.
> The code that reused SCEVDivide contains this comment that expects SCEVDivision
> to perform an unsigned division:

This is my feeling as well.

Cheers,
Tobias