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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

I didn't take a look at the detail of the function, but here are my first round of review comments.



================
Comment at: ELF/Driver.cpp:603
 
+static void readCallGraph(MemoryBufferRef MB) {
+  DenseMap<std::pair<StringRef, StringRef>, uint64_t> Ret;
----------------
Please write a function comment to describe the file format.


================
Comment at: ELF/Writer.cpp:894
+// further improve locality.
+template <class ELFT> static void sortByCGProfile() {
+  using NodeIndex = std::ptrdiff_t;
----------------
Create a new file CallGraph.cpp and move this function there.

Instead of reorder sections in this function, it is probably better to return the OrderMap from this function, and use that map from Writer.cpp, so that this function is side-effect free.

Please split this function into smaller functions.


================
Comment at: ELF/Writer.cpp:1062
   Script->fabricateDefaultCommands();
+  sortByCGProfile<ELFT>();
   sortBySymbolsOrder<ELFT>();
----------------
Maybe sortByCallGraph is better, as the meaning of CG is not obvious.


Repository:
  rL LLVM

https://reviews.llvm.org/D36351