[PATCH] Allow 0-weight branches in BranchProbabilityInfo.

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed May 6 10:58:41 PDT 2015


> On 2015 May 6, at 13:50, Diego Novillo <dnovillo at google.com> wrote:
> 
> On Wed, May 6, 2015 at 10:46 AM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> 
>>> On 2015 May 6, at 13:39, Diego Novillo <dnovillo at google.com> wrote:
>>> 
>>> 
>>> 
>>> On 05/06/15 13:37, Hans Wennborg wrote:
>>>> On Wed, May 6, 2015 at 10:09 AM, Duncan P. N. Exon Smith
>>>> 
>>>> Are you saying you don't want to guarantee that anymore, or that
>>>> forcing each weight <= UINT32_MAX / SI.getNumSuccessors() is
>>>> unnecessarily strict?
>>> 
>>> Yeah, I've wanted to remove this restriction for some time. The only requirement we really need is that the sum of all the weights fits within 32 bits.
>>> 
>>> I'm going to be removing this restriction in http://reviews.llvm.org/D9442
>> 
>> You'll have to leave it in place until the downstream consumers stop
>> flooring the weights at 1, but as long as there's a plan for lifting
>> the restriction I'm happy.
> 
> Well, currently BPI wants each branch weight to fit UINT32_MAX /
> successors, but in D9442, I will implement your suggestion of fitting
> weights such that Sum(weights) < UINT32_MAX.  This assert() may start
> triggering after my change.

Well, it looks to me like this `assert()` *will* start triggering; maybe
not with any checked in testcases, but we know how to trigger it and it's
not hard.  Splitting up the work (fixing switch lowering and fixing BPI)
makes sense to me, but I think that means you need to split the BPI changes
into two: (1) stop flooring in BPI, then (2) fix switch lowering (and other
consumers?) to handle 0 weights and remove assertions about
UINT32_MAX/successors, and only then (3) change BPI to Sum <= UINT32_MAX.






More information about the llvm-commits mailing list