[PATCH] D119880: [FuncSpec] Support function specialization across multiple arguments.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

In D119880#3380014 <https://reviews.llvm.org/D119880#3380014>, @labrinea wrote:

> That's true; the new formula is not in the current revision. The idea is to keep a sublinear number of specializations when the number of candidates grows enormously (not expected to happen in real life code). So imagine we had 1000 candidates n/2 would be 500 whereas `log10(n^4)+1` will be 13. I measured the instruction count of clang when compiling the llvm test suite with `log10(n^5)+1` ( this function has a steeper curve - see https://www.google.com/search?q=plot+log10(x%5E5)%2B1 ) and it had a significant impact on ClamAV (1% more instructions over baseline compared to 0,57% increase with `log10(x^4)+1`).

Oh okay, got it.

> Regarding your previous question; in order to use stable sort we would need to flatten the nested structure of `SmallDenseMap<Function *, SpecializationMap>` into a wider `SpecializationMap`, which would contain specializations of several functions in one data structure. The problem is that `calculateGains` currently expects an empty `SpecializationMap`, which corresponds to a single function, hence it would require heavy adaptation. I can experiment and see if it's worth pursuing this idea (maybe in follow up patches?).

Okay, cheers, will leave it up to you then. I expect both approaches to be very close in terms of compile times. Algorithmically it is the same I think. So it's more about readability of the code.

I am now going to read the whole patch again.



================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:134
 struct SpecializationInfo {
-  ArgInfo Arg;          // Stores the {formal,actual} argument pair.
-  InstructionCost Gain; // Profitability: Gain = Bonus - Cost.
-
-  SpecializationInfo(Argument *A, Constant *C, InstructionCost G)
-      : Arg(A, C), Gain(G){};
+  SmallVector<ArgInfo, 8> Args; // Stores the {formal,actual} argument pairs.
+  InstructionCost Gain = 0;     // Profitability: Gain = Bonus - Cost.
----------------
Nit: we might as well drop the 8 here. From the programmers manual:

> In the absence of a well-motivated choice for the number of inlined elements N, it is recommended to use SmallVector<T> (that is, omitting the N).


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:339
+      if (!ForceFunctionSpecialization && MinGainThreshold == 0) {
+        // Sort the specializations based on Gain.
+        for (auto &Entry : Specializations) {
----------------
SjoerdMeijer wrote:
> This isn't clear to me, I don't see how we sort things. First, we add things to a map here....
If you keep this version, can you at least add that this is sorting the candidates using a map.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119880