[Openmp-commits] [PATCH] D104710: [OpenMP] Introduce an CMake find module for OpenMP Target support

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added a comment.

In D104710#2833784 <https://reviews.llvm.org/D104710#2833784>, @jhuber6 wrote:

> In D104710#2833720 <https://reviews.llvm.org/D104710#2833720>, @ye-luo wrote:
>
>> Is this intended for libomptarget only or even to take care other offload runtime libraries?
>> if it is the former case, It needs to be renamed as FindLibOMPTarget to be specific about libomptarget.
>> If it is the later case, please upstream it to CMake directly.
>
> I intend for this to work for other compilers, but I don't have as much experience with the other compilers so I can't guarantee its effectiveness. Upstreaming to CMake could be a goal, but I think for now it's easier to keep it here until some more people have has a chance to test it.

other compiler with libomp+libomptarget or other compilers with their own OpenMP runtime libraries?



================
Comment at: openmp/tools/Modules/FindOpenMPTarget.cmake:62
+
+The module will also try to provide the OpenMP version variables:
+
----------------
jhuber6 wrote:
> ye-luo wrote:
> > I don't quite follow what is `OpenMP version` especially the device specific one.
> > As vendors usually only implement a subset, users should not rely on OpenMP spec version anyway.
> > If they really need it, just invoke FindOpenMP themselves. RIght now it is an illusion there is a separate OpenMPTarget_VERSION
> Since `FindOpenMP` returns a version, I think it's reasonable to return the same version for this module because it's intended to be a superset. It should include all the varialbes `FindOpenMP` offers, plus the ones needed for offloading. (I may need to check if I set the include dirs and libs correctly in addition).
I mean it is better not to add another version variable if the OpenMP_VERSION is sufficient and has been included already.


================
Comment at: openmp/tools/Modules/FindOpenMPTarget.cmake:99
+    set(OMPTarget_FLAG_XL "-qoffload")
+    set(OMPTarget_FLAG_PGI "-mp=${DEVICE}")
+
----------------
jhuber6 wrote:
> ye-luo wrote:
> > FLAGS, plural because you might need multiple flags.
> > 
> > Use NVHPC instead of PGI, the correct flag is -mp=gpu
> > does -mp=${DEVICE} work
> > 
> > GNU offload to NVIDIA is -foffload=nvptx-none
> > does -foffload=${DEVICE} work?
> > 
> PGI what the `COMPILDER_ID` returns when I ran with the NVHPC compiler last I checked. I think it only supports nvptx so there's no option to configure its target.
> 
> GNU allows this flag, technically linking math and atomic on the device are optional, but they are needed in most applications so it's much easier to do it by default.
> 
> I'll make it plural.
3.20 changed PGI to NVHPC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104710