[Openmp-commits] [PATCH] D41724: [OpenMP][libomptarget] Enable the compilation of multiple bc libraries for runtime inlining

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

On whatever name we decide, please document in `README.rst` that the user can pass multiple values.



================
Comment at: libomptarget/deviceRTLs/nvptx/CMakeLists.txt:63-64
   # SM_35 is what clang uses by default.
-  set(LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY 35 CACHE STRING
-    "CUDA Compute Capability to be used to compile the NVPTX device RTL.")
-  set(CUDA_ARCH -arch sm_${LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY})
----------------
gtbercea wrote:
> Hahnfeld wrote:
> > This should definitely stay a `CACHE` variable. And I still propose to rename it to `_CAPABILITIES`...
> Ah so you want me to keep this line. Sure I'll reinstate it.
> 
> Regarding the name ... this would lead to many people having to change their compiler build scripts. I'd like to avoid that.
Maybe we can add some compatibility? Upstream only has it since a few days and others will need to change their scripts anyway.

```lang=cmake
set(default_capabilities 35)
if (DEFINED LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY)
  set(default_capabilities ${LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY})
  libomptarget_warning_say("LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITY is deprecated, please use LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITIES")
endif()
set(LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITIES ${default_capabilities} CACHE STRING
  "List of CUDA Compute Capabilities to be used to compile the NVPTX device RTL.")
```


================
Comment at: libomptarget/deviceRTLs/nvptx/CMakeLists.txt:200
+        # Install device RTL under the lib destination folder.
+        install(FILES ${CMAKE_CURRENT_BINARY_DIR}/libomptarget-nvptx-${sm}.bc DESTINATION "lib")
+      endforeach()
----------------
gtbercea wrote:
> gtbercea wrote:
> > Hahnfeld wrote:
> > > Do we want to use eg `35` as suffix or full `sm_35`?
> > 35
> Actually I don't care which one we use. Depending on what we choose here I'll make clang agree with it :)
I agree: The user will probably never see the name anyway. So let's just keep it compatible.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D41724