[PATCH] D59311: [ELF] Dump symbols ordered by profiled guided section layout to file.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 21 15:02:10 PDT 2019


ruiu added inline comments.


================
Comment at: lld/ELF/CallGraphSort.cpp:244
+        for (Symbol *Sym: Sections[SecIndex]->File->getSymbols())
+          if (!Sym->getName().empty())
+            if (auto *D = dyn_cast<Defined>(Sym))
----------------
So I think that checking name is fragile. What do you actually want to skip by doing this?


================
Comment at: lld/ELF/Driver.cpp:1477
+  if (auto E = tryCreateFile(Config->PrintSymbolOrder))
+    error("cannot open symbol order output file " + Config->PrintSymbolOrder + ": " + E.message());
   if (errorCount())
----------------
80 cols.


================
Comment at: lld/ELF/Options.td:278
+defm print_symbol_order:
+  Eq<"print-symbol-order", "Print symbols ordered by --call-graph-ordering-file into <File>.">;
+  
----------------
This help message seem a little bit foreign compared to other help messages. A more natural help message would be probably something this:

  Print a symbol order specified by --call-graph-ordering-file into to the specified file

Be careful not to put a full stop at the end of a help message.


================
Comment at: lld/test/ELF/cgprofile-print.s:35-37
+
+
+
----------------
Remove trailing extra newlines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59311





More information about the llvm-commits mailing list