[PATCH] D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build.

[Xinliang David Li via llvm-commits]

On Sat, Jul 29, 2017 at 4:39 PM, Dehao Chen via Phabricator <
reviews at reviews.llvm.org> wrote:

> danielcdh added a comment.
>
> In https://reviews.llvm.org/D36052#825318, @davidxl wrote:
>
> > Instr PGO has its own set of simplification passes, but still I think we
> should keep this patch NFC for instrPGO. If there are performance numbers
> that justify the InstrPGO pipeline changes, it can be discussed/done in a
> separate thread.  This one should be left as SamplePGO only change.
>
>
> The tricky part is that PGOOptions was not passed in the ThinLTO backend
> before this patch. But after this patch, it will be passed in. However, in
> buildModuleSimplificationPipeline, it cannot know if this is the ThinLTO
> backend. As a result, we cannot disable instrumentation/annotation pass in
> there.
>
> The potential solution could be:
>
> - add another boolean value in buildModuleSimplificationPipeline to
> indicate it's ThinLTO backend
> - refactor the code from the beginning of buildModuleSimplificationPipeline
> to the PGOOpt handling to a separate function, and call it separately in
> the caller of buildModuleSimplificationPipeline.
>


If there is no good logical separation to split the
buildModuleSimplificationPipeline, it is better to go with option #1.

David


>
> Any suggestions on how to move forward?
>
> Thanks,
> Dehao
>
>
> https://reviews.llvm.org/D36052
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170729/ab5796b8/attachment.html>