[PATCH] D93943: [SimplifyCFG] Update SimplifyBranchOnICmpChain to recognize select form of and/or

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D93943#2475825 <https://reviews.llvm.org/D93943#2475825>, @aqjune wrote:

> In D93943#2475598 <https://reviews.llvm.org/D93943#2475598>, @nikic wrote:
>
>> I think this may not be correct if there is an `ExtraCase`. `ExtraCase || X == C || ...` should be fine as it is converted into `if (!ExtraCase) goto; switch (X)`, but something like `... || X == C || ExtraCase` would not be fine.
>
> You're right. Actually, while updating test/Transforms/SimplifyCFG/switch_create.ll I realized this and maybe I wanted to think that it wasn't very important, *but* that's not the right direction. I'll update this patch.

Thinking about this again, don't we need to always freeze the extra condition (unless known non-undef/poison)? While poison is only a problem when converting the select form into a branch, doesn't undef need to be frozen even in the and/or form? As the other operand may make it unconditionally true/false and thus well-defined.

In that case, I think we should either always insert the freeze, or not inserted it in the select form either (your original patch) with a FIXME for the more general problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93943