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

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added a comment.

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!



================
Comment at: lib/Transforms/IPO/FunctionSpecialization.cpp:209
+        // IPSCCP to propagate the constant arguments.
+        Function *Clone = cloneCandidateFunction(F);
+        Argument *ClonedArg = Clone->arg_begin() + A.getArgNo();
----------------
dongAxis1944 wrote:
> i think cloneCandidateFunction might become ‘compile time killer’, think the following case:
> 
> ```
> f(int arg1, int arg2) {...}
> g(1, 2);
> ```
> this patch might clone 'f' twice. But i think it might be avoid.
It looks like the outer loop would terminate on the first argument that can be used for specialization. So for your example, assuming there is only one call to `f` we would first specialize into
```
f.arg1(int arg2) { arg1 = 1; ... }
f.arg1(2);
```
and on the next call to `specializeFunctions` into 
```
f.arg1.arg2() { arg1 = 1; arg2 = 2; ... }
f.arg1.arg2();
```

In general, yes, this pass has the potential to impact compile time significantly, especially if there are many possible (known constant) values for some arguments. That is why a good cost model (`isArgumentInteresting`) is important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93838