[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
----------------
hyeongyukim wrote:
> lebedev.ri wrote:
> > 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`?
> In terms of correctness, it is still fine.
> However, `%a`and `%a.frozen` have different semantics, and the optimization performed in `%a` may not be performed in `%a.frozen`.
> I thought it might not be good for the frozen values to spread too much, so single-use `%a` is considered only.
> 
That is what i thought, too, but that's not what the comment says.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105392