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

[Michael Spencer via Phabricator via llvm-commits]

Bigcheese marked 10 inline comments as done.
Bigcheese added a comment.

This needs to be implemented in the linker because of the line:

  if (From.Size + To.Size > Target->PageSize)
    continue;

This is responsible for doubling the performance improvement on a game. Implementing this outside of the linker requires implementing most of the linker, including LTO.



================
Comment at: lld/ELF/CallGraphSort.cpp:124-125
+CallGraphSort::Node::Node(const InputSectionBase *IS) {
+  Sections.push_back(IS);
+  Size = IS->getSize();
+}
----------------
ruiu wrote:
> Sections is always empty, no? Why don't you do
> 
>   CallGraphSort::Node::Node(const InputSectionBase *IS) : Sections(IS), Size(IS->getSize()) {}
> 
> ?
Because `Sections(IS)` isn't valid?


================
Comment at: lld/ELF/CallGraphSort.cpp:162
+  // Create the graph.
+  for (const auto &C : Profile) {
+    const Symbol *FromSym = C.first.first;
----------------
ruiu wrote:
> Since `Config->CallGraphProfile` is available here, you don't need to pass it as an argument.
This class currently uses zero global state. I'd prefer not to add any for no gain.


================
Comment at: lld/ELF/CallGraphSort.cpp:217
+  }
+  std::vector<EdgeIndex> &FE = Nodes[CE.From].IncidentEdges;
+
----------------
ruiu wrote:
> Use ArrayRef.
`FE` is used as a std::vector here. Replacing with `ArrayRef` would not work.


================
Comment at: lld/ELF/Options.td:54-58
+def call_graph_profile_file: S<"call-graph-profile-file">,
+  HelpText<"Layout sections to optimize the given callgraph">;
+
+def call_graph_profile_sort: F<"call-graph-profile-sort">,
+  HelpText<"Sort sections by call graph profile information">;
----------------
ruiu wrote:
> I think these two options should be merged into one option which takes a filename as an argument and enables the feature.
A text file is not the only way to get input for sorting. The input can also come from object files.


https://reviews.llvm.org/D36351