[PATCH] D104579: [SampleFDO] Add Pass1 of MIRAddFSDiscriminatorsPass before Block-Placement

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D104579#2829676 <https://reviews.llvm.org/D104579#2829676>, @xur wrote:

> In D104579#2828639 <https://reviews.llvm.org/D104579#2828639>, @wenlei wrote:
>
>> In D104579#2828504 <https://reviews.llvm.org/D104579#2828504>, @xur wrote:
>>
>>> This might just be specific to our deployment. Our AFDO optimized build
>>> uses the profile generated in the previous release.
>>> If we hold this Pass1 discriminator change and commit together with the FS
>>> profile change. We won't see the FSAFDO performance in the release right
>>> after -- we need to wait for another release to get all the samples with
>>> Pass1 discriminations.
>>>
>>> -Rong
>>
>> Thanks for clarification, makes sense.
>>
>> Sounds like your production usage is going to have one FS-Disc pass before block layout. And if we add more FS-Disc pass, say before RA, or others, it's not worth it?
>
> That's a good suggestion. I haven't tried to experiment with one pass before RA. I know that the one before block placement will the performance. I also tried add one after block-placement but did not gain much.
> Let me do some experiments and report back.
>
> Another aspect of this change is that we want to do this more conservatively. We have many SampleFDO targets and we don't want to commit too much changes in the initial flip ( to minimize compile time, memory usage impact). We prefer to do this step by step.

Incremental approach sounds reasonable. Was just curious whether you have tried other passes and see no improvements.

Looks like FS-AFDO changes are mostly in tree now though the profile generation part for FS profile is still outside of llvm (in your create_llvm_prof tool), right? Otherwise, we can give it a try too.


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

https://reviews.llvm.org/D104579