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

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In D60972#1607088 <https://reviews.llvm.org/D60972#1607088>, @lildmh wrote:

> In D60972#1606990 <https://reviews.llvm.org/D60972#1606990>, @grokos wrote:
>
> > In D60972#1606804 <https://reviews.llvm.org/D60972#1606804>, @lildmh wrote:
> >
> > > Since the mapper is not really implemented in this patch, if I add a test, it will be something like below:
> > >
> > >   __tgt_push_mapper_component(h, base0, begin0, size0, type0);
> > >   __tgt_push_mapper_component(h, base1, begin1, size1, type1);
> > >   auto total_size = __tgt_mapper_num_components(h);
> > >   printf("size=%d", total_size);
> > >   // CHECK: size=2
> > >
> > >
> > > It seems to me this test is not meaningful. I can add a more meaningful test after all mapper patches are upstreamed.
> > >  Do you think we need a meaningless test like this now?
> >
> >
> > Yes, it's good to have a test, even a very elementary one. When full support for `declare mapper` is upstreamed we can revisit the test and extend it to check real-use scenarios.
>
>
> I just realized that these functions (`__tgt_push_mapper_component` and `__tgt_mapper_num_components`) are not exposed to users (i.e., defined in omp.h), so the test proposed above is not possible.
>
> I cannot imagine what test it can have for this patch now, so I think we can leave this patch testless. If you have an idea of test, please let me know.


You need to declare them yourself in the test. That's not really elegant, but many other runtime tests already do.


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

https://reviews.llvm.org/D60972