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

[George Rokos via Phabricator via Openmp-commits]

grokos added a comment.

I see that `target_data_mapper` pretty much copies the entire code from `target_data_begin` and `target_data_end`. That's 200+ duplicate lines of code. Couldn't we just add this check:

  // If a valid mapper is attached, call the mapper function to complete the
  // mapping of its members.
  if (arg_mapper_ptrs && arg_mapper_ptrs[i]) {
    int (*mapper_func_ptr)(int64_t, void *, void *, int64_t, int64_t);
    mapper_func_ptr = (int (*)(int64_t, void *, void *, int64_t, int64_t))(
        arg_mapper_ptrs[i]);
    int rt = (*mapper_func_ptr)(Device.DeviceID, HstPtrBase, HstPtrBegin,
                                data_size, arg_types[i]);
    if (rt != OFFLOAD_SUCCESS) {
      DP("User-defined mapper function failed.\n");
      return OFFLOAD_FAIL;
    }
  }

to the existing functions? All that is needed is change the function signature of `target_data_begin` and `target_data_end` to include the extra `arg_mapper_ptrs` argument and set a default value of NULL so that no other changes are needed throughout the code:

`target_data_begin(DeviceTy &Device, int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types, void **arg_mapper_ptrs = NULL);`
`target_data_end(DeviceTy &Device, int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types, void **arg_mapper_ptrs = NULL);`

After all, I don't see any point in having a function which tries to execute the "begin" code and the "end" code one after the other.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D60972