[PATCH] D59311: [ELF] Print symbols ordered by profiled guided section layout with verbose.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/CallGraphSort.cpp:230
+  if (errorHandler().Verbose) {
+    // If user passes --verbose to LLD , print out the symbols ordered by C3.
+    outs() << "******** Symbols ordered by call-chain clustering ********\n";
----------------
This is obvious, no need to explain by comment.


================
Comment at: lld/ELF/CallGraphSort.cpp:231
+    // If user passes --verbose to LLD , print out the symbols ordered by C3.
+    outs() << "******** Symbols ordered by call-chain clustering ********\n";
+    // Print the symbols ordered by C3, in the order of increasing CurOrder
----------------
This looks too fancy compared to other messages such as one in MapFile.cpp. Let's make it less fancy.

I'd make the first line "Symbols ordered by call-chain clustering:\n", followed by a list of symbols, terminated by a blank line.


================
Comment at: lld/ELF/CallGraphSort.cpp:236
+      for (int SecIndex : C.Sections)
+        outs() << Sections[SecIndex]->Name << "\n";
+    outs() << "**********************************************************\n";
----------------
Are you sure that all input sections have a name?


================
Comment at: lld/test/ELF/cgprofile-print.s:8
+# RUN: echo "D B 10" >> %t.call_graph
+# RUN: ld.lld -e A %t --call-graph-ordering-file %t.call_graph -o %t2 --verbose | FileCheck %s --check-prefix=VERBOSE
+# RUN: llvm-readobj -symbols %t2 | FileCheck %s
----------------
This does not seem to test the feature that you want to use.  Don't you want to print out the sorting order as a result of parsing .llvm.call-graph-profile sections, do you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59311