[PATCH] Allow 0-weight branches in BranchProbabilityInfo.

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed May 6 11:26:03 PDT 2015


> On 2015 May 6, at 14:20, Hans Wennborg <hans at chromium.org> wrote:
> 
> On Wed, May 6, 2015 at 10:58 AM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> 
>>> 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.
> 
> We could just drop this assert, or change it. It's unnecessarily
> strict, as that thing that really matters is that the sum of weights
> doesn't overflow.

As long as we're flooring to 1, this assert is necessary.  Adding 1 to
all the weights increases the sum, but this assert guarantees that it
doesn't increase it beyond `UINT32_MAX`.





More information about the llvm-commits mailing list