[PATCH] D11212: [SCEV] Apply NSW and NUW flags via poison value analysis

[Bjarke Hammersholt Roune]

broune added inline comments.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:4144
@@ +4143,3 @@
+      if (isLoopInvariant(OtherOp, AddRec->getLoop()) &&
+          isGuaranteedToExecuteForEveryIteration(BinOp, AddRec->getLoop()))
+        return Flags;
----------------
sanjoy wrote:
> broune wrote:
> > sanjoy wrote:
> > > I'm not sure that this is correct.  I think you need to prove that the instruction you used to justify UB if `V` was poison is what needs to execute on every loop iteration.
> > > 
> > > This is not a problem currently because `undefinedBehaviorIsGuaranteedIfFullPoison` only looks at a single basic block, but semantically, the following program will be problematic:
> > > 
> > > ```
> > > 
> > >  loop_header:
> > >    %x = add nsw %p, %q
> > >    ...
> > > 
> > >  outside_the_loop:
> > >    ;; no other uses of %x
> > >    store i32 42, GEP(@global, %x)
> > > 
> > > ```
> > > 
> > > You can conclude that the `%x` does not overflow in the last iteration, but that's it -- even if `%x` was poison in all the other iterations your program is well defined.
> > As you point out, UB is not guaranteed on poison in this example, so even if `undefinedBehaviorIsGuaranteedIfFullPoison` is made stronger by considering more basic blocks, it should still return false here, right?
> > 
> > 
> In that case why do we bother with `isGuaranteedToExecuteForEveryIteration`?
> 
> IOW, why not
> 
> ```
>   if (undefinedBehaviorIsGuaranteedIfFullPoison(BinOp))
>     return Flags;
> 
> ```
> 
That would prove that if BinOp is executed, then what it calculates will not wrap. There is then still the possibility that BinOp is not executed on a given iteration, in which case we have no information about wrapping of the SCEV for that iteration. Then we cannot apply the flag to the SCEV as other instructions in the loop that map to the same SCEV would then also get the flag on the shared SCEV, but we have not actually proven that the shared SCEV does not wrap.


http://reviews.llvm.org/D11212