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

Sat Jun 19 21:37:10 PDT 2021

aqjune 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.

+1 for caring about poison only.

It allows using `isGuaranteedNotToBePoison` (not `isGuaranteedNotToBeUndefOrPoison`) for this patch, which is a better analysis.

================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:4243
   if (ExtraCase && BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
     return false;

----------------
If it is settled to ignore the undef case, can we leave a comment here saying that `TODO: Once undef value is removed, this check isn't necessary`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104569