Discussion about OpenMP 5.0 declare mapper runtime interface

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Discussion about OpenMP 5.0 declare mapper runtime interface

Renato Golin via cfe-dev
Hi,

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.

As a result, the old interfaces will be deprecated, but they need to be kept there for backward compatibility. I think this scheme is clear and has no hidden problems. The down side is it will create more OpenMP runtime interfaces. The patches for this scheme can be found at https://reviews.llvm.org/D67833 and https://reviews.llvm.org/D68100.

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 scheme will introduce less runtime interfaces. Its problem is the implementation is not straightforward and needs to take extra precautions. For example, each OpenMP task should have a separate mapper storage, to prevent a target region from reading the mapper written by another task.

Your option about which one is better will be greatly appreciated.

Thanks,
Lingda Li

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Openmp-dev] Discussion about OpenMP 5.0 declare mapper runtime interface

Renato Golin via cfe-dev


On 9/30/19 3:49 PM, Lingda Li via Openmp-dev wrote:
Hi,

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.

As a result, the old interfaces will be deprecated, but they need to be kept there for backward compatibility. I think this scheme is clear and has no hidden problems. The down side is it will create more OpenMP runtime interfaces. The patches for this scheme can be found at https://reviews.llvm.org/D67833 and https://reviews.llvm.org/D68100.

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 scheme will introduce less runtime interfaces. Its problem is the implementation is not straightforward and needs to take extra precautions. For example, each OpenMP task should have a separate mapper storage, to prevent a target region from reading the mapper written by another task.

Your option about which one is better will be greatly appreciated.


I prefer option 1. I don't like the idea of introducing extra mandatory state in the library simply to avoid adding a new runtime call with additional parameters. My experience is that this kind of extra state is more likely to be a source of bugs than the extra parameters of a new function, plus the extra function-call overhead and memory traffic is likely more expensive than the extra parameter passing. We need to extend the API either way.

Furthermore, I suspect that breaking of the runtime call into several stateful calls will make the calls more difficult to analyze within the optimizer. Johannes, do you agree?

I realize that we've done this kind of thing in the past - and similar things exist in the CUDA runtime interface, etc. - but my experience with this kind of API extension has been largely negative.

 -Hal



Thanks,
Lingda Li

_______________________________________________
Openmp-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Openmp-dev] Discussion about OpenMP 5.0 declare mapper runtime interface

Renato Golin via cfe-dev
On 09/30, Finkel, Hal J. wrote:

>
> On 9/30/19 3:49 PM, Lingda Li via Openmp-dev wrote:
>
>     Hi,
>
>     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.
>
>     As a result, the old interfaces will be deprecated, but they need to be
>     kept there for backward compatibility. I think this scheme is clear and has
>     no hidden problems. The down side is it will create more OpenMP runtime
>     interfaces. The patches for this scheme can be found at https://
>     reviews.llvm.org/D67833 and https://reviews.llvm.org/D68100.
>
>     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 scheme will introduce less runtime interfaces. Its problem is the
>     implementation is not straightforward and needs to take extra precautions.
>     For example, each OpenMP task should have a separate mapper storage, to
>     prevent a target region from reading the mapper written by another task.
>
>     Your option about which one is better will be greatly appreciated.
>
>
> I prefer option 1. I don't like the idea of introducing extra mandatory state
> in the library simply to avoid adding a new runtime call with additional
> parameters. My experience is that this kind of extra state is more likely to be
> a source of bugs than the extra parameters of a new function, plus the extra
> function-call overhead and memory traffic is likely more expensive than the
> extra parameter passing. We need to extend the API either way.
>
> Furthermore, I suspect that breaking of the runtime call into several stateful
> calls will make the calls more difficult to analyze within the optimizer.
> Johannes, do you agree?
>
> I realize that we've done this kind of thing in the past - and similar things
> exist in the CUDA runtime interface, etc. - but my experience with this kind of
> API extension has been largely negative.
I totally agree, option 1 is preferable.

For other situations it is already clear that multiple calls make
optimizations harder and that we will introduce "meta" calls to
encapsulate intent and not implementation details of the runtime. (The
next on my list is task_alloc(...) and the subsequent task(...), but
TRegions were another example.)

The argument that there will be "more runtime interface" is, especially
in this case, not a strong one given that we can define old functions in
terms of the new ones with basically no maintenance overhead:

__tgt_target_teams(...) = __tgt_target_teams_mappers(..., nullptr);

Cheers,
  Johannes


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

signature.asc (235 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Openmp-dev] Discussion about OpenMP 5.0 declare mapper runtime interface

Renato Golin via 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.

-----Original Message-----
From: Openmp-dev [mailto:[hidden email]] On Behalf Of Doerfert, Johannes via Openmp-dev
Sent: Monday, September 30, 2019 3:35 PM
To: Finkel, Hal J. <[hidden email]>
Cc: cfe-dev <[hidden email]>; Lingda Li <[hidden email]>; [hidden email]
Subject: Re: [Openmp-dev] Discussion about OpenMP 5.0 declare mapper runtime interface

On 09/30, Finkel, Hal J. wrote:

>
> On 9/30/19 3:49 PM, Lingda Li via Openmp-dev wrote:
>
>     Hi,
>
>     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.
>
>     As a result, the old interfaces will be deprecated, but they need to be
>     kept there for backward compatibility. I think this scheme is clear and has
>     no hidden problems. The down side is it will create more OpenMP runtime
>     interfaces. The patches for this scheme can be found at https://
>     reviews.llvm.org/D67833 and https://reviews.llvm.org/D68100.
>
>     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 scheme will introduce less runtime interfaces. Its problem is the
>     implementation is not straightforward and needs to take extra precautions.
>     For example, each OpenMP task should have a separate mapper storage, to
>     prevent a target region from reading the mapper written by another task.
>
>     Your option about which one is better will be greatly appreciated.
>
>
> I prefer option 1. I don't like the idea of introducing extra
> mandatory state in the library simply to avoid adding a new runtime
> call with additional parameters. My experience is that this kind of
> extra state is more likely to be a source of bugs than the extra
> parameters of a new function, plus the extra function-call overhead
> and memory traffic is likely more expensive than the extra parameter passing. We need to extend the API either way.
>
> Furthermore, I suspect that breaking of the runtime call into several
> stateful calls will make the calls more difficult to analyze within the optimizer.
> Johannes, do you agree?
>
> I realize that we've done this kind of thing in the past - and similar
> things exist in the CUDA runtime interface, etc. - but my experience
> with this kind of API extension has been largely negative.

I totally agree, option 1 is preferable.

For other situations it is already clear that multiple calls make optimizations harder and that we will introduce "meta" calls to encapsulate intent and not implementation details of the runtime. (The next on my list is task_alloc(...) and the subsequent task(...), but TRegions were another example.)

The argument that there will be "more runtime interface" is, especially in this case, not a strong one given that we can define old functions in terms of the new ones with basically no maintenance overhead:

__tgt_target_teams(...) = __tgt_target_teams_mappers(..., nullptr);

Cheers,
  Johannes

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev