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

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 4 10:38:02 PST 2019


ABataev added a comment.

Some comments marked as ]Вщту] are not done actually, check them, please.



================
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)) {
----------------
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.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8342
                "Not expecting declaration with no component lists.");
-        DeclComponentLists.emplace_back(L.second, C->getMapType(),
+        DeclComponentLists.emplace_back(L.second, C->getMapType(),  // getMapType been hardcoded as tofrom
                                         C->getMapTypeModifiers(),
----------------
It should not be hardcoded, you need to teach sema about other possible values for map type.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:149
     SourceLocation DefaultAttrLoc;
-    DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-    SourceLocation DefaultMapAttrLoc;
+    DefaultmapInfo DefaultmapMap[3];
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204





More information about the cfe-commits mailing list