[PATCH] D145379: [FuncSpec] Cost model improvements.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:537
   auto *I = dyn_cast_or_null<Instruction>(U);
   // If not an instruction we do not know how to evaluate.
   // Keep minimum possible cost for now so that it doesnt affect
----------------
SjoerdMeijer wrote:
> Bit of duplication here with the comment above.
Good point, I forgot to remove these comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:564
 
 /// Compute a bonus for replacing argument \p A with constant \p C.
+void FunctionSpecializer::getSpecializationBonus(Argument *A, Constant *C,
----------------
SjoerdMeijer wrote:
> We are not calculating one bonus anymore, but two values latency and code size.
Will update the wording.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:583
+    Latency += UserLatency;
+    CodeSize -= UserSize;
   }
----------------
SjoerdMeijer wrote:
> I don't quite get why we are subtracting UserSize here. I guess something related to double or recounting, but after a quick look it  wasn't clear to me, perhaps a comment would be helpful here.
We start from `Cost CodeSize = Metrics.NumInsts + Metrics.NumInlineCandidates * MinFunctionSize;` (see the invocation of getSpecializationBonus). Then we start decreasing it by the footprint of each user.


================
Comment at: llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll:9
+
+; CHECK: FnSpecialization: Created 10 specializations in module
+
----------------
SjoerdMeijer wrote:
> Do we want to check the resulting IR too?
Not really. All we care about in this test is to show that the number of specializations created is not linear to `funcspec-max-iters`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145379