[Openmp-commits] [PATCH] D145513: [Libomptarget] Update handling of architectures for DeviceRTL

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:74
+elseif(LIBOMPTARGET_DEVICE_ARCHITECTURES STREQUAL "auto")
+  if (NOT LIBOMPTARGET_NVPTX_ARCH AND NOT LIBOMPTARGET_AMDGPU_ARCH)
+    libomptarget_error_say(
----------------
jhuber6 wrote:
> ye-luo wrote:
> > Are both tool always built together?
> Yes, they are always built by `clang`.
Did you mean they are always built by the clang target outside openmp project?
If I don't build NVPTX or AMDGPU backend, are they still being built?
Can it be better if we do this.
if nvptx-arch exists, add its list to the LIBOMPTARGET_DEVICE_ARCHITECTURES.
if it doesn't exists, print a warning but don't error out.
do the same for amdgpu-arch.
if LIBOMPTARGET_DEVICE_ARCHITECTURES is still empty, error out.


================
Comment at: openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake:121
     set(LIBOMPTARGET_FOUND_NVIDIA_GPU TRUE)
-    set(LIBOMPTARGET_DEP_CUDA_ARCH "${arch_string}")
+    set(LIBOMPTARGET_NVPTX_ARCH_LIST "${nvptx_arch_list}")
+    list(GET nvptx_arch_list 0 LIBOMPTARGET_DEP_CUDA_ARCH)
----------------
There are a few variables related to ARCH. Could you rename this to LIBOMPTARGET_NVPTX_ARCH_DETECTED_LIST.
Also the AMDGPU one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145513