[CallSiteSplitting] Simplify isPredicateOnPHI & continue checking PHIs.

[fhahn]

Summary

As pointed out by @thakis, currently CallSiteSplitting bails out after
checking the first PHI node. We should check all PHI nodes, until we
find one where call site splitting is beneficial.

This patch also slightly simplifies the code using BasicBlock::phis().

[davidxl]

llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

437

Should this be relaxed too -- allow splitting when there is one constant value?

[fhahn]

llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

437

I think we could , given that we also split if we find an argument only to be constant in one predecessor due to a condition. But I think that might be better done as a potential follow-up?

[davidxl]

lgtm

[thakis]

Thanks!

llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

 // Return true if the call-site has an argument which is a PHI with only
 // constant incoming values.
 static bool isPredicatedOnPHI(CallSite CS) {
   Instruction *Instr = CS.getInstruction();
   BasicBlock *Parent = Instr->getParent();
   if (Instr != Parent->getFirstNonPHIOrDbg())
     return false;
 
-  for (auto &BI : *Parent) {
-    if (PHINode *PN = dyn_cast<PHINode>(&BI)) {
-      for (auto &I : CS.args())
-        if (&*I == PN) {
-          assert(PN->getNumIncomingValues() == 2 &&
-                 "Unexpected number of incoming values");
-          if (PN->getIncomingBlock(0) == PN->getIncomingBlock(1))
-            return false;
-          if (PN->getIncomingValue(0) == PN->getIncomingValue(1))
-            continue;
-          if (isa<Constant>(PN->getIncomingValue(0)) &&
-              isa<Constant>(PN->getIncomingValue(1)))
-            return true;
-        }
-    }
-    break;
-  }
+  for (auto &PN : Parent->phis()) {
+    for (auto &Arg : CS.args()) {
+      if (&*Arg != &PN)
+        continue;
+      assert(PN.getNumIncomingValues() == 2 &&
+      if (PN.getIncomingBlock(0) == PN.getIncomingBlock(1))
+      if (PN.getIncomingValue(0) == PN.getIncomingValue(1))
+      if (isa<Constant>(PN.getIncomingValue(0)) &&
+          isa<Constant>(PN.getIncomingValue(1)))
   return false;
 }
 
 using PredsWithCondsTy = SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2>;
 
 // Check if any of the arguments in CS are predicated on a PHI node and return
 // the set of predecessors we should use for splitting.
 static PredsWithCondsTy shouldSplitOnPHIPredicatedArgument(CallSite CS) {

llvm/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll

 
 CallSiteBB:
   call i32 @callee(i32* %a_elt, i32 %c, i32 %c)
   br label %End
 
 End:
   ret void
 }
+
+; CHECK-LABEL: i32 @test_multiple_phis(
+; CHECK:      Header.split:
+; CHECK-NEXT:   %r2 = call i32 @callee(i32* null, i32 1, i32 5)
+; CHECK-NEXT:   br label %Tail
+
+; CHECK:      TBB.split:
+; CHECK-NEXT:   %r1 = call i32 @callee(i32* null, i32 2, i32 10)
+; CHECK-NEXT:   br label %Tail
+
+; CHECK:        Tail:
+; CHECK-NEXT:   %phi.call = phi i32 [ %r1, %TBB.split ], [ %r2, %Header.split ]
+; CHECK-NEXT:   %p.0 = phi i32 [ 0, %Header.split ], [ 99, %TBB.split ]
+; CHECK-NEXT:   %res = add i32 %phi.call, %p.0
+; CHECK-NEXT:   ret i32 %phi.call
+;
+define i32 @test_multiple_phis(i1 %c.1) {
+Header:
+  br i1 %c.1, label %Tail, label %TBB
+
+TBB:
+  br label %Tail
+
+Tail:
+  %p.0 = phi i32 [0, %Header], [99, %TBB]
+  %p.1 = phi i32[1, %Header], [2, %TBB]
+  %p.2 = phi i32 [5, %Header], [10, %TBB]
+  %r = call i32 @callee(i32* null, i32 %p.1, i32 %p.2)
+  %res = add i32 %r, %p.0
+  ret i32 %r
+
+End:
+  ret i32 10
+}