On Tue, Jun 28, 2022 at 10:29:53PM +0100, Andrew Stubbs wrote:
> On 09/06/2022 09:19, Jakub Jelinek via Gcc-patches wrote:
> > + switch (memspace)
> > + {
> > + case omp_high_bw_mem_space:
> > +#ifdef LIBGOMP_USE_MEMKIND
> > + struct gomp_memkind_data *memkind_data;
> > + memkind_data = gomp_get_memkind ();
> > + if (data.partition == omp_atv_interleaved
> > + && memkind_data->kinds[GOMP_MEMKIND_HBW_INTERLEAVE])
> > + {
> > + data.memkind = GOMP_MEMKIND_HBW_INTERLEAVE;
> > + break;
> > + }
> > + else if (memkind_data->kinds[GOMP_MEMKIND_HBW_PREFERRED])
> > + {
> > + data.memkind = GOMP_MEMKIND_HBW_PREFERRED;
> > + break;
> > + }
> > +#endif
> > + return omp_null_allocator;
> > + case omp_large_cap_mem_space:
> > +#ifdef LIBGOMP_USE_MEMKIND
> > + memkind_data = gomp_get_memkind ();
> > + if (memkind_data->kinds[GOMP_MEMKIND_DAX_KMEM_ALL])
> > + data.memkind = GOMP_MEMKIND_DAX_KMEM_ALL;
> > + else if (memkind_data->kinds[GOMP_MEMKIND_DAX_KMEM])
> > + data.memkind = GOMP_MEMKIND_DAX_KMEM;
> > +#endif
> > + break;
> > + default:
> > +#ifdef LIBGOMP_USE_MEMKIND
> > + if (data.partition == omp_atv_interleaved)
> > + {
> > + memkind_data = gomp_get_memkind ();
> > + if (memkind_data->kinds[GOMP_MEMKIND_INTERLEAVE])
> > + data.memkind = GOMP_MEMKIND_INTERLEAVE;
> > + }
> > +#endif
> > + break;
> > + }
> > +
>
> This conflicts with mine and Abid's patches to implement the device
> allocators and host unified shared memory via the overridable
> "MEMSPACE_ALLOC" hooks. I can still use those for the "else" case, but
> they'll be inactive on any configuration where libmemkind exists. That's
> fine for the device code, and may be OK for USM (given that libmemkind won't
> have an option for that). There's a problem for the NVidia-specific
> host-memory pinning I have planned though.
>
> How do you propose we resolve this conflict, please?
>
> Ideally it will be in such a way that the conditional is resolved at compile
> time and the routine can be inlined (so no fake-OO function pointers in
> structs, I think).
memkind isn't used just because it exists, but because it supports some
requested trait that isn't otherwise supported.
Using callbacks instead of selecting the allocator to call using ifs is
possible if all those callbacks have the same arguments or if we add enough
dummy arguments to the callbacks such that some wrappers can handle it all.
Right now, memkind is used if ->memkind in the allocator data isn't
GOMP_MEMKIND_NONE, so when it is, you can certainly call some callback
(performance question might be whether to call it in the else case always
using callback or whether to call malloc etc. directly if callback is NULL).
And omp_init_allocator needs to decide what to do if one asks for features
that need memkind as well as for features that need whatever you/Abid have
been working on. A possible resolution is punt (return omp_null_allocator),
or prefer one feature over the other one or vice versa.
>From the features currently handled by memkind, even before my changes for
consistency with libomp from llvm omp_high_bw_mem_space was considered a
hard request, ditto omp_atk_pinned omp_atv_true, but the rest was just taken
as optimization hint.
Jakub
