[PATCH] D105392: [InstCombine] Add optimization to prevent poison from being propagated.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3519
+InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
+  // Try to push freeze through instructions that propagate but don't produce
+  // poison as far as possible.  If operand of freeze follows three conditions
----------------
Hm, why would multiple `freeze` would be inserted?
The transform i envisioned is:
```
%a = <whatever> %x
%a.frozen = freeze %a
%b = <whatever> %a.frozen
%c = <whatever> %a
 =>
%a = <whatever> %x
%a.frozen = freeze %a
%b = <whatever> %a.frozen
%c = <whatever> %a.frozen
 =>
%x.frozen = freeze %x
%a.frozen = <whatever> %x.frozen
%b = <whatever> %a.frozen
%c = <whatever> %a.frozen
```
My question being, why do we not to change other users of `%a` to use `%a.frozen`?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3563-3564
+
+  // We already folded "freeze (phi const, x)" to "phi const, (freeze x)"
+  assert(!isa<PHINode>(OrigOp));
+
----------------
I do not expect that this assertion will hold, `foldOpIntoPhi()` has a number of bailouts.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105392