[Openmp-commits] [PATCH] D55588: [OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Joel E. Denny via Phabricator via Openmp-commits]

jdenny added inline comments.


================
Comment at: openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake:116
+if (CUDA_TOOLKIT_ROOT_DIR)
+  set(cuda_toolkit_root_dir_old TRUE)
+endif()
----------------
grokos wrote:
> There's a convention that cmake variables are capitalized. Also, cmake variables local to libomptarget should start with the prefix `LIBOMPTARGET_`. So this variable's name should be changed to `LIBOMPTARGET_CUDA_TOOLKIT_ROOT_DIR_PRESET` ("preset" makes more sense than "old").
Thanks for the review.  Sure, I'll make those changes.

However, I had thought that non-cached or local variables are usually lowercase.  Is there a distinction here I'm not seeing, or is that just a different style than you prefer here?


================
Comment at: openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake:185
+      OUTPUT_STRIP_TRAILING_WHITESPACE)
+    set(candidate_dir /usr/lib/cuda)
+    if ((LSB_RELEASE_ID STREQUAL "Debian" OR LSB_RELEASE_ID STREQUAL "Ubuntu")
----------------
Should candidate_dir be LIBOMPTARGET_CANDIDATE_DIR?


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

https://reviews.llvm.org/D55588