[PATCH] D104365: [FuncSpec] Enabling specializing direct constant

Wed Jun 16 04:26:54 PDT 2021

ChuanqiXu added a comment.

In D104365#2821547 <https://reviews.llvm.org/D104365#2821547>, @SjoerdMeijer wrote:

> Supporting integer constants opens up a whole new set of opportunities for function specialisation. This will have a big impact on compile-times, which is the main concern of this pass.
> I suggest we take one step at a time, and add support for integer constants under an option that is off by default. Just for testing purposes this would be convenient, but it also allows to measure the impact on compile-times by toggling this option on/off.

Yeah, I forgot to measure the compile-time. I would measure it later. But I guess it may be OK since we control the cost to specialize function incrementally (implemented in getSpecializationCost()). So the amount of new specialized functions wouldn't be much larger.

================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:15
 // - It does not handle specialization of recursive functions,
 // - It does not yet handle integer constants, and integer ranges,
 // - Only 1 argument per function is specialised,
----------------
SjoerdMeijer wrote:
> Well, it is not really surprising, it was documented here as a limitation, so you can change that line. I.e., remove 'integer constants', keep the 'integer ranges'.
> 
> In your subject and description, you use the term 'direct constant', but better to keep the same terminology, which I think is 'integer constant'.
Yeah, I missed it. And I think the terminology 'integer constants' may not be accurate. Since it seems like that float constant would be supported by this patch too.

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

https://reviews.llvm.org/D104365