[PATCH] D92739: [ValueTracking] Branch on poison is UB

Sun Dec 6 23:29:26 PST 2020

aqjune added a comment.

In D92739#2436145 <https://reviews.llvm.org/D92739#2436145>, @nikic wrote:

> In D92739#2436141 <https://reviews.llvm.org/D92739#2436141>, @fhahn wrote:
>
>> I put up D80875 <https://reviews.llvm.org/D80875> a while ago which I think did something similar. Consensus at the time was that there still are some transforms that may introduce branches on undef/ poison and we should fix them before.
>
> I see... Unfortunately this is a bit of a chicken and egg problem. In this instance, my primary interest is to fix a poison handling bug in LSR, but if programUndefinedIfPoison() is artificially crippled, the fix has much more fallout than it should. Stronger poison logic allows us to fix poison bugs without unduly affecting optimization.

This boils down to making miscompilation fixes cheap..

Since the LSR bug is a real one, it has to be fixed without being blocked by those.
I'm not a fan of this solution, but what about adding a parameter to programUndefinedIfPoison that allows the function to return true when br/switch is given, and let LSR call it with the flag set?

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

https://reviews.llvm.org/D92739