[PATCH] D40037: [CallSiteSplitting] Remove some indirection (NFC).

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:374
+  // contains the first compare of the OR and OrBB the second.
+  if (Preds[1]->getSinglePredecessor() == Preds[0]) {
+    HeaderBB = Preds[0];
----------------
junbuml wrote:
> In case HeaderBB has more than two successors (e.g, if a switch is it's terminator), this change could allow splitting.  If you want to keep this change NFC, the number of successors of HeaderBB may need to be checked. 
I think for the OR case the checks for ICMP would catch this case later (as switch for i1 only can have 2 successors I think) , but I could add an explicit check.


================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:381
+  } else
     return false;
 
----------------
junbuml wrote:
> We also split  a call-site if it use a PHI  with all constant incoming values. In this case, Preds are not necessarily an OR structure.  For example, in the diamond pattern below, we should split the call site. 
> 
> 
> ```
> define i32 @test(i32* %a,  i32 %v) {
> entry:
>   br i1 undef, label %TBB0, label %TBB1
> TBB0:
>   br i1 undef, label %Tail, label %End
> TBB1:
>   br i1 undef, label %Tail, label %End
> Tail:
>   %p = phi i32[1,%TBB0], [2, %TBB1]
>   %r = call i32 @callee(i32* %a, i32 %v, i32 %p)
>   ret i32 %r
> End:
>   ret i32 %v
> }
> 
> ```
Done! I've separated the code paths splitting on PHI and on Or, as they have different legality checks.


https://reviews.llvm.org/D40037