[PATCH] D136233: [SimpleLoopUnswitch] Insert loop-invariant conditions and unswitch them when it's profitable

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

> How does this type of unswitching interact with poison?

We used to have something like:

    %cmp1 = icmp ult i32 %x, %A
    br i1 cmp1, label %taken, label %exit
  
  taken:
    %cmp2 = icmp ult i32 %x, %B
    br i1 cmp2, label %taken2, label %exit2

We want to insert a new check `A <=u B` right before `br i1 cmp2`. Note that, in the original program, when we execute this branch, and either `A` or `B` was poison, then the original program has undefined behavior. So inserting a new comparison and branch by `A <=u B` doesn't change this fact: it's still UB if either of them is a poison. So the answer to your question is "interaction with poison remains unchanged".

> I'm not fond of the term "virtual" here as it is very non-descriptive. I don't have a great suggestion, but maybe "unswitch by injected invariants" or something like this?

Agreed, but let's have more opinions on what name is more suitable here.

> Looks like you are doing some canonicalization of conditions in this patch: replacing signed with unsigned, handling zexts. I suggest splitting all of this into separate patches so as to make the initial review smaller.

Makes sense, will do.


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

https://reviews.llvm.org/D136233