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

[Michael Spencer via Phabricator via llvm-commits]

Bigcheese added inline comments.


================
Comment at: lld/ELF/CallGraphSort.cpp:208
+  Edge CE = Edges[CEI];
+  if (CE.From == CE.To) {
+    auto I = WorkLookup.find(CEI);
----------------
ruiu wrote:
> Bigcheese wrote:
> > ruiu wrote:
> > > How can the graph have a self edge?
> > Two symbols which are in the same section.
> Can you elaborate? I'm trying to understand it, so giving me just an answer isn't that helpful to me.
Given an object file which contains a .text section and two symbols, A and B, which point to that section. And given the following profile:

`A -> B 1000`

The code in `CallGraphSort::CallGraphSort` will generate a graph with one node and one edge:

```
Nodes:
  - index: 0
    weight: 1000
Edges:
  - from: 0
    to: 0
    weight: 1000
```

This correctly gives node[0] a weight of 1000 which is needed for the final sort by density.

`CallGraphSort::CallGraphSort` could skip adding self edges which would remove the need for this. I'll do that.


================
Comment at: lld/ELF/CallGraphSort.cpp:248-249
+  // 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]; });
+
----------------
ruiu wrote:
> Bigcheese wrote:
> > ruiu wrote:
> > > Can't this just be `std::sort(FE.begin(), FE.end())`?
> > No.
> Why?
Because the following code merges consecutive equal edges to remove duplicate edges. FE is a list of edges indices, not edges. 


https://reviews.llvm.org/D36351