[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