Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BranchProbabilityInfo: Report branch_weights 1, 0 for invoke #72947

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MatzeB
Copy link
Contributor

@MatzeB MatzeB commented Nov 21, 2023

The calcEstimatedHeuristics used to report probabilities of invoke
exception paths as BlockExecWeight::UNWIND in relation to
BlockExecWeight::DEFAULT. This is "just" a 0xfffff : 1 ratio which can
lead to overestimated block counts for loops with high execution counts.
For example for a loop with a trip count of 5 million the exception loop
exit was estimated as 4 times more likely than the regular loop exit.

This change circumvents the limited range of calcEstimatedHeuristics
by hardcoding the estimated probabilities for invoke instructions as one, zero.
This also happens to match the behavior when loading PGO profiles.

@MatzeB
Copy link
Contributor Author

MatzeB commented Nov 21, 2023

@ebrevnov It seems you are the last person to work on calcEstimatedHeuristics, do you think it is fine to hardcode branch probabilities like this and skip the heuristics for exception control flow?

The `calcEstimatedHeuristics` used to report probabilities of invoke
exception paths as `BlockExecWeight::UNWIND` in relation to
`BlockExecWeight::DEFAULT`. This is "just" a 0xfffff : 1 ratio which can
lead to overestimated block counts for loops with high execution counts.
For example for a loop with a trip count of 5 million the exception loop
exit was estimated as 4 times more likely than the regular loop exit.

This change circumvents the limited range of `calcEstimatedHeuristics`
by hardcoding the probabilities of exception edge as 1, 0. This also
matches the behavior when loading PGO profiles.
@MatzeB MatzeB marked this pull request as ready for review November 21, 2023 03:36
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 21, 2023

@llvm/pr-subscribers-llvm-analysis

Author: Matthias Braun (MatzeB)

Changes

The calcEstimatedHeuristics used to report probabilities of invoke
exception paths as BlockExecWeight::UNWIND in relation to
BlockExecWeight::DEFAULT. This is "just" a 0xfffff : 1 ratio which can
lead to overestimated block counts for loops with high execution counts.
For example for a loop with a trip count of 5 million the exception loop
exit was estimated as 4 times more likely than the regular loop exit.

This change circumvents the limited range of calcEstimatedHeuristics
by hardcoding the estimated probabilities for invoke instructions as one, zero.
This also happens to match the behavior when loading PGO profiles.


Full diff: https://github.com/llvm/llvm-project/pull/72947.diff

6 Files Affected:

  • (modified) llvm/include/llvm/Analysis/BranchProbabilityInfo.h (+3-1)
  • (modified) llvm/lib/Analysis/BranchProbabilityInfo.cpp (+17-1)
  • (modified) llvm/test/Analysis/BranchProbabilityInfo/basic.ll (+10-10)
  • (modified) llvm/test/Analysis/BranchProbabilityInfo/deopt-invoke.ll (+6-6)
  • (modified) llvm/test/Analysis/BranchProbabilityInfo/loop.ll (+2-2)
  • (modified) llvm/test/Analysis/BranchProbabilityInfo/noreturn.ll (+2-2)
diff --git a/llvm/include/llvm/Analysis/BranchProbabilityInfo.h b/llvm/include/llvm/Analysis/BranchProbabilityInfo.h
index fb02997371bfb87..51d7a729a7bdeff 100644
--- a/llvm/include/llvm/Analysis/BranchProbabilityInfo.h
+++ b/llvm/include/llvm/Analysis/BranchProbabilityInfo.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_ANALYSIS_BRANCHPROBABILITYINFO_H
 #define LLVM_ANALYSIS_BRANCHPROBABILITYINFO_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/DenseSet.h"
@@ -181,7 +182,7 @@ class BranchProbabilityInfo {
   /// can be used when updating the CFG to update the branch probability
   /// information.
   void setEdgeProbability(const BasicBlock *Src,
-                          const SmallVectorImpl<BranchProbability> &Probs);
+                          ArrayRef<BranchProbability> Probs);
 
   /// Copy outgoing edge probabilities from \p Src to \p Dst.
   ///
@@ -406,6 +407,7 @@ class BranchProbabilityInfo {
   /// Based on computed weights by \p computeEstimatedBlockWeight set
   /// probabilities on branches.
   bool calcEstimatedHeuristics(const BasicBlock *BB);
+  bool calcFixedWeights(const BasicBlock *BB);
   bool calcMetadataWeights(const BasicBlock *BB);
   bool calcPointerHeuristics(const BasicBlock *BB);
   bool calcZeroHeuristics(const BasicBlock *BB, const TargetLibraryInfo *TLI);
diff --git a/llvm/lib/Analysis/BranchProbabilityInfo.cpp b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
index b45deccd913db47..bc229977970a7c6 100644
--- a/llvm/lib/Analysis/BranchProbabilityInfo.cpp
+++ b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
@@ -870,6 +870,20 @@ void BranchProbabilityInfo::computeEestimateBlockWeight(
   } while (!BlockWorkList.empty() || !LoopWorkList.empty());
 }
 
