[PATCH] D127960: [InstCombine] Push freeze through recurrence phi

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/freeze.ll:746
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INIT_FR:%.*]] = freeze i32 [[INIT:%.*]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
----------------
nlopes wrote:
> This case is annoying because if undef didn't exist*, we could get rid of the freeze here altogether.
> The reason is that if %init=poison, then %init.fr becomes any value we want. 
> Since the add below is nuw, we can pick a convenient value that makes the addition overflow and thus poison.
> Thus, if %init is poison, %cond becomes poison, and the loop triggers UB.
> 
> * The argument still applies to full undef values. But it does not for values that are partially undef (e.g., (undef & 1) | (x & ~1)). We've never enabled partial undefs in Alive2 because they are quite expensive and we only had some artificial examples on when they are needed. But this example shows it well.
> We can decide to ignore partial undefs here, or at least leave a FIXME for that day when we remove undef from LLVM 🤣
I think there are two aspects here: The first is the dropped nowrap flags, and the other the freeze on the start value.

If we drop support for undef, then we'll automatically start preserving nowrap flags here (I checked that this happens if we replace isGuaranteedNotToBeUndefOrPoison with isGuaranteedNotToBePoison, which can exploit poison propagation + branch on poison UB).

The argument you make that would allow dropping the freeze entirely is a lot more subtle, and depends on the exact instructions being used. I think it would not work if the increment is a variable, for example (because it could be zero, in which case there might not be a choice of frozen value that results in a branch on poison). I tried to demonstrate this with https://alive2.llvm.org/ce/z/w9eiFn, but alive happily accepts this. I would have thought that `init = poison`, `step = 0` is an obvious counter-example, where `tgt` causes branch on poison, while `src` is well-defined.


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

https://reviews.llvm.org/D127960