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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/FunctionSpecialization.h:87
+// keeps the discovered specialisation opportunities for the module in a single
+// vector, where the specialisations of each function form a contiguous range.
+// This map's value is the beginning and the end of that range.
----------------
It's not clear to me what this range exactly is.


================
Comment at: llvm/include/llvm/Transforms/IPO/FunctionSpecialization.h:89
+// This map's value is the beginning and the end of that range.
+using SpecMap = DenseMap<Function *, std::pair<unsigned, unsigned>>;
+
----------------
Nit: perhaps consider a more informative name for this.


================
Comment at: llvm/include/llvm/Transforms/IPO/FunctionSpecialization.h:91
+
+// Just a shorter abreviation.
+using Cost = InstructionCost;
----------------
Nit: abreviation -> abbreviation
and you can drop `shorter`, or the comment entirely, up to you.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:389
+  // specialisations.
+  DenseMap<SpecSig, unsigned> UM;
+
----------------
Nit: `UM` is a bit cryptic. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145379