[PATCH] D33540: [PM/ThinLTO] Port the ThinLTO pipeline (both components) to the new PM.

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 25 15:49:44 PDT 2017


tejohnson added a comment.

In https://reviews.llvm.org/D33540#765031, @chandlerc wrote:

> In https://reviews.llvm.org/D33540#764937, @davidxl wrote:
>
> > For partial inlining, leave it consistent with old PM for now.   We can introduce later an internal option to enable/disable pre-thin-link partial inlining and collect more performance number.
>
>
> Is there a rationale for the position in the old PM? Was this discussed somewhere? I'd almost rather adjust the position in the old PM to be consistent with here. Without data, I think the position I gave it here actually fits with the clear intent indicated by the comments. The previous position, honestly, looked like an accident to me. Maybe it isn't but I couldn't find any really clear reason written down anywhere, and the only test that fails if I reorder these things is just the one that asserts a particular order.
>
> Anyways, I'll change this back to the old PM one for now, but I really do want to understand why this is the right initial ordering to use.


Thanks - I'd like the ability to be able to enable/disable it in the pre-link optimization step when I test its performance with ThinLTO.


https://reviews.llvm.org/D33540





More information about the llvm-commits mailing list