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

Jun Bum Lim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 15 10:25:34 PST 2017


junbuml 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];
----------------
fhahn wrote:
> 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.
Currently, the below test case is not handled, but with this change, we can split the call-site.  Although it's not the OR structure, it doesn't seem to break the original intention of this pass.  Looks like we can even extend this pass to support switch better.  However, if we want to support this in this change, I think we should run tests. 

```

define i32 @test(i32* %a, i32 %i, i32 %v) {
Header:
  switch i32 %i, label %End [
    i32 0, label %BB0
    i32 1, label %BB1
    i32 2, label %BB2
    i32 3, label %Tail
    i32 4, label %Tail
  ]

BB0:
  br label %End

BB1:
  br label %End

BB2:
  %cmp = icmp eq i32 %v, 1
  br i1 %cmp, label %Tail, label %End

Tail:
  %r = call i32 @callee(i32* %a, i32 %v)
  ret i32 %r

End:
  ret i32 %v
}

```


================
Comment at: test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll:232
+;CHECK: ret i32 %[[MERGED]]
+define i32 @test_cfg_no_or(i32* %a,  i32 %v) {
+entry:
----------------
Maybe test_cfg_no_or --> test_cfg_no_or_phi  ? 


https://reviews.llvm.org/D40037





More information about the llvm-commits mailing list