[PATCH] D11915: Let edge weight be always greater than zero in both BPI and MBPI.

[Cong Hou via llvm-commits]

congh added a comment.

In http://reviews.llvm.org/D11915#221748, @davidxl wrote:

> Since we plan to change [M]MPI interfaces to hide weight which is internal, can we minimize this patch? My understanding is that minimal change is needed to make the weight normalization patch work well with zero weights.


Yes, I can reduce the size of this patch so that we only need to make sure there is no zero weight from BPI. This should make edge weights normalization work.


================
Comment at: include/llvm/CodeGen/MachineBasicBlock.h:386
@@ -381,3 +385,3 @@
   /// Note that duplicate Machine CFG edges are not allowed.
-  void addSuccessor(MachineBasicBlock *succ, uint32_t weight = 0);
+  void addSuccessor(MachineBasicBlock *succ, uint32_t weight = DEFAULT_WEIGHT);
 
----------------
davidxl wrote:
> Is 16 a good default value to indicate missing information? 
> 
> The current underlying (without this patch) assumption in MBPI is that if the weight list is empty, 0 weight represents 'unknown' weight -- otherwise it will be treated as a real zero value weight.  This assumption is of course very weak and can break down at any time.
> 
> All these needs to be considered in the new design when weight is eliminated from the profiling related interfaces -- i.e., reserve a special value for unknown 'probability'.
Both BPI and MBPI use 16 as default weight and that is why I also use 16 here. I agree with you. This doesn't matter now and we need to find out how to assign a default probability to a new successor if it is indicated explicitly.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:210
@@ -208,1 +209,3 @@
 
+  // Weight cannot be zero. Here we find out all zero weights read from MD, and
+  // replace them by 1. If any other weight is not zero, we need to normalize
----------------
davidxl wrote:
> Which clients produce zero weights ? It seems that Clang FE does not do that. See scaleBranchWeight in tools/clang/lib/CodeGen/CodeGenPGO.cpp
It happens in test cases where the branch weight is explicitly assigned as zero.


http://reviews.llvm.org/D11915