[llvm-commits] [PATCH] Preserve branch weight metadata when creating switches in SimplifyCFG

[Michael Ilseman]

Attached is a test file I made for a few of the different rebalancing scenarios along with the expected IR code. It shows how rebalancing should occur for different scenarios on this one code path. This is all just a single code path in SimplifyCFG; hopefully Alastair will be able to get all paths rebalancing weights.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: tmp.cpp
Type: application/octet-stream
Size: 4716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120829/f780583f/attachment.obj>
-------------- next part --------------



On Aug 29, 2012, at 3:03 PM, Andrew Trick <atrick at apple.com> wrote:

> Alastair,
> 
> Michael Ilseman was independently working on a patch based on a design that we had discussed informally. I just reviewed his patch offline and committed r162868. He has verified the patch with set of tests that he'll be posting. I attempted to explain the weight balancing formula in the checkin comments. Michael did not fix all places where we convert to switches, so there is still work to do, but I hope this helps you.
> 
> Michael is not actively working on it, so if you find any more cases that need to be implemented that would be much appreciated. Also feel free to improve or factor the code however is convenient for you.
> 
> -Andy
> 
> On Aug 23, 2012, at 3:05 PM, Alastair Murray <alastairmurray42 at gmail.com> wrote:
> 
>> Hello,
>> 
>> This is still being worked on (the complicated case is exactly that!).
>> 
>> Regards,
>> Alastair.
>> 
>> On 17/08/12 22:55, Manman Ren wrote:
>>> 
>>> Hi Alastair,
>>> 
>>> Thanks for the patch.
>>> 
>>> Some of the logic is not clear to me.
>>> For example, in FoldValueComparisionIntoPredecessors, if PredDefault is not equal to BB:
>>> P --> 1 --> S1                           (weightP_1)
>>>          2 --> BB                           (weightP_2)
>>>          3 --> BB                           (weightP_3)
>>>          default -> PredDefault  (weightP_0)
>>> BB -> 2 --> S2                       (weightB_1)
>>>           4 --> S3                       (wegihtB_2)
>>>           default -> BBDefault (weightB_0)
>>> 
>>> We will create a new switch for P:
>>> P --> 1 --> S1                              (weightP_1)
>>>          2 --> S2                              (weightP_2)
>>>          3 --> default                      (weightP_3)
>>>          default -> PredDefault    (weightP_0)
>>> 
>>> But from you patch, I think we will get
>>> P --> 1 --> S1                              (weightP_1)
>>>          2 --> S2                              (weightB_2)
>>>          3 --> default                      (0)
>>>          default -> PredDefault    (not sure, it seems that you are updating DefaultWeight inside a loop)
>>> 
>>> The case where PredDefault is equal to BB is more complicated:
>>> P --> 1 --> BB                           (weightP_1)
>>>          2 --> S1                           (weightP_2)
>>>          3 --> S2                           (weightP_3)
>>>          default -> BB                 (weightP_0)
>>> BB -> 2 --> S3                       (weightB_1)
>>>           4 --> S4                       (weightB_2)
>>>           default -> BBDefault (weightB_0)
>>> We will create:
>>> P --> 2 --> S1                       (weightP_2*(weightB_0 + weightB_2))
>>>          3 --> S2                       (weightP_3*(weightB_0 + weightB_2))
>>>          4 --> S4                       ((weightP_1+weightP_0)*weightB_2)
>>>          default -> BBDefault    ((weightP_1+weightP_0)*weightB_0)
>>> 
>>> I may be wrong in what the weight should be, since it is just complicated :)
>>> You can look at how we update the metadata in FoldBranchToCommonDest if you have not done so.
>>> 
>>> I also think we should have some utilities for fixed-point arithmetic	such as (APInt with a scale factor) to avoid handling overflow all the time.
>>> 
>>> Thanks,
>>> Manman
>>> 
>>> On Aug 17, 2012, at 11:45 AM, Alastair Murray wrote:
>>> 
>>>> Hello all,
>>>> 
>>>> Evan Cheng is CC'ed because he originally suggested I look at this issue, Manman Ren is CC'ed because he has expressed interest in this topic.
>>>> 
>>>> Please review the attached patch. Note: I don't have commit rights.
>>>> 
>>>> The patch preserves branch weight metadata in SimplifyCFG in various situations where switch instructions are inserted (converting linked if-statements or icmp's to a switch instruction, merging switch instructions, merging an if-statement into a switch instruction).
>>>> 
>>>> As metadata can not be assumed to be consistent with the current form of the IR the added code only preserves metadata if it seems consistent.
>>>> 
>>>> It has been tested with both profiling data (over projects/test-suite) and __builtin_expect.  In the case of __builtin_expect the branch weights are scaled when creating switch instructions to account for the different probability model.  if (C1) { A } else if (C2) { B} means P(B) = P(C2 | !C1), but a switch statement just models the probability as P(C2).
>>>> 
>>>> Regards,
>>>> Alastair Murray
>>>> <preserve_switch_branch_metadata.patch>
>>> 
>>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>