[PATCH] D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:11518
+    // We have proven that IV can not overflow, remember that fact.
+    setNoWrapFlags(const_cast<SCEVAddRecExpr *>(IV), WrapType);
   }
----------------
nikic wrote:
> reames wrote:
> > nikic wrote:
> > > If we consider the `canIVOverflowOnLT()` case, the only thin this guarantees is that `RHS + Stride - 1` does not overflow, it doesn't make a direct statement about the addrec.
> > > 
> > > If the loop exits before reaching this exit (for simplicity: abnormal exit on first iteration), then I don't think we can really make any statement about the nowrap behavior of the addrec.
> > Right, but we know that the result of the expression being considered must reach a conditional branch, and that conditional branch must dominate the latch.  Given that, we know that if this condition did overflow, then the iteration of the loop would be UB unless we exit from some other exit before this one.  In either case, that seems to give an upper bound on the trip count which does imply that AddRec can't overflow in a well defined loop doesn't it?
> Ah yes, you're right. It does still provide an upper bound. I got this mixed up with concerns from D101722. I've played around with some examples and convinced myself that this is correct...
After thinking about it some more, there is still one case that may be problematic. What if the addrec starts out above the RHS (meaning the loop exits after the first iteration). I'm thinking something like this:

```
; Assume %start is larger than 200.
define void @test(i8 %start) {
entry:
  br label %loop

loop:
  %iv = phi i8 [ %start, %entry ], [ %iv.next, %loop ]
  %iv.next = add i8 %iv, 2
  call void @use_and_exit(i8 %iv.next)
  %cond = icmp ult i8 %iv.next, 200 
  br i1 %cond, label %loop, label %exit

exit:
  ret void
}

declare void @use_and_exit(i8)
```

I believe your change would now infer that `{2 + %start,+,2}` is `<nuw>`, but if `@use_and_exit()` exits beforehand that is not necessarily a given.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103844/new/

https://reviews.llvm.org/D103844