[PATCH] D152840: A new code layout algorithm for function reordering [3a/3]

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added inline comments.


================
Comment at: lld/ELF/CallGraphSort.cpp:314
+
+DenseMap<const InputSectionBase *, int> elf::applyCDS() {
+  // Creating a call graph and necessary data.
----------------
Add a function comment for this.


================
Comment at: lld/ELF/CallGraphSort.cpp:324
+  // Run the layout algorithm.
+  CDSortConfig Config;
+  std::vector<uint64_t> SortedSections =
----------------
It doesn't look like this can take configurations by flags?


================
Comment at: lld/ELF/CallGraphSort.cpp:331-333
+  for (uint64_t SecIdx : SortedSections) {
+    OrderMap[Sections[SecIdx]] = CurOrder++;
+  }
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: lld/ELF/CallGraphSort.cpp:345
+    return applyCDS();
+  else
+    return CallGraphSort().run();
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return


================
Comment at: lld/ELF/CallGraphSort.h:17
 
+llvm::DenseMap<const InputSectionBase *, int> applyCDS();
+
----------------
Why are we abbreviating here? It's not clear to me what CDS stands for.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152840/new/

https://reviews.llvm.org/D152840