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

[Lingda Li via Phabricator via Openmp-commits]

lildmh added a comment.

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.


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

https://reviews.llvm.org/D60972