On Mon, Aug 02, 2021 at 09:10:57PM +0800, Chung-Lin Tang wrote:
> > I think this won't work properly with the intel micoffload, where the host
> > libgomp is used in the offloaded code.
> > For omp_is_initial_device, the plugin solves it by:
> > liboffloadmic/plugin/offload_target_main.cpp
> > overriding it:
> > /* Override the corresponding functions from libgomp. */
> > extern "C" int
> > omp_is_initial_device (void) __GOMP_NOTHROW
> > {
> > return 0;
> > }
> > extern "C" int32_t
> > omp_is_initial_device_ (void)
> > {
> > return omp_is_initial_device ();
> > }
> > but guess it will need slightly more work because we need to copy the value
> > to the offloading device too.
> > It can be done incrementally though.
>
> I guess this part of intelmic functionality will just have to wait later.
> There seem to be other parts of liboffloadmic that seems to need re-work,
> e.g. omp_get_num_devices() return mic_engines_total, where it should actually
> return the number of all devices (not just intelmic). omp_get_initial_device()
> returning -1 (which I don't quite understand), etc.
For omp_get_num_devices() the standard says:
When called from within a target region the effect of this routine is
unspecified.
Ditto for omp_get_initial_device and various other routines.
So it is UB if those functions are called in offloaded regions.
> > For a single var it is acceptable (though, please avoid the double space
> > before offload plugin in the comment), but once we have more than one
> > variable, I think we should simply have a struct which will contain all the
> > parameters that need to be copied from the host to the offloading device at
> > image load time (and have eventually another struct that holds parameters
> > that we'll need to copy to the device on each kernel launch, I bet some ICVs
> > will be one category, other ICVs another one).
>
> Actually, if you look at the 5.[01] specifications, omp_get_device_num() is
> not
> defined in terms of an ICV. Maybe it conceptually ought to be, but the current
> description of "the device number of the device on which the calling thread is
> executing" is not one if the defined ICVs.
>
> It looks like there will eventually be some kind of ICV block handled in a
> similar
> way, but I think that the modifications will be straightforward then. For now,
> I think it's okay for GOMP_DEVICE_NUM_VAR to just be a normal global variable.
Yeah, it is ok for now, but even for the below mentioned omp_display_env
we'll need to replace it...
> There is a new function check_effective_target_offload_target_intelmic() in
> testsuite/lib/libgomp.exp, used to test for non-intelmic offloading
> situations.
>
> Re-tested with no regressions, seeking approval for trunk.
>
> Thanks,
> Chung-Lin
>
> 2021-08-02 Chung-Lin Tang <clt...@codesourcery.com>
>
> libgomp/ChangeLog
>
> * icv-device.c (omp_get_device_num): New API function, host side.
> * fortran.c (omp_get_device_num_): New interface function.
> * libgomp-plugin.h (GOMP_DEVICE_NUM_VAR): Define macro symbol.
> * libgomp.map (OMP_5.0.2): New version space with omp_get_device_num,
> omp_get_device_num_.
> * libgomp.texi (omp_get_device_num): Add documentation for new API
> function.
> * omp.h.in (omp_get_device_num): Add declaration.
> * omp_lib.f90.in (omp_get_device_num): Likewise.
> * omp_lib.h.in (omp_get_device_num): Likewise.
> * target.c (gomp_load_image_to_device): If additional entry for device
> number exists at end of returned entries from 'load_image_func' hook,
> copy the assigned device number over to the device variable.
>
> * config/gcn/icv-device.c (GOMP_DEVICE_NUM_VAR): Define static global.
> (omp_get_device_num): New API function, device side.
> * config/plugin/plugin-gcn.c ("symcat.h"): Add include.
> (GOMP_OFFLOAD_load_image): Add addresses of device GOMP_DEVICE_NUM_VAR
> at end of returned 'target_table' entries.
>
> * config/nvptx/icv-device.c (GOMP_DEVICE_NUM_VAR): Define static global.
> (omp_get_device_num): New API function, device side.
> * config/plugin/plugin-nvptx.c ("symcat.h"): Add include.
> (GOMP_OFFLOAD_load_image): Add addresses of device GOMP_DEVICE_NUM_VAR
> at end of returned 'target_table' entries.
>
> * testsuite/lib/libgomp.exp
> (check_effective_target_offload_target_intelmic): New function for
> testing for intelmic offloading.
> * testsuite/libgomp.c-c++-common/target-45.c: New test.
> * testsuite/libgomp.fortran/target10.f90: New test.
Ok, thanks.
Jakub
