[PATCH] D107897: [FuncSpec] Don't specialize function which are easy to inline

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

> For spec2017int, this patch could reduce the number of specialized functions by 33%. Then the compile time didn't increase for every benchmark.

Do you mean that with this new default of 100, we see less function specialising for SPEC2017?



================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:70
+static cl::opt<unsigned> SmallFunctionThreshold(
+    "func-spec-small-function-threshold", cl::Hidden,
+    cl::desc("For functions whose lines of code below this threshold, "
----------------
Nit: don't think "small" adds anything as it it is just the function threshold. Also the naming is inconsistent with other options (there is already some, but no need to make it worse), so can suggest:

  -func-specialization-function-size-threshold

but because that's a bit verbose, perhaps shorter and still descriptive is:

  -func-specialization-size-threshold


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:72
+    cl::desc("For functions whose lines of code below this threshold, "
+             " they wouldn't be specialized to avoid redundant sepcializations."),
+    cl::init(100));
----------------
Typo: `sepcializations`

About the description, I am not sure about "redundant". Don't think redundant is the right description, and I would guess it's a profitability / compile-time trade-off, and we do not want to consider too small functions?


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:444
+    // Or if the lines of codes implies that this function is easy to get inlined
+    // so that we shouldn't sepcialize it.
+    if (Metrics.notDuplicatable ||
----------------
Typo: `sepcialize`


================
Comment at: llvm/test/Transforms/FunctionSpecialization/function-specialization2.ll:5
+; RUN: opt -function-specialization -func-specialization-max-iters=0 -deadargelim -func-spec-small-function-threshold=10 -S < %s | FileCheck %s --check-prefix=DISABLED
+; RUN: opt -function-specialization -func-specialization-avg-iters-cost=1 -deadargelim -func-spec-small-function-threshold=10 -S < %s | FileCheck %s
 
----------------
Perhaps change one these `-func-spec-small-function-threshold=10` additions into `-force-function-specialization` to check that interaction and behaviour.


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

https://reviews.llvm.org/D107897