Discussion about OpenMP 5.0 declare mapper runtime

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.

  1. 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.

Cheers

(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.
- We cannot remove arguments in a reasonable way. Please correct me if
   I'm wrong here.

>
>
> 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

Thanks for the comments Johannes.

Anyone has comments?

(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.

Lingda, when you replied to this message you re-added the cfe-dev
mailing list. Please don't do that. As Johannes said, we discourage
cross-posting discussion threads to multiple LLVM mailing lists. Not
everyone is on all of the lists, and so cross-posting adds to the
workload for our list moderators when someone not subscribed to all of
the lists responds to the thread. This thread on the interface to the
OpenMP runtime library belongs on the openmp-dev list. In the future,
please start threads on only one list. It's fine to send a separate note
to other relevant lists alerting subscribers to a thread that might be
of interest to them on a different list - but please don't cc the other
lists directly.

Thanks again,

Hal

Got it, thanks. I read Johannes’ message incorrectly as “not dropping a list”.