[PATCH] D126455: [FuncSpec] Make the Function Specializer part of the IPSCCP pass.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:301
-  /// \returns true if at least one function is specialized.
-  bool specializeFunctions(FuncList &Candidates, FuncList &WorkList) {
-    bool Changed = false;
----------------
fhahn wrote:
> can you fix the indentation here in a NFC?
How? It seems ok.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:778
+    for (Argument &Arg : F->args())
+      if (!Arg.use_empty() && tryToReplaceWithConstant(&Arg))
+        LLVM_DEBUG(dbgs() << "FnSpecialization: Replaced constant argument: "
----------------
fhahn wrote:
> IIUC this is done during in between solver runs, right? Is this needed? Isn't it sufficient to continue with the constant value in the value mapping? This would probably remove the need to tell the solver to forget instructions/values.
This is not the only invocation of `tryToReplaceWithConstant` in FuncSpec. On this instance we try to replace the arguments of cloned functions. There's another invocation inside the functor `RunSCCPSolver`. On that instance we try to replace the instructions of cloned functions. Both calls occur as many times as  `FuncSpecializationMaxIters` is set to. 

Moreover, the SCCP pass itself does the same thing on arguments of tracked functions and on instructions of executable blocks (with `tryToReplaceWithConstant` and `simplifyInstsInBlock` accordingly). This happens after the Solver runs and before the Function Specializer is invoked. Therefore, I think we still need to tell the Solver to forget instructions/values if we want to merge the two passes.


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

https://reviews.llvm.org/D126455