[Openmp-commits] [PATCH] D100185: [OpenMP] [OMPD] [5/6] Implementation of OMPD debugging library - libompd. gdb-plugin: A Plugin code to gdb to leverage libompd to provide debugging support.

[Vignesh Balasubramanian via Phabricator via Openmp-commits]

Vigneshbalu added inline comments.


================
Comment at: openmp/libompd/CMakeLists.txt:14
     add_subdirectory(src)
+    if(NOT DISABLE_OMPD_GDB_PLUGIN)
+        add_subdirectory(gdb-plugin)
----------------
aaronpuchert wrote:
> Could you introduce this as cache variable? Something like
> ```
> set(DISABLE_OMPD_GDB_PLUGIN OFF CACHE BOOL "<Some description>")
> ```
> See also https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry.
> 
> And perhaps we could choose the default based on the availability of Python 3 development files?
thanks for the comments.
I will make it as cache variable. 
yes, we can disable when python is not available, with a warning.
Let me check with others and update.




================
Comment at: openmp/libompd/gdb-plugin/CMakeLists.txt:17
+find_package (Python3 COMPONENTS Interpreter Development)
+find_package (PythonLibs REQUIRED)
+
----------------
aaronpuchert wrote:
> Isn't this the same as the `Development` component from the previous line? Down there I don't see direct usage of `PythonLibs`, only `Python3_INCLUDE_DIRS` and `Python3_LIBRARIES` from the previous line.
> 
> [CMake documentation](https://cmake.org/cmake/help/latest/module/FindPythonLibs.html) also says that this is deprecated in favor of `Python3` that you're already using. So this line can likely simply be removed.
thanks for the catch. It seems redundant. 
Let me verify and remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100185