[cfe-dev] Discussion about OpenMP 5.0 declare mapper runtime

[Lingda Li via cfe-dev]

Thanks for the comments Johannes.

Anyone has comments?

On Thu, Oct 3, 2019 at 5:47 PM Doerfert, Johannes <jdoerfert at anl.gov> wrote:

> (I tried to collect the cross-posts from the cfe and openmp list here,
>  unsure if I got the latest ones though. No more cross posts and/or
>  dropping a list please.)
>
> > Ravi's email to cfe-dev
> >
> > Everytime we want to pass something new to the libomptarget runtime we
> > would need to add a new interface.  Why not pass a pointer to a
> > structure, 1st field says how many valid fields this structure
> > contains and populate the fields with info or null.  Run time can
> > check if the field is null or not and take appropriate action.
> >
> > Struct {
> >     Int Num_fields 2
> >     Int * mapper;
> >     Int * nowait_object
> >  }
> >
> > To support async (aka nowait)  we need an object to call back to the
> > libomp library.  This object can be passed in this struct.
>
> Initially, I thought this was a similarly good design. While fairly
> future prove, I have some problems I want to mention. First, however,
> I'd like to point out that new API functions alone are not really
> problematic though, especially if they are just wrappers around the
> newest version.
>
> Drawbacks of a "version struct":
>  - It makes the API backwards compatible, at least until runtime. I
>    mean, when you recompile a component of your application with a
>    (maybe newer) compiler it is unclear what version the rest of the
>    application, and the used structs, are using. If you assume the
>    struct passed around is at least of the current version, it will
>    break if it is not, silently at runtime. If you don't assume
>    anything, you need to emit dynamic checks and versioning when you
>    want to modify the code. The same situation with the "new API" model
>    will cause a link error if the runtime you link into is not as new
>    as the API calls you used. This allows you to rewrite the code based
>    on the API call semantic you know without versioning and fear of
>    silent miscompilation. I know this explanation is very abstract, let
>    me know if I should try to make it more concrete.
>

I agree. In this case, the runtime will need to check 'Num_fields' to make
sure it only uses what the struct contains. But I have to say Ravi's scheme
is quite extendable.


>  - We cannot remove arguments in a reasonable way. Please correct me if
>    I'm wrong here.
>

I agree. I think you can only add arguments in this case.


>
>
> On 10/01, Jon Chesterfield via cfe-dev wrote:
> > >
> > >
> > > I would like to bring your attention to the choice of 2 proposals for
> the
> > > declare mapper runtime interface:
> > >
> > > 1. The current design which creates new runtime functions for declare
> > > mappers. For example, right now we have `__tgt_target_teams(...)` which
> > > corresponds to the runtime interface for `omp target teams`. Now we add
> > > `__tgt_target_teams_mapper(..., void **mappers)` to replace it.
> > >
> >
> > New feature, new library call seems reasonable. The current interface can
> > presumably be reimplemented as a call to (the internals of) the proposed
> > interface.
>
> Yes, I mentioned that as well:
>   old(args) = new(args, nullptr)
>
> > > 2. Introduce a function `__tgt_push_mappers`, which should be called
> before
> > > every target function call (e.g., `__tgt_target_teams`) to pass the
> mapper
> > > argument for that function. The call of `__tgt_push_mappers` is
> implicitly
> > > bonded with the actual target call.
> > >
> >
> > This seems error prone to use and likely to degrade performance.
> >
> > Choice 1 seems more appealing to me.
>
> So far, most people seem to prefer choice 1. We should go with that one
> if there are no complaints soon.
>
>
> Cheers,
>   Johannes
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20191003/4528aa65/attachment.html>