[Patch][RFC?] libgomp: Init hash table for 'indirect'-clause of 'declare target' on the host [PR114445, PR119857]

[Tobias Burnus]

Minor update (as attached): Fixed two bugs in plugin-gcn.c.

The patch has now also been tested with AMD GPUs (MI300) offloading
and on a PowerPC (powerpc64le) system with offloading
to a Volta GPU.

Tobias Burnus wrote:

> The main motivation for this patch is that on MI300,
> libgomp.c-c++-common/declare-target-indirect-2.c always fails
> because of the known fragile/racy code - as noticed before
> (PR114445 and PR119857), albeit triggering the fail has become
> more reliably.
>
> 'indirect' means that such-tagged functions are added to a
> lookup table on the device - and when invoking a function
> through a function pointer, a lookup is done. If matched,
> the looked-up address is used - otherwise, the original one.
> That permits to call the device function associated with a
> host function.
>
> Currently, the check whether the hash table has to be build
> happens every time in a target region - but only for the first
> team. However, that's not multi-team safe: Either by launching
> multiple teams in one kernel (+ doing a lookup early in
> a team != first one) – or by launching one with nowait, quickly
> followed by another one.
>
> The new approach is to instead create the hash table on the host
> during GOMP_OFFLOAD_load_image - which the attached patch
> implements. (Also on the pro side, it also avoids one
> device-side allocation call and uses a bit less memory on
> the device.)
>
> The current patch is backward compatible (both ways); I would
> like to remove the fallback on the device side (i.e. binary with
> new libgomp on the device side running with older host libgomp.so),
> but I guess we can't do this, can we?
>
> RFC — Any comments, remarks or concerns?
>
> * * *
>
> Notes:
>
> * The patch has only been tested on x86-64-gnu-linux with
>   an Nvidia GPU. Testing with an AMD GPU and offloading
>   from PowerPC or Aarch64 (Grace) to an Nvidia GPU still
>   needs to be done.
>
> * Currently, the hash is created on the host, copied to the
>   device and - done. There is for sure a memory leak of the
>   hash table itself on the host as the pointer to it is
>   neither freed nor stored somewhere.
>
> [And related:]
>
> * This patch also does not address PR114690 - namely,
>   loading multiple images - or unloading images.
>   Loading multiple kernels is fine if at most one has
>   an indirect function table. (Same as before.)
>
> Technically, replacing the hash table by a new one
> with additional entries - or just adding more if there
> is still enough space - relatively simple. (But requires,
> see above, storing some data on the host - at least the
> pointer to the current host hash table.)
>
> It is less clear to me how unloading an image is supposed
> to be handled - both one image (but not all) and all. In
> most programs, there is only one image - and in particular,
> images only get unloaded at the very end. - Thus, just
> leaving the mess on the device is fine - most of the time.
> The question is how to deal with the corner cases?
> Knowing the addresses, one can remove all indirect entries
> one by one (or all, if known that there is only one image
> that added it) - but doing so takes a while and, if not
> required, I really would like to avoid it!
>
> Currently, nearly all code will have no indirect functions.
> With OpenMP 5.2 support for virtual functions in C++, I think
> the number of code which will have them will increase, but
> still it is expected that there is only one image - and not
> multiple.
>
> Thoughts how to handle this part best? Or general comments?
>
> Tobias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20250901/1d7211f2/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-indirect-v2.diff
Type: text/x-patch
Size: 24466 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20250901/1d7211f2/attachment-0001.bin>