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

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added a comment.

Thanks Florian for cleaning up this pass.  Overall look good, but this doesn't seem a NFC and can miss an opportunity for a constant phi in non-OR  structure. Please see my 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];
----------------
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. 


================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:381
+  } else
     return false;
 
----------------
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
}

```


https://reviews.llvm.org/D40037