[PATCH] D79071: [BPI] Incorrect probability reported in case of mulptiple edges.

[Evgeniy via Phabricator via llvm-commits]

ebrevnov created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

By design 'BranchProbabilityInfo:: getEdgeProbability(const BasicBlock *Src, const BasicBlock *Dst) const' should return sum of probabilities over all edges from Src to Dst. Current implementation is buggy and returns 1/num_of_successors if probabilities are not explicitly set.

Note also current implementation assumes that either all or none edges have probabilities set. This is not the only place which uses such assumption. At least we should assert that in verifier. In addition we can think on a more robust API of BPI which would prevent situations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79071

Files:
  llvm/lib/Analysis/BranchProbabilityInfo.cpp
  llvm/test/Analysis/BranchProbabilityInfo/switch.ll


Index: llvm/test/Analysis/BranchProbabilityInfo/switch.ll
===================================================================
--- /dev/null
+++ llvm/test/Analysis/BranchProbabilityInfo/switch.ll
@@ -0,0 +1,53 @@
+; RUN: opt < %s -analyze -branch-prob | FileCheck %s
+; RUN: opt < %s -analyze -lazy-branch-prob | FileCheck %s
+; RUN: opt < %s -passes='print<branch-prob>' -disable-output 2>&1 | FileCheck %s
+
+declare void @g(i32)
+
+; Check correctness of reported probabilities in case of multiple edges between
+; basic blocks. In this case sum of probabilities over all edges should be
+; returned by BranchProbabilityInfo::getEdgeProbability.
+
+define void @test1(i32 %x) {
+;CHECK: edge entry -> return probability is 0x0ccccccd / 0x80000000 = 10.00%
+;CHECK: edge entry -> bb0 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb0 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb0 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb1 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb1 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb1 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb2 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb2 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge entry -> bb2 probability is 0x26666666 / 0x80000000 = 30.00%
+;CHECK: edge bb0 -> return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge bb1 -> return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+;CHECK: edge bb2 -> return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+
+entry:
+  switch i32 %x, label %return [
+    i32 0, label %bb0
+    i32 3, label %bb0
+    i32 6, label %bb0
+    i32 1, label %bb1
+    i32 4, label %bb1
+    i32 7, label %bb1
+    i32 2, label %bb2
+    i32 5, label %bb2
+    i32 8, label %bb2
+  ]
+
+bb0:                                              ; preds = %entry, %entry, %entry
+  tail call void @g(i32 0)
+  br label %return
+
+bb1:                                              ; preds = %entry, %entry, %entry
+  tail call void @g(i32 1)
+  br label %return
+
+bb2:                                              ; preds = %entry, %entry, %entry
+  tail call void @g(i32 2)
+  br label %return
+
+return:                                           ; preds = %bb2, %bb1, %bb0, %entry
+  ret void
+}
Index: llvm/lib/Analysis/BranchProbabilityInfo.cpp
===================================================================
--- llvm/lib/Analysis/BranchProbabilityInfo.cpp
+++ llvm/lib/Analysis/BranchProbabilityInfo.cpp
@@ -936,8 +936,10 @@
                                           const BasicBlock *Dst) const {
   auto Prob = BranchProbability::getZero();
   bool FoundProb = false;
+  uint32_t EdgeCount = 0;
   for (const_succ_iterator I = succ_begin(Src), E = succ_end(Src); I != E; ++I)
     if (*I == Dst) {
+      ++EdgeCount;
       auto MapI = Probs.find(std::make_pair(Src, I.getSuccessorIndex()));
       if (MapI != Probs.end()) {
         FoundProb = true;
@@ -945,7 +947,7 @@
       }
     }
   uint32_t succ_num = std::distance(succ_begin(Src), succ_end(Src));
-  return FoundProb ? Prob : BranchProbability(1, succ_num);
+  return FoundProb ? Prob : BranchProbability(EdgeCount, succ_num);
 }
 
 /// Set the edge probability for a given edge specified by PredBlock and an


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79071.260857.patch
Type: text/x-patch
Size: 3444 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200429/920bbaf8/attachment.bin>