[PATCH] D36351: [lld][ELF] Add profile guided section layout

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: ELF/CallGraphSort.cpp:112
+// Take the edge list in Config->CallGraphProfile, resolve symbol names to
+// SymbolBodys, and generate a graph between InputSections with the provided
+// weights.
----------------
Simbol Bodies.


================
Comment at: ELF/CallGraphSort.cpp:117
+  DenseMap<const InputSectionBase *, NodeIndex> SecToNode;
+  std::map<Edge, EdgeIndex> EdgeMap;
+
----------------
why std::map and not densemap?
if there's a reason, add a comment.


================
Comment at: ELF/CallGraphSort.cpp:119
+
+  auto GetOrCreateNode = [&](const InputSectionBase *IS) -> NodeIndex {
+    auto Res = SecToNode.insert(std::make_pair(IS, Nodes.size()));
----------------
is the explicit return needed here?


================
Comment at: ELF/CallGraphSort.cpp:130
+      continue;
+    auto FromDR = dyn_cast_or_null<DefinedRegular>(Symtab->find(C.first.first));
+    auto ToDR = dyn_cast_or_null<DefinedRegular>(Symtab->find(C.first.second));
----------------
ruiu wrote:
> auto -> auto *
auto * presumably?


================
Comment at: ELF/CallGraphSort.cpp:186
+  // as equal edges will be merged in an order independent manner.
+  std::sort(FE.begin(), FE.end(),
+            [&](EdgeIndex AI, EdgeIndex BI) { return Edges[AI] < Edges[BI]; });
----------------
Can you guarantee determinism of the output if two sort implementations break ties in different ways?


================
Comment at: ELF/CallGraphSort.cpp:189
+
+  // std::unique, but also merge equal values.
+  auto First = FE.begin();
----------------
std::unique?


================
Comment at: ELF/CallGraphSort.cpp:241
+
+  // Sort by density. Invalidates all NodeIndexs.
+  std::sort(Nodes.begin(), Nodes.end(), [](const Node &A, const Node &B) {
----------------
node indices (I don't think indexes is a word).


https://reviews.llvm.org/D36351