[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

Wed Oct 14 09:18:47 PDT 2020

ABataev added a comment.

In D86119#2330310 <https://reviews.llvm.org/D86119#2330310>, @jdenny wrote:

> Thanks for working on this.  Sorry to take so long to review.  Before I try to digest the code, I have a few high-level questions.
>
> Based on the test suite changes, `TARGET_PARAM` is disappearing from many cases.  Can you explain a bit how that supports overlapping and reordering?

`TARGET_PARAM` is used only for marking the data that should be passed to the kernel function as an argument. We just generate it in many cases but the runtime actually does not use them. Thу patch relies on this thing, otherwise, we may pass an incorrect number of arguments to the kernel functions.

> Have you considered issue 2337 for the OpenMP spec and how your implementation handles the ambiguous cases cited there?

Can you provide the details about this issue?

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7283
+        llvm::find(MotionModifiers, OMPC_MOTION_MODIFIER_present) !=
+            MotionModifiers.end())
       Bits |= OMP_MAP_PRESENT;
----------------
jdenny wrote:
> Unrelated change?
Well, in general, yes, I just tried to simplify the code, can commit it as a separate NFC.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86119