[Openmp-commits] [PATCH] D60972: [OpenMP 5.0] libomptarget interface for declare mapper functions

Mon Jun 10 08:19:59 PDT 2019

lildmh added a comment.

In D60972#1536070 <https://reviews.llvm.org/D60972#1536070>, @Hahnfeld wrote:

> In D60972#1533013 <https://reviews.llvm.org/D60972#1533013>, @lildmh wrote:
>
> > @Hahnfeld Do you really think it is necessary to pass these two functions as arguments, instead of exporting them. If you do, could you explain why?
>
>
> I don't say it's necessary, but rather an option. Asked differently, why do we need to export them (and decide on externally visible names) if they're only used in a function where we can pass them as arguments?
>
> At the moment, this patch doesn't really add any value (meaning changed behavior) to the runtime. It's needed for D59474 <https://reviews.llvm.org/D59474> which is already large enough on its own, but it doesn't get to any state where it can be used.

I don't have a strong reason to export it either. One reason is it avoids one indirect memory access so could be potentially better for performance (the difference should be very minor).

Yea I agree this patch doesn't really do anything itself, it just provide a runtime interface for  D59474 <https://reviews.llvm.org/D59474>. Please let me know if there is anything that prevents this patch from being accepted.

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

https://reviews.llvm.org/D60972