[PATCH] D127037: [OpenMP][IRBuilder] 'omp target enter data' support.

Mon Jun 6 08:32:35 PDT 2022

kiranchandramohan added a comment.

In D127037#3560272 <https://reviews.llvm.org/D127037#3560272>, @raghavendhra wrote:

> In D127037#3558292 <https://reviews.llvm.org/D127037#3558292>, @kiranchandramohan wrote:
>
>> The OpenACC translation for `enter data` was implemented in the patch https://reviews.llvm.org/D101504. It was subsequently refactored and now is available as https://github.com/llvm/llvm-project/blob/369ce54bb302f209239b8ebc77ad824add9df089/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp#L510. From what I understand the implementation is more translation heavy and only uses the OpenMPIRBuilder (renamed as OpenACCIRBuilder) for creating the runtime calls. Would a good first step be to move more of the translation code (if possible) to the OpenMPIRBuilder and reuse that rather than starting to implement from scratch?
>>
>> Also, I see that `enter data` should at least have one map clause and the OpenACC implementation and Clang (from a quick look) seems to use the `tgt_target_data_begin_mapper` function. I do not see that runtime function used in this patch.
>
> From OpenMP spec map clause is not mandatory that is why I started without mapper function. Also, OMPRTL___tgt_target_data_begin was calling tgt_target_data_begin_mapper in it's definition. So I started with using OMPRTL___tgt_target_data_begin.

The restrictions of `target enter data` in https://www.openmp.org/spec-html/5.0/openmpsu58.html has the following entry,

- At least one map clause must appear on the directive.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127037