[JumpThreading] Handle zero !prof branch_weights

[yrouban]

Summary

Avoid division by zero in updatePredecessorProfileMetadata().
A test is added.

[yrouban]

@kazu, @nikic, @yamauchi, @ebrevnov, @efriedma, please review this small fix.

llvm/lib/Transforms/Scalar/JumpThreading.cpp

260–265

Here BranchProbability::getBranchProbability(..., TrueWeight + FalseWeight) results in zero denominator if both TrueWeight and FalseWeight are zero.

[hjyamauchi]

llvm/lib/Transforms/Scalar/JumpThreading.cpp

219

Add a code comment that this is to avoid a division by zero below?

[yrouban]

added comment. Moved BP variable to its definition.

[yrouban]

Landed as 707836ed4edb21e7133007f0200c3fd3a04d3409

[wenlei]

In D81449#2100080, @yrouban wrote:

Landed as 707836ed4edb21e7133007f0200c3fd3a04d3409

Curious why the test case threading_prof3.ll didn't make it into the commit?

[yrouban]

In D81449#3268387, @wenlei wrote:

Curious why the test case threading_prof3.ll didn't make it into the commit?

Good catch! The test looks to be lost. I will push it tomorrow if you do not mind.

[wenlei]

In D81449#3268408, @yrouban wrote:

In D81449#3268387, @wenlei wrote:

Curious why the test case threading_prof3.ll didn't make it into the commit?

Good catch! The test looks to be lost. I will push it tomorrow if you do not mind.

Sounds good, thanks!

[yrouban]

In D81449#3268409, @wenlei wrote:

Sounds good, thanks!

added with commit 707836ed4ed https://reviews.llvm.org/rG707836ed4edb21e7133007f0200c3fd3a04d3409

llvm/lib/Transforms/Scalar/JumpThreading.cpp

 //  In other words, if we know P(cond == true) is unlikely, we know
 //  that P(t == true) is also unlikely.
 //
 static void updatePredecessorProfileMetadata(PHINode *PN, BasicBlock *BB) {
   BranchInst *CondBr = dyn_cast<BranchInst>(BB->getTerminator());
   if (!CondBr)
     return;
 
-  BranchProbability BP;
   uint64_t TrueWeight, FalseWeight;
   if (!CondBr->extractProfMetadata(TrueWeight, FalseWeight))
     return;
 
+  if (TrueWeight + FalseWeight == 0)
+    // Zero branch_weights do not give a hint for getting branch probabilities.
+    // Technically it would result in division by zero denominator, which is
+    // TrueWeight + FalseWeight.
+    return;
+
   // Returns the outgoing edge of the dominating predecessor block
   // that leads to the PhiNode's incoming block:
   auto GetPredOutEdge =
       [](BasicBlock *IncomingBB,
          BasicBlock *PhiBB) -> std::pair<BasicBlock *, BasicBlock *> {
     auto *PredBB = IncomingBB;
     auto *SuccBB = PhiBB;
     SmallPtrSet<BasicBlock *, 16> Visited;
static void updatePredecessorProfileMetadata(PHINode *PN, BasicBlock *BB) {
   };
 
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
     Value *PhiOpnd = PN->getIncomingValue(i);
     ConstantInt *CI = dyn_cast<ConstantInt>(PhiOpnd);
 
     if (!CI || !CI->getType()->isIntegerTy(1))
       continue;
 
-    BP = (CI->isOne() ? BranchProbability::getBranchProbability(
-                            TrueWeight, TrueWeight + FalseWeight)
-                      : BranchProbability::getBranchProbability(
-                            FalseWeight, TrueWeight + FalseWeight));
+    BranchProbability BP =
+        (CI->isOne() ? BranchProbability::getBranchProbability(
 
     auto PredOutEdge = GetPredOutEdge(PN->getIncomingBlock(i), BB);
     if (!PredOutEdge.first)
       return;
 
     BasicBlock *PredBB = PredOutEdge.first;
     BranchInst *PredBr = dyn_cast<BranchInst>(PredBB->getTerminator());
     if (!PredBr)

llvm/test/Transforms/JumpThreading/threading_prof3.ll

+; RUN: opt -jump-threading -S < %s | FileCheck %s
+; RUN: opt -passes=jump-threading -S < %s | FileCheck %s
+
+; Check that all zero branch weights do not cause a crash.
+define void @zero_branch_weights(i32 %tmp, i32 %tmp3) {
+bb:
+  %tmp1 = icmp eq i32 %tmp, 1
+  br i1 %tmp1, label %bb5, label %bb2
+; CHECK-NOT: br i1 %tmp1,{{.*}} !prof
+
+bb2:                                              
+  %tmp4 = icmp ne i32 %tmp3, 1
+  br label %bb5
+; CHECK: br i1 %tmp4, {{.*}} !prof ![[PROF:[0-9]+]]
+
+bb5:                                             
+  %tmp6 = phi i1 [ false, %bb ], [ %tmp4, %bb2 ]
+  br i1 %tmp6, label %bb8, label %bb7, !prof !{!"branch_weights", i32 0, i32 0}
+
+bb7:                                            
+  br label %bb9
+
+bb8: 
+  br label %bb9
+
+bb9:                                           
+  ret void
+}
+
+;CHECK: ![[PROF]] = !{!"branch_weights", i32 0, i32 0}