[BPI] Apply invoke heuristic before loop branch heuristic

[apilipenko]

Summary

Currently the loop branch heuristic is applied before the invoke heuristic which makes us overestimate the probability of the unwind destination of invokes inside loops. This in turn makes us grossly underestimate the frequencies of loops with invokes.

loop:
  %i = phi i32 [ 0, %entry ], [ %i.next, %invoke.cont ]
  invoke void @foo() to label %invoke.cont unwind label %lpad

invoke.cont:
  %i.next = add i32 %i, 1
  %cont = icmp ult i32 %i.next, %n
  br i1 %cont, label %loop, label %exit, !prof !{!"branch_weights", i32 9999, i32 1}

-analyze -branch-prob: 
 ...
 edge loop -> invoke.cont probability is 0x7c000000 / 0x80000000 = 96.88% [HOT edge]
 edge loop -> lpad probability is 0x04000000 / 0x80000000 = 3.12%
 ...

-analyze -block-freq:
 ...
 - loop: float = 31.901
 ...

Applying the invoke heuristic before the loop one makes the results more resonable:

-analyze -branch-prob: 
 ...
 edge loop -> invoke.cont probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
 edge loop -> lpad probability is 0x00000800 / 0x80000000 = 0.00%	
 ...

-analyze -block-freq:
 ...
 - loop: float = 9905.6
 ...

[vsk]

Thanks for the patch. Full disclaimer: I don't have much knowledge about this part of the codebase.

I've noticed that calcUnreachableHeuristics and calcColdCallHeuristics both avoid work when the terminator is an invoke. Do you think it would make more sense to handle invokes first, and to then assert that invokes aren't seen in the other heuristics? IMO it would reduce the amount of special-casing needed.

[apilipenko]

The suggestion to apply calcInvokeHeuristics before calcUnreachableHeuristics and calcColdCallHeuristics makes sense.

[vsk]

I think this looks good, but I'm not familiar enough with BPI to give an official lgtm.

[skatkov]

LGTM. If no one objects in 1-2 days, feel free to land.

