[PATCH] D90841: [JumpThreading] Fix branch probabilities in DuplicateCondBranchOnPHIIntoPred()

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban created this revision.
yrouban added reviewers: kazu, MaskRay, wmi, efriedma, ebrevnov.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
yrouban requested review of this revision.

When instructions are cloned from block //BB// to //PredBB// in the method //DuplicateCondBranchOnPHIIntoPred()// number of successors of //PredBB// changes from 1 to number of successors of //BB//. So we have to copy branch probabilities from //BB// to //PredBB//. I found this place when the single successor of //PredBB// got 1.0 probability which was left unchanged after //PredBB// got the instructions from its successor along with a terminator with several successors. So the first edge from //ProbBB// had probability 1.0 and the second resulted in 0.5 as it was unset explicitly. I cannot create an LL test since D90284 <https://reviews.llvm.org/D90284>. Here is a short test that failed before:

; CMD: opt -passes=jump-threading -debug-only=branch-prob -debug-only=jump-threading -S test.ll
define void @foo(i1 %f0, i1 %f1, i1 %f2) !prof !{!"function_entry_count", i64 2} {
bb1:

  br i1 %f0, label %bb3, label %bb2

bb2:

  br label %bb3

bb3:

  %ph = phi i1 [ %f1, %bb1 ], [ %f2, %bb2 ]
  br i1 %ph, label %exit1, label %unreach

exit1:

  ret void

unreach:

  unreachable

}

Here is the log with additional printing of probabilities at the end of //DuplicateCondBranchOnPHIIntoPred()// (compare the probs of bb3 and bb2, they must be the same):

- Branch Probability Info : foo ----

Computing probabilities for exit1
Computing probabilities for unreach
Computing probabilities for bb3
set edge bb3 -> 0 successor probability to 0x7fffffff / 0x80000000 = 100.00%
set edge bb3 -> 1 successor probability to 0x00000001 / 0x80000000 = 0.00%
Computing probabilities for bb2
Computing probabilities for bb1
Jump threading on function 'foo'

  Duplicating block 'bb3' into end of 'bb2' to eliminate branch on phi.  Cost: 0 block is:

bb3:                                              ; preds = %bb2, %bb1

  %ph = phi i1 [ %f1, %bb1 ], [ %f2, %bb2 ]
  br i1 %ph, label %exit1, label %unreach

Probability of bb3 -> (0) = 0x7fffffff / 0x80000000 = 100.00%
Probability of bb3 -> (1) = 0x00000001 / 0x80000000 = 0.00%
Probability of bb2 -> (0) = 0x40000000 / 0x80000000 = 50.00%
Probability of bb2 -> (1) = 0x40000000 / 0x80000000 = 50.00%

- Result

define void @foo(i1 %f0, i1 %f1, i1 %f2) !prof !0 {
bb1:

  br i1 %f0, label %bb3, label %bb2

bb2:                                              ; preds = %bb1

  br i1 %f2, label %exit1, label %unreach

bb3:                                              ; preds = %bb1

  %ph = phi i1 [ %f1, %bb1 ]
  br i1 %ph, label %exit1, label %unreach

exit1:                                            ; preds = %bb2, %bb3

  ret void

unreach:                                          ; preds = %bb2, %bb3

  unreachable

}

!0 = !{!"function_entry_count", i64 2}
--------------------------------------

Copying can be done with the newly introduced method in D90839 <https://reviews.llvm.org/D90839>.
The change in the method //ThreadThroughTwoBasicBlocks()// as a small related NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90841

Files:
  llvm/lib/Transforms/Scalar/JumpThreading.cpp


Index: llvm/lib/Transforms/Scalar/JumpThreading.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2237,12 +2237,8 @@
       CloneInstructions(PredBB->begin(), PredBB->end(), NewBB, PredPredBB);
 
   // Copy the edge probabilities from PredBB to NewBB.
-  if (HasProfileData) {
-    SmallVector<BranchProbability, 4> Probs;
-    for (BasicBlock *Succ : successors(PredBB))
-      Probs.push_back(BPI->getEdgeProbability(PredBB, Succ));
-    BPI->setEdgeProbability(NewBB, Probs);
-  }
+  if (HasProfileData)
+    BPI->copyEdgeProbabilities(PredBB, NewBB);
 
   // Update the terminator of PredPredBB to jump to NewBB instead of PredBB.
   // This eliminates predecessors from PredPredBB, which requires us to simplify
@@ -2672,6 +2668,8 @@
 
   // Remove the unconditional branch at the end of the PredBB block.
   OldPredBranch->eraseFromParent();
+  if (HasProfileData)
+    BPI->copyEdgeProbabilities(BB, PredBB);
   DTU->applyUpdatesPermissive(Updates);
 
   ++NumDupes;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90841.303086.patch
Type: text/x-patch
Size: 1098 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201105/1c276dd0/attachment.bin>