[PATCH] D135463: [FuncSpec] Do not generate multiple copies for identical specializations.

[Momchil Velikov via Phabricator via llvm-commits]

chill added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/FunctionSpecialization.h:82
 
-using CallSpecBinding = std::pair<CallBase *, SpecializationInfo>;
-// We are using MapVector because it guarantees deterministic iteration
-// order across executions.
-using SpecializationMap = SmallMapVector<CallBase *, SpecializationInfo, 8>;
+using UniqueSpecSet = std::set<SpecInfo, std::not_equal_to<SpecInfo>>;
 
----------------
This has to be a `DenseSet`. `std::set` has logarithmic complexity of operations.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:364
     // with constant arguments.
-    bool Added = false;
+    SpecInfo Spec{{}, 0 - Cost, /*Clone=*/nullptr};
     for (Argument *A : Args) {
----------------
Here evaluation of the gain occurs even if the specialization is a duplicate.
I  would expect `getSpecializationBonus`  to take non-negligible time.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:441
 Function *
 FunctionSpecializer::createSpecialization(Function *F,
+                                          SpecInfo &Specialization) {
----------------
(nit) This function return `Close` twice, once as a return value and once as an out argument. A cleaner design would be to just return the value as the function does not depend on the `Clone` member variable and need not know anything about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135463