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

[Tiancong Wang via Phabricator via llvm-commits]

tcwang added a comment.

Thanks again for the comments!



================
Comment at: lld/ELF/CallGraphSort.cpp:235
+      for (int SecIndex : C.Sections)
+        outs() << Sections[SecIndex]->Name << "\n";
+  }
----------------
pcc wrote:
> Should this go to a file with a user-specified name (without the header) instead of stdout? I'd expect the output of this to be consumed by a script, and sending it to a file would probably make it easier for said script to consume.
> 
> Also, should this be listing the symbols instead of the sections if the intent is to pass this to --symbol-ordering-file?
I strongly agree that these suggestions would improve our lives if we want to use the outputs to feed --symbol-ordering-file in later pass. (Actually, right now I have to use a separate script to process the standard output and removing the headers to generate order file.)

I'd love to improve this part, if it is OK to add a flag to support it. (And do you have any suggestions on the name of the flag?) Also, do you have some suggestions how to remove the headers cleanly? I don't see there's an option to remove the added prefix in the Name field of InputSectionBase? I don't think I should just try to remove ".text" (or ".text.hot" in some cases). Thank you!


================
Comment at: lld/ELF/CallGraphSort.cpp:236
+      for (int SecIndex : C.Sections)
+        outs() << Sections[SecIndex]->Name << "\n";
+    outs() << "**********************************************************\n";
----------------
ruiu wrote:
> tcwang wrote:
> > ruiu wrote:
> > > Are you sure that all input sections have a name?
> > Not sure... But if the section doesn't, should I print an empty line, or just skip it?
> I don't think printing out a blank line is useful, but what I wanted to ask is if it is ever possible to be unnamed. We shouldn't guard it against an empty name "just in case".
I do see empty Name when testing against some programs. Maybe we should check empty Name before printing to avoid empty lines?


================
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
----------------
ruiu wrote:
> tcwang wrote:
> > ruiu wrote:
> > > 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?
> > I am sorry that I think I am not sure I understand the comment completely. I was trying to add --verbose to LLD while passing in a call graph file and check the standard print with the expected ordering. (And I have removed the checks in the object file, since this check should be done in another test already.) Should I test something else instead?
> I thought that you wanted to print a result of a compiler-generated callgraph data stored into .llvm.call-graph-profile sections (there's such feature in lld and LLVM). If that's not the case, never mind.
I think this option --call-graph-ordering-file is consuming the call graph and directly generate a map of sections with order. I think I only need to print out the ordering in the map for this purpose. So that's why I test it against.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59311