[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#1605250 <https://reviews.llvm.org/D60972#1605250>, @grokos wrote:

> In D60972#1605210 <https://reviews.llvm.org/D60972#1605210>, @lildmh wrote:
>
> > In D60972#1605188 <https://reviews.llvm.org/D60972#1605188>, @grokos wrote:
> >
> > > Now that the clang-side patch is finalized, this is almost good to go. What is missing from this patch is a test. Can you add something under `libomptarget/test/offloading/`?
> >
> >
> > Sure, since this is very simple I'll add a simple test. Do you think the test should be in libomptarget/test/offloading/ or libomptarget/test/api/?
>
>
> Sorry, I meant `libomptarget/test/mapping/`!
>
> The `api` directory is meant for test cases calling `omp_*` API functions, which is not the case here.


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?


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

https://reviews.llvm.org/D60972