[PATCH] D40729: [CallSiteSplitting] Remove isOrHeader restriction.

Jun Bum Lim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 14:46:05 PST 2017


junbuml added a comment.

I believe we need to check the code size increase with this because it will be applied widely and clone call-sites which could be inlined.



================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:335
   auto Preds = getTwoPredecessors(CS.getInstruction()->getParent());
-  if (!isOrHeader(Preds[0], Preds[1]) && !isOrHeader(Preds[1], Preds[0]))
-    return false;
----------------
We should bail out when Preds[0] == Preds[1].  For below IR , we should not split CS. 

```
define i32 @test(i32* %a, i32 %v, i32 %p) {
TBB:
  %cmp = icmp eq i32* %a, null
  br i1 %cmp, label %Tail, label %Tail
Tail:
  %r = call i32 @callee(i32* %a, i32 %v, i32 %p)
  ret i32 %r
}
```



================
Comment at: test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll:169
 
+;CHECK-LABEL: @test_eq_eq_eq_untaken
+;CHECK-LABEL: Tail.predBB1.split:
----------------
We need more test cases because this change will hit even very simple case like : 

```
define i32 @test(i32* %a, i32 %v, i32 %p) {
Header:
  br i1 undef, label %Tail, label %End

TBB:
  %cmp = icmp eq i32* %a, null
  br i1 %cmp, label %Tail, label %End

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

End:
  ret i32 %v
}
```


https://reviews.llvm.org/D40729





More information about the llvm-commits mailing list