+bool BranchProbabilityInfo::calcFixedWeights(const BasicBlock *BB) {
+  const Instruction *Terminator = BB->getTerminator();
+  if (const InvokeInst *Invoke = dyn_cast<InvokeInst>(Terminator)) {
+    assert(Invoke->getNormalDest() == Invoke->getSuccessor(0) &&
+           Invoke->getUnwindDest() == Invoke->getSuccessor(1) &&
+           "unexpected successor ordering");
+    const BranchProbability BP[] = {BranchProbability::getOne(),
+                                    BranchProbability::getZero()};
+    setEdgeProbability(BB, BP);
+    return true;
+  }
+  return false;
+}
+
 // Calculate edge probabilities based on block's estimated weight.
 // Note that gathered weights were not scaled for loops. Thus edges entering
 // and exiting loops requires special processing.
@@ -1130,7 +1144,7 @@ BranchProbabilityInfo::getEdgeProbability(const BasicBlock *Src,
 
 /// Set the edge probability for all edges at once.
 void BranchProbabilityInfo::setEdgeProbability(
-    const BasicBlock *Src, const SmallVectorImpl<BranchProbability> &Probs) {
+    const BasicBlock *Src, ArrayRef<BranchProbability> Probs) {
   assert(Src->getTerminator()->getNumSuccessors() == Probs.size());
   eraseBlock(Src); // Erase stale data if any.
   if (Probs.size() == 0)
@@ -1256,6 +1270,8 @@ void BranchProbabilityInfo::calculate(const Function &F, const LoopInfo &LoopI,
       continue;
     if (calcMetadataWeights(BB))
       continue;
+    if (calcFixedWeights(BB))
+      continue;
     if (calcEstimatedHeuristics(BB))
       continue;
     if (calcPointerHeuristics(BB))
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/basic.ll b/llvm/test/Analysis/BranchProbabilityInfo/basic.ll
index 682de24ac5828af..90d34a418d98d47 100644
--- a/llvm/test/Analysis/BranchProbabilityInfo/basic.ll
+++ b/llvm/test/Analysis/BranchProbabilityInfo/basic.ll
@@ -243,8 +243,8 @@ entry:
 if.then:
   invoke i32 @InvokeCall()
           to label %invoke.cont unwind label %lpad
-; CHECK:  edge if.then -> invoke.cont probability is 0x7fff8000 / 0x80000000 = 100.00% [HOT edge]
-; CHECK:  edge if.then -> lpad probability is 0x00008000 / 0x80000000 = 0.00%
+; CHECK:  edge if.then -> invoke.cont probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK:  edge if.then -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
 
 invoke.cont:
   call void @ColdFunc() #0
@@ -270,8 +270,8 @@ if.then:
   invoke i32 @InvokeCall()
           to label %invoke.cont unwind label %lpad
 ; The cold call heuristic should not kick in when the cold callsite is in EH path.
-; CHECK: edge if.then -> invoke.cont probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
-; CHECK: edge if.then -> lpad probability is 0x00000800 / 0x80000000 = 0.00%
+; CHECK: edge if.then -> invoke.cont probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge if.then -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
 
 invoke.cont:
   br label %if.end
@@ -298,8 +298,8 @@ if.then:
           to label %invoke.cont unwind label %lpad
 ; Regardless of cold calls, edge weights from a invoke instruction should be
 ; determined by the invoke heuristic.
-; CHECK: edge if.then -> invoke.cont probability is 0x7fff8000 / 0x80000000 = 100.00% [HOT edge]
-; CHECK: edge if.then -> lpad probability is 0x00008000 / 0x80000000 = 0.00%
+; CHECK: edge if.then -> invoke.cont probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge if.then -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
 
 invoke.cont:
   call void @ColdFunc() #0
@@ -318,13 +318,13 @@ if.end:
 ; CHECK-LABEL: test_invoke_code_profiled
 define void @test_invoke_code_profiled(i1 %c) personality ptr @__gxx_personality_v0 {
 entry:
-; CHECK: edge entry -> invoke.to0 probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
-; CHECK: edge entry -> lpad probability is 0x00000800 / 0x80000000 = 0.00%
+; CHECK: edge entry -> invoke.to0 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge entry -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
   invoke i32 @InvokeCall() to label %invoke.to0 unwind label %lpad
 
 invoke.to0:
-; CHECK: edge invoke.to0 -> invoke.to1 probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
-; CHECK: edge invoke.to0 -> lpad probability is 0x00000800 / 0x80000000 = 0.00%
+; CHECK: edge invoke.to0 -> invoke.to1 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge invoke.to0 -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
   invoke i32 @InvokeCall() to label %invoke.to1 unwind label %lpad,
      !prof !{!"branch_weights", i32 444}
 
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/deopt-invoke.ll b/llvm/test/Analysis/BranchProbabilityInfo/deopt-invoke.ll
index ae46ed7851f895e..1679d0903ef903f 100644
--- a/llvm/test/Analysis/BranchProbabilityInfo/deopt-invoke.ll
+++ b/llvm/test/Analysis/BranchProbabilityInfo/deopt-invoke.ll
@@ -11,8 +11,8 @@ declare void @cold() cold
 define void @test1(i32 %0) personality ptr @"personality_function"  !prof !1 {
 ;CHECK: edge entry -> unreached probability is 0x00000001 / 0x80000000 = 0.00%
 ;CHECK: edge entry -> invoke probability is 0x7fffffff / 0x80000000 = 100.00% [HOT edge]
-;CHECK: edge invoke -> invoke.cont.unreached probability is 0x00000000 / 0x80000000 = 0.00%
-;CHECK: edge invoke -> land.pad probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge invoke -> invoke.cont.unreached probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge invoke -> land.pad probability is 0x00000000 / 0x80000000 = 0.00%
 ;CHECK: edge land.pad -> exit probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
 
 entry:
@@ -41,8 +41,8 @@ exit:
 define void @test2(i32 %0) personality ptr @"personality_function" {
 ;CHECK: edge entry -> unreached probability is 0x00000000 / 0x80000000 = 0.00%
 ;CHECK: edge entry -> invoke probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
-;CHECK: edge invoke -> invoke.cont.cold probability is 0x7fff8000 / 0x80000000 = 100.00% [HOT edge]
-;CHECK: edge invoke -> land.pad probability is 0x00008000 / 0x80000000 = 0.00%
+;CHECK: edge invoke -> invoke.cont.cold probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge invoke -> land.pad probability is 0x00000000 / 0x80000000 = 0.00%
 ;CHECK: edge land.pad -> exit probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
 
 entry:
@@ -71,8 +71,8 @@ exit:
 define void @test3(i32 %0) personality ptr @"personality_function" {
 ;CHECK: edge entry -> unreached probability is 0x00000000 / 0x80000000 = 0.00%
 ;CHECK: edge entry -> invoke probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
-;CHECK: edge invoke -> invoke.cont.cold probability is 0x7fff8000 / 0x80000000 = 100.00% [HOT edge]
-;CHECK: edge invoke -> land.pad probability is 0x00008000 / 0x80000000 = 0.00%
+;CHECK: edge invoke -> invoke.cont.cold probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge invoke -> land.pad probability is 0x00000000 / 0x80000000 = 0.00%
 ;CHECK: edge land.pad -> exit probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
 entry:
   br i1 undef, label %unreached, label %invoke
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/loop.ll b/llvm/test/Analysis/BranchProbabilityInfo/loop.ll
index c2aa705d3495793..562d979ed0b7e63 100644
--- a/llvm/test/Analysis/BranchProbabilityInfo/loop.ll
+++ b/llvm/test/Analysis/BranchProbabilityInfo/loop.ll
@@ -499,8 +499,8 @@ 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%
+; CHECK: edge loop -> invoke.cont probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge loop -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
 
 invoke.cont:
   %inc = add nsw i32 %i.0, 1
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/noreturn.ll b/llvm/test/Analysis/BranchProbabilityInfo/noreturn.ll
index e4bcdfc082ff917..028f1070b8dc5f9 100644
--- a/llvm/test/Analysis/BranchProbabilityInfo/noreturn.ll
+++ b/llvm/test/Analysis/BranchProbabilityInfo/noreturn.ll
@@ -118,8 +118,8 @@ if.then:                                          ; preds = %entry
   %exception = call ptr @__cxa_allocate_exception(i64 1) #0
   invoke i32 @smallFunction(i32 %idx)
           to label %invoke.cont unwind label %lpad
-; CHECK: edge if.then -> invoke.cont probability is 0x40000000 / 0x80000000 = 50.00%
-; CHECK: edge if.then -> lpad probability is 0x40000000 / 0x80000000 = 50.00%
+; CHECK: edge if.then -> invoke.cont probability is 0x80000000 / 0x80000000 = 100.00%
+; CHECK: edge if.then -> lpad probability is 0x00000000 / 0x80000000 = 0.00%
 
 invoke.cont:                                      ; preds = %if.then
   call void @__cxa_throw(ptr %exception, ptr @_ZTIi, ptr null) #1

@WenleiHe
Copy link
Member

Not sure if this is worth adding a new calcFixedWeights...

One can always contrive a case where 0xfffff : 1 is actually close to ground truth than 1:0 for exception. If this is just for non-PGO case, it's not possible to make a generally good guess anyways.

However even if there is something that is generally better, can that be achieved with current calcEstimatedHeuristics? I see two ways of doing it, either increase DEFAULT or make UNWIND 0. Having an inconsistent way of arriving at a branch probability seems unnecessary IMO.

@david-xl
Copy link
Contributor

Agree with @WenleiHe here. Perhaps changing BlockExecWeight to be uint64 type and scale up all the categories other than UNWIND.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants