[PATCH] D145008: [ControlHeightReduction] Don't combine a "poison" branch
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 1 11:40:20 PST 2023
nikic added a comment.
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.
> The swap is triggered because of the profile data says it is profitable -- while it is itself a bug to deal with, it is independent to this fix.
How can the swap be profitable when flattening control flow into a select? Shouldn't it be independent of order at that point?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145008/new/
https://reviews.llvm.org/D145008
More information about the llvm-commits
mailing list