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

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:95
+      "force-function-specialization", cl::init(false), cl::Hidden,
+          cl::desc("Ignore the cost-model and force function specialization"));
+
----------------
I would like to rewording it into 'Force function specialization for every call site with constant argument'


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:97-100
+static cl::opt<unsigned> FuncSpecializationMaxIters(
+    "func-specialization-max-iters", cl::Hidden,
+    cl::desc("The maximum number of iterations function specialization is run"),
+    cl::init(1));
----------------
It looks like we need to re-evaluate the performance, compile-time and code-size for different parameters now.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:904
+      // FIXME: Only one argument per function.
+      return true;
+    }
----------------
SjoerdMeijer wrote:
> .... here!
> 
> It stops after specialising one argument. This is very arbitrary and probably what we want to fix in the first version.
> This requires a bit of a reshuffle, which is what I am addressing now.
I am OK to fix this in successive patches.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1408-1410
+  unsigned MaxIters = 1;
+  if (FuncSpecializationMaxIters.getNumOccurrences())
+    MaxIters = FuncSpecializationMaxIters;
----------------
Could we write:
```
 unsigned MaxIters = FuncSpecializationMaxIters; // Since the default value for FuncSpecializationMaxIters is 1
```


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

https://reviews.llvm.org/D93838