[PATCH] D112164: [lld-macho] Port CallGraphSort from COFF/ELF

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/CallGraphSort.cpp:251
+DenseMap<const InputSection *, size_t> macho::computeCallGraphProfileOrder() {
+  return CallGraphSort().run();
+}
----------------
TimeTraceScope here? (as it'd be doing non-negligible amount of work)


================
Comment at: lld/MachO/CallGraphSort.h:16
+namespace macho {
+class InputSection;
+
----------------
why not #include the header instead?
(you'll need it in the .cpp anyway)


================
Comment at: lld/MachO/Driver.cpp:1035
+static void extractCallGraphProfile() {
+  for (const InputFile *file : inputFiles) {
+    auto *obj = dyn_cast_or_null<ObjFile>(file);
----------------
[optional] perhaps add a `TimeTraceScope` here?


================
Comment at: lld/MachO/Driver.cpp:1442
     gatherInputSections();
-
+    extractCallGraphProfile();
+  
----------------
only if `callGraphProfileSort==true` ?


================
Comment at: lld/MachO/InputFiles.cpp:308
+      if (name == "__cg_profile" && config->callGraphProfileSort) {
+        BinaryStreamReader reader(data, support::little);
+        while (!reader.empty()) {
----------------
lgrey wrote:
> I couldn't find prior art on finding endianness, though there's code elsewhere that assumes little. Is there a better way to do this?
I think the unwritten assumption here is that macho is all little-endian now.



================
Comment at: lld/MachO/InputFiles.h:61
+struct CallGraphEntry {
+  uint32_t fromIndex;
+  uint32_t toIndex;
----------------
would be helpful to have some comment clarifying on "Index" into what.


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

https://reviews.llvm.org/D112164