aganea added inline comments.
================
Comment at: lld/ELF/Driver.cpp:515
+
+ if (llvm::timeTraceProfilerEnabled()) {
+ SmallString<128> path(config->outputFile);
----------------
Could you possibly move this block (and the init block above) into a new file
in `lld/Common/` so we can use it in the COFF driver as well?
================
Comment at: lld/ELF/Driver.cpp:527
+ }
+ return;
}
----------------
Extraneous return.
================
Comment at: lld/ELF/Options.td:361
+def time_trace: F<"time-trace">, HelpText<"Record time trace">;
+
----------------
Given this option is a candidate for the other LLD drivers, I am wondering if
we couldn't have a shared `lld/Common/CommonOpt.td`. @ruiu WDYT?
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:70
void begin(std::string Name, llvm::function_ref<std::string()> Detail) {
+ std::lock_guard<std::mutex> Lock(Mu);
Stack.emplace_back(steady_clock::now(), TimePointType(), std::move(Name),
----------------
The lock is definitely not the way to go, it would skew all the timings
especially on tens-of-cores machines. Like you're suggesting, make
`TimeTraceProfilerInstance` a TLS, along with a new combiner upon the call to
`timeTraceProfilerWrite()`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69043/new/
https://reviews.llvm.org/D69043
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
