[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't really like that we are adding a public shared library for every tiny intel feature. Could we at least merge this "plugin" with the existing "intel-mpx plugin" to create one "intel support" library?

Also, adding an external dependency probably deserves a discussion on lldb-dev.



================
Comment at: tools/CMakeLists.txt:8
 add_subdirectory(lldb-mi)
+option(LLDB_BUILD_INTEL_PT "Enable Building of Intel(R) Processor Trace Tool" OFF)
+if (LLDB_BUILD_INTEL_PT)
----------------
clayborg wrote:
> Can we default this to enabled?
We probably can't, as this code depends on a third party library.  In any case, this option should go to LLDBConfig.cmake


================
Comment at: tools/intel-pt/CMakeLists.txt:42
+
+add_library(lldbIntelPT SHARED
+  PTDecoder.cpp
----------------
any reason you're not using add_lldb_library here?


================
Comment at: tools/intel-pt/CMakeLists.txt:53
+
+if (NOT LLDB_DISABLE_PYTHON)
+  target_link_libraries(lldbIntelPT PRIVATE
----------------
All of this needs to go away. I think you only needed it because you are plucking NativeProcessLinux internals, so fixing that should fix this too.


https://reviews.llvm.org/D33035