[Openmp-commits] [PATCH] D86781: [LIBOMPTARGET]Do not try to optimize bases for the next parameters.

Sat Aug 29 18:13:57 PDT 2020

ABataev added inline comments.

================
Comment at: openmp/libomptarget/src/omptarget.cpp:1068
+      const bool IsFirstPrivate =
+          (I >= ArgNum - 1 || !(ArgTypes[I + 1] & OMP_TGT_MAPTYPE_MEMBER_OF));
       Ret = PrivateArgumentManager.addArg(HstPtrBegin, ArgSizes[I],
----------------
tianshilei1992 wrote:
> I have a quick question: Does front end order all arguments? For example, let's say a user write the following code:
> ```
> #pragma omp target first private(a.p1[0:N], b, a.p2[0:N], c, a.p3[0:N])
> ```
> where `p1`, `p2` and `p3` are members of a struct `a`. Then when passing to the runtime library, `Args[I]`, `Args[I+1]` and `Args[I+2]` should be corresponding to `a.p1[0:N]`, `a.p2[0:N]`, `a.p3[0:N]`, right? They'll not be interleaved by `b` and `c`.
The order is important for correct passing of captured variables to the target regions (order of the arguments for the outlined target functions). For target regions it is not always defined by the order in the mappping/data-sharing clauses, but the order of captured variables, which can be different.
Yes, frontend already reorders maps to gather elements with the same bases. It is required for correct memory allocation of the partially copied structures.

================
Comment at: openmp/libomptarget/test/mapping/lambda_mapping.cpp:10
+template <typename LOOP_BODY>
+inline void forall(int Begin, int End, LOOP_BODY LoopBody) {
+#pragma omp target parallel for schedule(static)
----------------
tianshilei1992 wrote:
> Really interesting fix. I didn't realize that lambda function is captured as a first-private argument.
In future we may try to implement the deepcopy of the record with data members having reference data types in the similar way.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86781