[PATCH] D22491: Added ThinLTO inlining statistics

[Piotr Padlewski via llvm-commits]

Prazek marked 5 inline comments as done.

================
Comment at: lib/Transforms/Utils/ThinLTOInlinerStats.cpp:26
@@ +25,3 @@
+/// (1) Number of inlined imported functions,
+/// (2) Number of inlined imported functions to importing module (indirect),
+/// (3) Number of inlined non imported functions to importing module (indirect).
----------------
tejohnson wrote:
> Here and in below line, I think it is clearer to restructure like "Number of imported functions inlined into...".
wtf I was pretty sure I changed that.

================
Comment at: lib/Transforms/Utils/ThinLTOInlinerStats.cpp:91
@@ +90,3 @@
+                                           const Function &Callee) {
+  std::lock_guard<std::mutex> Guard(Mutex);
+  auto &CallerNode = NodesMap[&Caller];
----------------
tejohnson wrote:
> What is the multithreaded case we are concerned about? Is it when the ThinLTO backend threads are launched in parallel via a ThreadPool? In that case we want each thread to have a completely independent set of stats, which I don't think this mutex provides.
I am not sure if there is any problem right now, but we also probably don't want to risk breaking it in the future. The overhead should not be very big and it won't be runned frequently.


https://reviews.llvm.org/D22491