[PATCH] D133121: [PGO] Annotate branch_weight for invoke instruction

[Rong Xu via Phabricator via llvm-commits]

xur added a comment.

In D133121#3774815 <https://reviews.llvm.org/D133121#3774815>, @hoy wrote:

> In D133121#3774799 <https://reviews.llvm.org/D133121#3774799>, @xur wrote:
>
>> In D133121#3774764 <https://reviews.llvm.org/D133121#3774764>, @hoy wrote:
>>
>>> In D133121#3774657 <https://reviews.llvm.org/D133121#3774657>, @xur wrote:
>>>
>>>> In D133121#3772956 <https://reviews.llvm.org/D133121#3772956>, @hoy wrote:
>>>>
>>>>> Thanks for the change. Wondering if you saw performance improvement with it.
>>>>>
>>>>> The corresponding change on the sample profile side isn't included, do you have any plan for it?
>>>>
>>>> Yes. For AutoFDO, we still need some change for the invoke in sampleloader
>>>> The situation is slightly different in AutoFDO. In FDO, we do branch_weight annotation first, then do the VP.
>>>>
>>>> In AutoFDO, we do VP annotation first  and then branch_weight annotation. We need to set the branch_weight metadata where there is already a VP metadata (which is the reverse of FDO).
>>>> I was planning to have a separated patch for AutoFDO, but I can also include it in this patch.
>>>
>>> A separate patch sounds good, thanks.
>>
>> AutoFDO is less problematic as branch_weights metadata overwrites VP in the end. (and many VP metadata is already consumed by the prompting in preinliner).
>>
>> In FDO, VP metadata overwrites branch_weights -- This is bad as we lost more information in BPI/BFI.
>> (branch_weights is more important for downstream opt).
>
> It looks like same happens to autofdo, i.e, no branch_weights assigned to indirect invokes? https://github.com/llvm/llvm-project/blob/7b81a81d5f9cccb1b091cfc5264bc483b0acc83a/llvm/lib/Transforms/IPO/SampleProfile.cpp#L1628
>
> Oh, I guess branch_weight is attached to invokes that are promoted from an indirect invoke, so it is better handled than the FDO case.

the branch weight for invoke is set in line 1724 (https://github.com/llvm/llvm-project/blob/7b81a81d5f9cccb1b091cfc5264bc483b0acc83a/llvm/lib/Transforms/IPO/SampleProfile.cpp#L1724)

line 1628 sets the call's branch-weight (which is a single number, call counts) -- this should be overwritten by line 1724.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133121/new/

https://reviews.llvm.org/D133121