[PATCH] D93838: [LLVM] [SCCP] [WIP] : Add Function Specialization pass

Vinay Madhusudan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 20 04:07:55 PST 2021


mivnay added a comment.

In D93838#2509427 <https://reviews.llvm.org/D93838#2509427>, @junparser wrote:

> hi @mivnay, could you please rebase this and D93762 <https://reviews.llvm.org/D93762> ?  So we can test it.
> It say that this patch can improve mcf 8%, we are interested at this because we also found that mcf has performance regression caused by ILP data dependency.

Hi,

In D93838#2557123 <https://reviews.llvm.org/D93838#2557123>, @fhahn wrote:

> In D93838#2527276 <https://reviews.llvm.org/D93838#2527276>, @mivnay wrote:
>
>> In D93838#2525167 <https://reviews.llvm.org/D93838#2525167>, @sanwou01 wrote:
>>
>>> Hi @mivnay,
>>>
>>> As the approach here is pretty much identical to D36432 <https://reviews.llvm.org/D36432>, do you have any plans on the cost model for this pass? The earlier proposal seems to have been abandoned due to a lack of good heuristics to trade off between the (potentially huge) increase in code size (as well as compile time) and performance improvements (which may or may not arise due to later optimizations). Perhaps it would help to look at what trade-offs GCC makes? Specifically, it would be good to have an idea of how you would implement getSpecializationCost and getSpecializationBonus.
>>>
>>> To help with review, could you make sure this patch is based on top of D93762 <https://reviews.llvm.org/D93762> so that those changes don't appear here again?
>>>
>>> Thanks!
>>
>> Hi,
>>
>> Thanks for looking into the patch. I have uploaded this patch just as a reference for the review D93762 <https://reviews.llvm.org/D93762>.  The pass is not complete and might have issues.  As you mentioned, the code is identical to D36432 <https://reviews.llvm.org/D36432> but we are trying to add a new pass (which can be invoked optionally) instead of tight integration with the SCCP functions. We are trying to utilize the `SCCPSolver` by moving it to the header file. Can you please look at D393762 <https://reviews.llvm.org/D93762>?
>
> IICU the review for D393762 raised some issues with the approach and also has comments with a few alternatives that may be viable to explore instead of function specialization? I am a bit reluctant to add a pass without a clear path towards enabling without causing regressions in general, especially when there are potential alternatives that may not suffer from the same problem.

@fhahn I am not sure if there was any conclusion with respect to the different approach. Are you referring to "integrating with inlining"?  D93762 <https://reviews.llvm.org/D93762> looked at specializing the function pointer types only and relied heavily on inlining the specialized function arguments. We are trying to accept other types as well. We are currently running spec with various architectures (AArch64 / X86) to enable the pass by default by having an aggressive mode option as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93838



More information about the llvm-commits mailing list