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

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added a comment.

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.

> User code may want to defined either own OpenMPTarget target and alias it to the target created by this module if they use libomptarget or define others if other vendor OpenMP offload runtime library is used.

You should be able to manually set the flags by setting the `OpenMPTarget_<Device>_FLAGS` or `OpenMPTarget_<Device>_DEVICE` since it will forward those if they're already set.

> OpenMP is both a language extension and library, creating a cmake module is a challenge.

Yeah, I primarily copied the method used for the existing `FindOpenMP.cmake` module, fortunately I can reuse a lot of features.



================
Comment at: openmp/tools/Modules/FindOpenMPTarget.cmake:62
+
+The module will also try to provide the OpenMP version variables:
+
----------------
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).


================
Comment at: openmp/tools/Modules/FindOpenMPTarget.cmake:99
+    set(OMPTarget_FLAG_XL "-qoffload")
+    set(OMPTarget_FLAG_PGI "-mp=${DEVICE}")
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104710