Page MenuHomePhabricator

[Openmp] Libomptarget plugin for NEC SX-Aurora
Needs ReviewPublic

Authored by manorom on Thu, Mar 26, 6:26 AM.

Details

Summary

This patch adds a a libomptarget plugin for the NEC SX-Aurora TSUBASA Vector Engine, which will also support OpenMP offloading in the future.
The code is mostly based on the plugins for generic-elf and uses NEC's VEO and VEOSINFO libraries to run the offloaded code on the target device.

The plugin was developed by the HPC group at RWTH Aachen for offloading OpenMP using NEC's ncc compiler for offloading compilation, but can also be used with Clang as target compiler.

Diff Detail

Event Timeline

manorom created this revision.Thu, Mar 26, 6:26 AM

Thanks for working on this!
Comments are mostly nitpicks. I am not sure we can assume that the VEOS device ids are consecutive.

openmp/libomptarget/plugins/ve/src/rtl.cpp
10

typo: Aurora TSUBASE

122

Can we assume that?

168

else after return

214

else after return

222

typo: craete

298

else after return

336

else after return

362

Looks pretty good to me. Thin shim over veo_*.

openmp/libomptarget/plugins/ve/CMakeLists.txt
43

Worth considering adding "-Wl,-z,defs" here to detect unresolved symbols up front.

openmp/libomptarget/plugins/ve/src/rtl.cpp
47

Nice. I hadn't noticed this file.

183

I think a few plugins do this, just found it in hsa. It's stylistically unusual though. omptargetplugin.h declares the API under extern "C", and there's an export file that internalises the other symbols, so I'm pretty sure this #ifdef block and the one at the end can be removed.

grokos added inline comments.Thu, Mar 26, 1:17 PM
openmp/libomptarget/plugins/ve/CMakeLists.txt
10

additonal --> additional

Thanks for pushing this upstream, Manoel! Some comments below, inline.

openmp/libomptarget/plugins/ve/src/rtl.cpp
50

Please get rid of the VEO_MAX_VERSION. The VEO calls that you use are stable and will not be changed, so you won't need this. Also you'd want to build libomptarget with AVEO because it is faster and less limited. I plan to change AVEOs API version number because there are new calls inside, but have no agreement with the VEOS devs whether it shall become "8" or get an offset to VEO API versions.

122

No. Devices can be missing or offline. For sizing the objects below this is ok, but not for selecting the ve nodeids.

189

I think you need the online devices here, so you'll need to keep the node_info structure around and check the status array, while going through the nodeids.

219

See above: would be good to delete this else block because VEO_MAX_VERSION is not actually needed.

445

Please at least accumulate the return codes as the call actually can fail.

manorom added inline comments.Tue, Apr 7, 9:39 AM
openmp/libomptarget/plugins/ve/src/rtl.cpp
122

Do we have to assume that VE node ids can be assigned completely at random?

We hoped that that would not be the case. Our rationale for simply re-using the device ids we get passed from libomptarget was, that the user would be able to set OMP_DEFAULT_DEVICE or the device() clause to a VE node id.

Now it seems to me that libomptarget expects openmp device ids to be consecutive, so if VE node ids are random or if we only report those which are present and online, we need to have a mapping between openmp device ids and VE node ids which should be somewhat predictable for a user.