[PATCH] SCEV should use NSW to get trip count for loop with nonunit stride.

[Dan Gohman]

Hi Andy,

The only thing I can think of is that if %n is INT4_MAX, code might be
trying to assume that this will be an infinite loop, even with nsw,
because starting at 0 and stepping by 2 will never produce a value
which is less than INT4_MAX. In a loop without other side effects the
"poison" of nsw never gets a chance to actuate.

However, I think the flaw in this reasoning is that poison values
start out as fully-unconstrained undef values, so the add can produce
a value which behaves as if the least significant bit is set, even
though that wouldn't otherwise be possible. That means the loop isn't
guaranteed to loop forever.

So yes, I agree that the testcase is wrong.

Dan

On Fri, Oct 18, 2013 at 9:43 AM, Andrew Trick <atrick at apple.com> wrote:
> I'm not able to defend SCEV's current inability to handle loops with non-unit stride. In other cases, SCEV is rightly conservative in ignoring NSW, and I've been defending that approach, but this particular case keeps coming up and I don't have a good answer.
>
> For example:
> void foo(int n, int *x) {
>  for (int i = 0; i < n; i += 3) {
>    x[i] = i;
>    x[i+1] = i+1;
>    x[i+2] = i+2;
>  }
> }
>
> In ScalarEvolution::HowManyLessThans we ignore the NSW flag when considering whether the loop counter increment may skip the loop test. However, if signed-wrap occurs, the result of the loop test is undefined. Providing a loop trip count to downstream optimization seems harmless. The only assumption we've made is that if executing the loop causes the counter to wrap, then the loop test will fail. In practice the test may not fail, but the assumption is still valid.
>
> There is one cryptic test case (below) attempting to enforce the conservative behavior. But it doesn't reveal why this case is problematic. I haven't been able to pick up clues from old bugs either. I know several related bugs in this area have been fixed since this test case was added. My guess is that either this was test case was preemtive or the original problem was fixed elsewhere.
>
> ---
> ; Be careful with this one. If %n is INT4_MAX, %i.next will wrap. The nsw bit
> ; says that the result is undefined, but ScalarEvolution must respect that
> ; subsequent passes may result the undefined behavior in predictable ways.
> ; CHECK: Determining loop execution counts for: @nsw_step2
> ; CHECK: Loop %loop: Unpredictable backedge-taken count.
> ; CHECK: Loop %loop: Unpredictable max backedge-taken count.
> define void @nsw_step2(i4 %n) {
> entry:
>   %s = icmp sgt i4 %n, 0
>   br i1 %s, label %loop, label %exit
> loop:
>   %i = phi i4 [ 0, %entry ], [ %i.next, %loop ]
>   %i.next = add nsw i4 %i, 2
>   %t = icmp slt i4 %i.next, %n
>   br i1 %t, label %loop, label %exit
> exit:
>   ret void
> }