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

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added inline comments.


================
Comment at: ELF/CallGraphSort.cpp:32
+///
+/// It does so given a call graph profile by the following:
+/// * Build a weighted call graph from the profile
----------------
The part about profile should be in a followup patch. For now this is given the weighted call graph.


================
Comment at: ELF/CallGraphSort.cpp:63
+
+  std::vector<const InputSectionBase *> ISBs;
+  std::vector<int> Sections;
----------------
You can remove this vector.

The only real use is when computing the returned DenseMap and you can use the Sections vector there:

  for (const Cluster &C : Clusters)
    for (int SecIndex : C.Sections)
      OrderMap[Sections[SecIndex].ISB] = CurOrder++;


================
Comment at: ELF/CallGraphSort.cpp:137
+
+Cluster::Cluster(int Sec, const InputSectionBase *IS) {
+  ISBs.push_back(IS);
----------------
Define this function inline.


================
Comment at: ELF/CallGraphSort.cpp:263
+  for (Cluster &C : Clusters) {
+    SecToCluster[C.Sections.front()] = &C;
+  }
----------------
You don't need this loop, just add

 SecToCluster[SI] = &Clusters.back();

to the previous one.


================
Comment at: ELF/CallGraphSort.cpp:297
+
+    for (int SI : C.Sections)
+      SecToCluster[SI] = PredC;
----------------
This is a slow union find solution when there are many union operations.  Maybe that doesn't impact this in practice, but at least leave a comment about a possible improvement if we ever hit it.



https://reviews.llvm.org/D36351