[PATCH] D44892: Add PerfJITEventListener for perf profiling support.

[Andres Freund via Phabricator via llvm-commits]

anarazel added a comment.

Ping?  Any suggestions for other reviewers?



================
Comment at: CMakeLists.txt:448
+  endif( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )
+endif( LLVM_USE_PERF )
+
----------------
Should this be enable this by default on linux?


================
Comment at: lib/ExecutionEngine/MCJIT/MCJIT.cpp:224
+  // Can't call notifiers yet as relocations have not yet been performed, and
+  // memory hasn't been marked executable.
+  PendingLoadedObjects.push_back(LoadedObject->get());
----------------
Arguably this should be merged separately?


================
Comment at: lib/ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp:160
+  uint64_t NrEntry;
+  // followed by NrEntry LLVMPerfJitDebugEntry records
+};
----------------
Using the structs and reinterpret cast seems easiest, but then I'm a C programmer at hard. Does that look ok?


================
Comment at: lib/ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp:479
+                      sizeof(LineInfo));
+    Dumpstream->write(Line.FileName.c_str(), Line.FileName.size() + 1);
+  }
----------------
This should be optimized to avoid re-emitting the filename if previous filename is the same.


Repository:
  rL LLVM

https://reviews.llvm.org/D44892