[PATCH] D11442: Provide an interface normalizeSuccWeights in MachineBasicBlock to normalize its successors' weights and use it in other places.

[Duncan P. N. Exon Smith]

> On 2015-Aug-05, at 14:16, Xinliang David Li <davidxl at google.com> wrote:
> 
>> 
>> If you go ahead with this, then you should first change
>> `BranchProbability` to use this design, and then you can reuse that
>> class for branch weights.
> 
> I think BranchProbability (defined in
> llvm/Support/BranchProbability.h) is well suited as a data structure
> used in interface. It is the underlying 'weight' storage in
> BranchProbabilityInfo and MBPI needs the change.

IIUC, you want the underlying weight to be treated like a
probability (represented in fixed point), since BranchProbability
isn't convenient for doing math.  So I'm suggesting we change BP to
be convenient for doing math (by making it a fixed-point number, or
otherwise adding math operations), and change BPI to return
BranchProbabilities instead of weights.

Maybe weights should be fixed-point probabilities as well?  But I
think you should fix BranchProbability first.

Or are you saying that it's convenient somewhere that
BranchProbability is a ratio?  We could expose both ratios and
fixed point probabilities from BPI.  But they're both probabilities
(the "weights" terminology doesn't really make sense).

> For BranchProbability, I also think we need to generalizes it as
> ValueRatio class and let BranchProbablity derives from it. I see many
> cases that BranchProbability is simply used a convenient class to
> present ratio.

BranchProbability is a little more restricted than just a `Ratio`,
since it disallows ratios larger than 1.  But yes, if other users
are using BP for general ratios, we should split that out into a
Ratio class (regardless of whether BranchProbability gets changed).

>> BTW, another approach would be to change `getSumForBlock()` to return a
>> 64-bit number, and just use 64-bit math everywhere (weights would still
>> be 32-bit).  Sounds like a lot less work.  Thoughts on that?
>> 
> 
> yes -- this sounds like less work -- but I think 'getSumForBlock()' is
> really an internal interface that should not be exposed to clients.

Yeah, maybe it shouldn't be exposed.  We could change BPI (and MBPI)
to expose only probabilities.