[PATCH] D77868: [InstSimplify] fold select of bools using bitwise logic

[Nuno Lopes via Phabricator via llvm-commits]

nlopes accepted this revision.
nlopes added a comment.
This revision is now accepted and ready to land.

In D77868#1977808 <https://reviews.llvm.org/D77868#1977808>, @spatel wrote:

> >   // select Cond, T, false --> Cond & T
> >   if (match(F, m_ZeroInt()))
> >     return SimplifyAndInst(Cond, T, Q);
>
> I still do not see how this can go wrong in practice. If InstSimplify can prove that T is poison, then doesn't it always manifest that knowledge by saying T is undef?
>  Looking at it another way (see if you can spot a logic flaw):
>
> 1. Assume T is poison.
> 2. Assume InstSimplify fails to simplify T to undef or constant.
> 3. For poison T' (either T or some other poisoned value) to leak through as the result, InstSimplify must prove that Cond & T' == T'.
> 4. So InstSimplify must prove that Cond == true or Cond == T'.
> 5. If Cond == T', there's no problem (if Cond is poison, the select is poison).
> 6. Therefore -- for there to be a problem -- we must prove that Cond is true.
> 7. If we can prove that Cond is true, then that guarantees that value T' can't depend on Cond (nothing depends on Cond - it simplifies to constant true).


I agree with your reasoning, and that this patch is correct.
The crucial piece of information that was missing, for me at least, is that SimplifyAndInst() doesn't create an 'and' instruction if it fails to simplify (it just returns null). Also, it doesn't create new instructions (it could also create a mask or smth like that, but that doesn't happen either).
Given this, LGTM, thanks.


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

https://reviews.llvm.org/D77868