lib/Analysis/BranchProbabilityInfo.cpp

 
 /// Calculate edge weights for successors lead to unreachable.
 ///
 /// Predict that a successor which leads necessarily to an
 /// unreachable-terminated block as extremely unlikely.
 bool BranchProbabilityInfo::calcUnreachableHeuristics(const BasicBlock *BB) {
   const TerminatorInst *TI = BB->getTerminator();
   assert(TI->getNumSuccessors() > 1 && "expected more than one successor!");
-
-  // Return false here so that edge weights for InvokeInst could be decided
-  // in calcInvokeHeuristics().
-  if (isa<InvokeInst>(TI))
-    return false;
+  assert(!isa<InvokeInst>(TI) &&
+         "Invokes should have already been handled by calcInvokeHeuristics");
 
   SmallVector<unsigned, 4> UnreachableEdges;
   SmallVector<unsigned, 4> ReachableEdges;
 
   for (succ_const_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I)
     if (PostDominatedByUnreachable.count(*I))
       UnreachableEdges.push_back(I.getSuccessorIndex());
     else

 /// cold function.  Those edges are unlikely to be taken, so we give
 /// them relatively low weight.
 ///
 /// Return true if we could compute the weights for cold edges.
 /// Return false, otherwise.
 bool BranchProbabilityInfo::calcColdCallHeuristics(const BasicBlock *BB) {
   const TerminatorInst *TI = BB->getTerminator();
   assert(TI->getNumSuccessors() > 1 && "expected more than one successor!");
-
-  // Return false here so that edge weights for InvokeInst could be decided
-  // in calcInvokeHeuristics().
-  if (isa<InvokeInst>(TI))
-    return false;
+  assert(!isa<InvokeInst>(TI) &&
+         "Invokes should have already been handled by calcInvokeHeuristics");
 
   // Determine which successors are post-dominated by a cold block.
   SmallVector<unsigned, 4> ColdEdges;
   SmallVector<unsigned, 4> NormalEdges;
   for (succ_const_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I)
     if (PostDominatedByColdCall.count(*I))
       ColdEdges.push_back(I.getSuccessorIndex());
     else
LLVM_DEBUG(dbgs() << "Computing probabilities for " << BB->getName()
                       << "\n");
     updatePostDominatedByUnreachable(BB);
     updatePostDominatedByColdCall(BB);
     // If there is no at least two successors, no sense to set probability.
     if (BB->getTerminator()->getNumSuccessors() < 2)
       continue;
     if (calcMetadataWeights(BB))
       continue;
+    if (calcInvokeHeuristics(BB))
+      continue;
     if (calcUnreachableHeuristics(BB))
       continue;
     if (calcColdCallHeuristics(BB))
       continue;
     if (calcLoopBranchHeuristics(BB, LI, SccI))
       continue;
     if (calcPointerHeuristics(BB))
       continue;
     if (calcZeroHeuristics(BB, TLI))
       continue;
     if (calcFloatingPointHeuristics(BB))
       continue;
-    calcInvokeHeuristics(BB);
   }
 
   PostDominatedByUnreachable.clear();
   PostDominatedByColdCall.clear();
 
   if (PrintBranchProb &&
       (PrintBranchProbFuncName.empty() ||
        F.getName().equals(PrintBranchProbFuncName))) {

test/Analysis/BlockFrequencyInfo/loop_with_invoke.ll

+; RUN: opt < %s -analyze -block-freq | FileCheck %s
+; RUN: opt < %s -passes='print<block-freq>' -disable-output 2>&1 | FileCheck %s
+
+; CHECK-LABEL: Printing analysis {{.*}} for function 'loop_with_invoke':
+; CHECK-NEXT: block-frequency-info: loop_with_invoke
+define void @loop_with_invoke(i32 %n) personality i8 0 {
+; CHECK-NEXT: entry: float = 1.0, int = [[ENTRY:[0-9]+]]
+entry:
+  br label %loop
+
+; CHECK-NEXT: loop: float = 9905.6
+loop:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %invoke.cont ]
+  invoke void @foo() to label %invoke.cont unwind label %lpad
+
+; CHECK-NEXT: invoke.cont: float = 9905.6
+invoke.cont:
+  %i.next = add i32 %i, 1
+  %cont = icmp ult i32 %i.next, %n
+  br i1 %cont, label %loop, label %exit, !prof !0
+
+; CHECK-NEXT: lpad: float = 0.0094467
+lpad:
+  %ll = landingpad { i8*, i32 }
+          cleanup
+  br label %exit
+
+; CHECK-NEXT: exit: float = 1.0, int = [[ENTRY]]
+exit:
+  ret void
+}
+
+declare void @foo()
+
+!0 = !{!"branch_weights", i32 9999, i32 1}

test/Analysis/BranchProbabilityInfo/loop.ll

 
 for.inc:
   %flip.1 = phi i32 [ 0, %if.then ], [ 1, %if.else ]
   %sum.1 = phi i32 [ %sum.0, %if.then ], [ %add2, %if.else ]
   %inc = add nsw i32 %i.0, 1
   br label %for.cond
 ; CHECK: edge for.inc -> for.cond probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
 }
+
+; The loop heuristic should not overwrite the invoke heuristic. The unwind destination
+; of an invoke should be considered VERY rare even in a loop.
+define void @test12(i32 %a) personality i8 0 {
+entry:
+  br label %loop
+; CHECK: edge entry -> loop probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+
+loop:
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %invoke.cont ]
+  invoke i32 @InvokeCall()
+          to label %invoke.cont unwind label %lpad
+; CHECK: edge loop -> invoke.cont probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge loop -> lpad probability is 0x00000800 / 0x80000000 = 0.00%
+
+invoke.cont:
+  %inc = add nsw i32 %i.0, 1
+  %cmp = icmp slt i32 %inc, %a
+  br i1 %cmp, label %loop, label %exit
+; CHECK: edge invoke.cont -> loop probability is 0x7c000000 / 0x80000000 = 96.88% [HOT edge]
+; CHECK: edge invoke.cont -> exit probability is 0x04000000 / 0x80000000 = 3.12%
+
+lpad:
+  %ll = landingpad { i8*, i32 }
+          cleanup
+  br label %exit
+
+exit:
+  ret void
+}
+
+declare i32 @InvokeCall()