[PATCH] D104569: [SimplifyCFG] Fix SimplifyBranchOnICmpChain to be undef/poison safe.

Sun Jun 20 15:26:09 PDT 2021

nlopes added a comment.

In D104569#2828921 <https://reviews.llvm.org/D104569#2828921>, @nikic wrote:

> I have a general policy question here: Do we care about miscompiles that are caused by undef, but not poison? This makes a difference for transforms like these, e.g. no freeze is required if the condition is first in a logical or (or at any position in bitwise or) if we only care about poison.
>
> As we are moving from undef to poison across the board, I'm not sure we should introduce mitigations that would become irrelevant once undef no longer exists. My view here is that miscompiles involving undef are mostly nominal, in that the undef value really should be a poison value and we just haven't switched it one yet.

I agree we shouldn't waste time with miscompilations of undef only, unless they show up in real life. That's why in the Alive2 dashboard I've separated the issues that are correct if undef didn't exist.
This particular patch fixes issues with poison as well (see `@test10_select`), so I think we should fix it. Plus, as the MSAN hack shows, even the undef bit was causing headaches. So maybe this time it's useful to fix both poison & undef.

That said, some of the tests clearly don't need freeze. This patch is introducing freeze on an existing branch (e.g., `@test9`).  For some reason `isGuaranteedNotToBeUndefOrPoison` is not working here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104569