[PATCH] D90686: [lld][ELF] Add additional time trace categories

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/DriverUtils.cpp:242
 Optional<std::string> elf::searchLibrary(StringRef name) {
+  llvm::TimeTraceScope timeScope("Locate library", name);
   if (name.startswith(":"))
----------------
This is a pretty simple operation. Is the scope needed? (If file traversal IO turns out to be an issue, this still needs more information to locate the issue.)


================
Comment at: lld/ELF/Writer.cpp:207
 void elf::combineEhSections() {
+  llvm::TimeTraceScope timeScope("Combine exception sections");
   for (InputSectionBase *&s : inputSections) {
----------------
EH is probably more appropriate than exception.

.gcc_except_table is not handled here.


================
Comment at: lld/ELF/Writer.cpp:2010
+  {
+    llvm::TimeTraceScope timeScope("Finalize symbols");
+    // Now that we have defined all possible global symbols including linker-
----------------
This may be ambiguous.

Symbol resolution has been done. This finalizes .symtab and part of .dynsym . You'll need to include the next block to finalize the rest of .dynsym (there is still a horrible Haxagon hack below but that is just one symbol which can be ignored from the profiling)


================
Comment at: lld/ELF/Writer.cpp:2118
+  {
+    llvm::TimeTraceScope timeScope("Finalize synthetic sections");
+
----------------
If we do something to support RISC-V -mrelax in the future (D77694), there is possibility that this needs to break down.


================
Comment at: lld/ELF/Writer.cpp:2175
+    //    correct values. They also might change after adding the thunks.
+    finalizeAddressDependentContent();
+    if (errorCount())
----------------
finalizeAddressDependentContent is a special address assigning operation. It does not belong to "Finalize synthetic sections".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90686