[PATCH] D145008: [ControlHeightReduction] Don't combine a "poison" branch

[Kazu Hirata via Phabricator via llvm-commits]

kazu added a comment.

In D145008#4162710 <https://reviews.llvm.org/D145008#4162710>, @nikic wrote:

> In D145008#4162691 <https://reviews.llvm.org/D145008#4162691>, @kazu wrote:
>
>> In D145008#4162517 <https://reviews.llvm.org/D145008#4162517>, @nikic wrote:
>>
>>> In D145008#4162106 <https://reviews.llvm.org/D145008#4162106>, @davidxl wrote:
>>>
>>>> In D145008#4160793 <https://reviews.llvm.org/D145008#4160793>, @nikic wrote:
>>>>
>>>>> The value may be poison even if it's not literally poison. The problem seems to be that the condition gets swapped. CHR generates `select i1 poison, i1 %arg, i1 false` rather than the correct `select i1 %arg, i1 poison, i1 false`. Either that swapping should not occur (if it's not important to what CHR is doing) or the condition needs to be frozen.
>>>>
>>>> Correct -- kazu@ brought it up before sending the patch as well -- is there existing interface to query for propagated poison?
>>>
>>> There is `isGuaranteedNotToBeUndefOrPoison()`, but here you want to use `freeze` as a fallback.
>>
>> Do you mean something like this in pseudo code?
>>
>>   if (isGuaranteedNotToBeUndefOrPoison(Cond))
>>     Use Cond As is;
>>   else
>>     Use freeze to prevent the potential poison from propagating further;
>
> Yes, see the existing code in addToMergedCondition() for handling branch to select conversion. It needs to be extended for the branch case if the conditions are swapped.

OK.  Now, what if `Cond` is guaranteed to be a `poison`?  Should we still transform the code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145008