[Openmp-commits] [PATCH] D68100: [OpenMP 5.0] declare mapper runtime implementation

Alexey Bataev via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Fri Sep 27 12:27:53 PDT 2019


ABataev added a comment.

In D68100#1686395 <https://reviews.llvm.org/D68100#1686395>, @lildmh wrote:

> In D68100#1686337 <https://reviews.llvm.org/D68100#1686337>, @grokos wrote:
>
> > In D68100#1686044 <https://reviews.llvm.org/D68100#1686044>, @ABataev wrote:
> >
> > > In D68100#1685975 <https://reviews.llvm.org/D68100#1685975>, @lildmh wrote:
> > >
> > > > Please review when you have time, thanks
> > >
> > >
> > > Maybe, it is better to add a single function to pass the list of mappers to the runtime and use the old functions rather than just duplicate them? Something like `__tgt_mappers(...)` and store them in the runtime. Plus, modify the original functions to use mappers if they were provided. Thoughts?
> >
> >
> > I'm in favor of this solution, as it is less intrusive than the one posted in this patch. I think a revised patch will be quite smaller and the changes it introduces will be clearer.
>
>
> I think Alexey and George's concern is that this patch introduces many new runtime apis. My arguments are:
>
> 1. This patch will deprecate the old interfaces as we discussed before in the meeting. E.g., `__tgt_target_teams` will no longer be used. They are kept there just for legacy code support. So I didn't actually introduce any new runtime apis.
> 2. If we have something like `__tgt_mapper` instead, and integrate all mapper functionalities in it, we will need to pass extra arguments to it to distinguish whether it is for `__tgt_targt`, `__tgt_target_data_begin`, etc. On the other hand, the current runtime has an interface for each case. For instance, we have `__tgt_targt`, `__tgt_target_data_begin`,  `__tgt_target_data_update_nowait`, etc., instead of a single `__tgt_target` function which does it all. Therefore, I think the current design fits the overall design of OpenMP runtime better: have a function for each case.


Why we will need this extra argument? It just provides a list of mapper functions and stores them in the runtime before we call any `__tgt_...` function. Each particular `__tgt_...` runtime function will know what do with all these mappers if they were stored.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68100/new/

https://reviews.llvm.org/D68100





More information about the Openmp-commits mailing list