[PATCH] D27862: [BPI] Use saturating multiplication to calculate denominators (WIP)

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: congh, davidxl, junbuml.
vsk added a subscriber: llvm-commits.

BPI may trigger signed overflow UB while computing branch probabilities
for cold calls or to unreachables. For example, with our current choice
of weights, we'll crash if there are >= 2^12 branches to an unreachable.

One solution to the problem is to use saturating multiplication to
compute the BP denominators. Another solution is to use APInt, but that
seems like a more expensive option with marginal benefit.

I marked this patch WIP because I'm not convinced that the test case is
useful. I've held off on writing a test for the change to the cold call
heuristic function for this reason.


https://reviews.llvm.org/D27862

Files:
  lib/Analysis/BranchProbabilityInfo.cpp


Index: lib/Analysis/BranchProbabilityInfo.cpp
===================================================================
--- lib/Analysis/BranchProbabilityInfo.cpp
+++ lib/Analysis/BranchProbabilityInfo.cpp
@@ -21,6 +21,7 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
@@ -162,12 +163,13 @@
     return true;
   }
 
-  BranchProbability UnreachableProb(UR_TAKEN_WEIGHT,
-                                    (UR_TAKEN_WEIGHT + UR_NONTAKEN_WEIGHT) *
-                                        UnreachableEdges.size());
-  BranchProbability ReachableProb(UR_NONTAKEN_WEIGHT,
-                                  (UR_TAKEN_WEIGHT + UR_NONTAKEN_WEIGHT) *
-                                      ReachableEdges.size());
+  BranchProbability UnreachableProb(
+      UR_TAKEN_WEIGHT, SaturatingMultiply(UR_TAKEN_WEIGHT + UR_NONTAKEN_WEIGHT,
+                                          (uint32_t)UnreachableEdges.size()));
+  BranchProbability ReachableProb(
+      UR_NONTAKEN_WEIGHT,
+      SaturatingMultiply(UR_TAKEN_WEIGHT + UR_NONTAKEN_WEIGHT,
+                         (uint32_t)ReachableEdges.size()));
 
   for (unsigned SuccIdx : UnreachableEdges)
     setEdgeProbability(BB, SuccIdx, UnreachableProb);
@@ -300,12 +302,13 @@
     return true;
   }
 
-  BranchProbability ColdProb(CC_TAKEN_WEIGHT,
-                             (CC_TAKEN_WEIGHT + CC_NONTAKEN_WEIGHT) *
-                                 ColdEdges.size());
-  BranchProbability NormalProb(CC_NONTAKEN_WEIGHT,
-                               (CC_TAKEN_WEIGHT + CC_NONTAKEN_WEIGHT) *
-                                   NormalEdges.size());
+  BranchProbability ColdProb(
+      CC_TAKEN_WEIGHT, SaturatingMultiply(CC_TAKEN_WEIGHT + CC_NONTAKEN_WEIGHT,
+                                          (uint32_t)ColdEdges.size()));
+  BranchProbability NormalProb(
+      CC_NONTAKEN_WEIGHT,
+      SaturatingMultiply(CC_TAKEN_WEIGHT + CC_NONTAKEN_WEIGHT,
+                         (uint32_t)NormalEdges.size()));
 
   for (unsigned SuccIdx : ColdEdges)
     setEdgeProbability(BB, SuccIdx, ColdProb);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27862.81795.patch
Type: text/x-patch
Size: 2202 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161216/f240edea/attachment.bin>