[PATCH] D93065: [InstCombine] Disable optimizations of select instructions that causes propagation of poison values

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

In D93065#2461517 <https://reviews.llvm.org/D93065#2461517>, @congzhe wrote:

> In D93065#2461196 <https://reviews.llvm.org/D93065#2461196>, @nlopes wrote:
>
>> Using freeze loses information (if some of the inputs was poison). Plus It requires an extra op.
>> If we canonicalize around select there's no loss of information and it's just 1 instruction.
>>
>> The disadvantage is that then we have 2 ways or doing boolean ANDs/ORs. Though most analyses can be patched easily, as most LLVM analyses' results are of the form "x has property foo unless it's poison". So for those analyses using and/or or select is the same (as the only difference between these is propagation of poison).
>> Other analyses/optimization can learn about select as needed.
>
> Thank you for raising up the good point! I understand that we lose information by preventing poison values from propagation using freeze. But I'm unclear what would be the side effect or problem with that? I'd appreciate it if you could clarify a bit, thanks!

In practice, probably not a lot. But it may have implications for loop optimization, like:

  for (i=0; some_bool && i < limit; ++i) {
  ...
  }

If you remove the poison from the `i+1 < limit` bit it may make the work of SCEV harder (or impossible; didn't think the example through carefully).
Another example is hoisting of sext(i32)->i64 out of loops. This is important for perf and it relies on poison to prove no wrapping and we can't destroy that. Probably most loop conditions don't have ANDs/ORs, hence why I say the impact in practice is small.


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

https://reviews.llvm.org/D93065