[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7246-7248
+    // Set implicit behavior except for "default" for defaultmap
+    if ((Bits & OMP_MAP_IMPLICIT) &&
+        (ImplicitBehavior != OMPC_DEFAULTMAP_MODIFIER_default)) {
----------------
cchen wrote:
> ABataev wrote:
> > Hmm, this is strange, Do we really need this kind of processing here? The variables must be mapped implicitly in Sema and, thus, all this processing of the default mapping rules should not be required.
> I'm now having design question about setting the correct implicit map type in Sema for the below situation:
> ```
> int *ptr_1, *ptr_2, arr[50];
> #pragma omp target defaultmap(alloc:pointer) defaultmap(from:aggregate)
> {
>   ptr_1++, ptr_2++;
>   arr[0]++;
> }
> ```
> In this case we need to store two maptypes - alloc and from for an `ActOnOpenMPMapClause` but `ActOnOpenMPMapClause` only pass one maptype so I'm wondering should I modify the interface of OMPMapClause which pass an array of maptypes rather than one maptype variable?
Just create 3 arrays instead of single array for mapped items and call `ActOnOpenMPMapClause` for each of them


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:149
     SourceLocation DefaultAttrLoc;
-    DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-    SourceLocation DefaultMapAttrLoc;
+    DefaultmapInfo DefaultmapMap[3];
+
----------------
cchen wrote:
> ABataev wrote:
> > ABataev wrote:
> > > Maybe, it would be better to make `DMVC_unspecified` the last one in the `DefaultMapVariableCategory` and use it as an array dimension here rather than rely on the magical number?
> > Not done.
> Not sure about this one. I've already put DMVC_unspecified to the last one in the DefaultMapVariableCategory enum so that I now don't need magic number. Or you are pointing something else? Thanks
Use `DefaultmapInfo DefaultmapMap[DMVC_unspecified]` instead of `DefaultmapInfo DefaultmapMap[3]`, this is what I meant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204