Early exit with cheaper checks

[davide]

This is probably fine, but does it matter? i.e. you have compile time numbers?

[hiraditya]

In D49617#1170536, @davide wrote:

This is probably fine, but does it matter? i.e. you have compile time numbers?

I dont have numbers, maybe you can help with that.

[davide]

In D49617#1170597, @hiraditya wrote:

In D49617#1170536, @davide wrote:

This is probably fine, but does it matter? i.e. you have compile time numbers?

I dont have numbers, maybe you can help with that.

I guess my question is, what's the motivation for this change? You found this by inspection?

[hiraditya]

Adding BasicBlock::isEHPad back to preserve the semantics.

[hiraditya]

In D49617#1170601, @davide wrote:

In D49617#1170597, @hiraditya wrote:

In D49617#1170536, @davide wrote:

This is probably fine, but does it matter? i.e. you have compile time numbers?

I dont have numbers, maybe you can help with that.

I guess my question is, what's the motivation for this change? You found this by inspection?

Yes. For basic blocks with large number of instructions before a callsite the previous approach may consume unnecessary cycles.

[sebpop]

looks good

llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp

 static bool canSplitCallSite(CallSite CS, TargetTransformInfo &TTI) {
   // FIXME: As of now we handle only CallInst. InvokeInst could be handled
   // without too much effort.
   Instruction *Instr = CS.getInstruction();
   if (!isa<CallInst>(Instr))
     return false;
 
   BasicBlock *CallSiteBB = Instr->getParent();
+  // Need 2 predecessors and cannot split an edge from an IndirectBrInst.
+  SmallVector<BasicBlock *, 2> Preds(predecessors(CallSiteBB));
+  if (Preds.size() != 2 || isa<IndirectBrInst>(Preds[0]->getTerminator()) ||
+      isa<IndirectBrInst>(Preds[1]->getTerminator()))
+    return false;
+
+  // BasicBlock::canSplitPredecessors is more agressive, so checking for
+  // BasicBlock::isEHPad as well.
+  if (!CallSiteBB->canSplitPredecessors() || CallSiteBB->isEHPad())
+    return false;
+
   // Allow splitting a call-site only when the CodeSize cost of the
   // instructions before the call is less then DuplicationThreshold. The
   // instructions before the call will be duplicated in the split blocks and
   // corresponding uses will be updated.
   unsigned Cost = 0;
   for (auto &InstBeforeCall :
        llvm::make_range(CallSiteBB->begin(), Instr->getIterator())) {
     Cost += TTI.getInstructionCost(&InstBeforeCall,
                                    TargetTransformInfo::TCK_CodeSize);
     if (Cost >= DuplicationThreshold)
       return false;
   }
 
-  // Need 2 predecessors and cannot split an edge from an IndirectBrInst.
-  SmallVector<BasicBlock *, 2> Preds(predecessors(CallSiteBB));
-  if (Preds.size() != 2 || isa<IndirectBrInst>(Preds[0]->getTerminator()) ||
-      isa<IndirectBrInst>(Preds[1]->getTerminator()))
-    return false;
-
-  // Do not split a call-site in an exception handling block. This check
-  // prevents triggering an assertion in SplitEdge used via
-  // DuplicateInstructionsInSplitBetween. 
-  if (CallSiteBB->isEHPad())
-    return false;
-
-  return CallSiteBB->canSplitPredecessors();
+  return true;
 }
 
 static Instruction *cloneInstForMustTail(Instruction *I, Instruction *Before,
                                          Value *V) {
   Instruction *Copy = I->clone();
   Copy->setName(I->getName());
   Copy->insertBefore(Before);
   if (V)