[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#2465056 <https://reviews.llvm.org/D93065#2465056>, @congzhe wrote:

> In D93065#2463069 <https://reviews.llvm.org/D93065#2463069>, @nlopes wrote:
>
>> 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).
>
> Can I just make sure my understanding is correct -- so when we check the SCEV of `some_bool && i < limit;`  we do recursion backwards on this select instruction (after this patch) or on an AND instruction (before this patch). If we choose the freeze approach, we'll do recursion on the AND instruction and eventually hit a freeze instruction which SCEV does not know how to handle, hence SCEV will just return `CouldNotCompute`?

It's more than just teaching SCEV how to handle freeze. A freeze instruction destroys UB and therefore not even the most precise SCEV would be able to recover the information that is lost. select retains more information/UB.
So if SCEV requires poison from the `add nsw` to e.g. determine the number of iterations, we would miss out with the freeze implementation. It's a hypothetical loss that I can't quantify without testing. My argument for using select is just that it makes the IR smaller and therefore preferable. Since it has the nice extra benefit of keeping more UB, then great, but not the main argument.


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

https://reviews.llvm.org/D93065