[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7849-7851
+            if (&Component != &*Components.begin()) {
+              ElementType = ElementType->getPointeeOrArrayElementType();
+            }
----------------
No need for braces here


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8956
+        Info.PointersArray, 0, I);
+    Address PAddr(P, C.getTypeAlignInChars(C.VoidPtrTy));
+    CGF.Builder.CreateStore(DAddr.getPointer(), PAddr);
----------------
`C.getTypeAlignInChars(C.VoidPtrTy)`->`CGF.getPointerAlign()`


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10330-10333
+    // Fill up non-contiguous information.
+    Info.Offsets = std::move((Offsets));
+    Info.Counts = std::move((Counts));
+    Info.Strides = std::move((Strides));
----------------
Better just to pass `Info.Offsets`, `Info.Counts` and `Info.Strides` as arguments to `generateAllInfo()` function and do not create local copies at all.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10583-10586
+    // Fill up non-contiguous information.
+    Info.Offsets = std::move((Offsets));
+    Info.Counts = std::move((Counts));
+    Info.Strides = std::move((Strides));
----------------
Same, pass the fields as arguments instead.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12515
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false));
----------------
Still calling an extra constructor here, just `.emplace_back(AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false);`


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12633-12634
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExprPr, AssociatedDecl, IsNonContiguous));
   }
----------------
Just `Components.emplace_back(AssociatedExprPr, AssociatedDecl, IsNonContiguous);`


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12684
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExprPr, AssociatedDecl, IsNonContiguous));
----------------
`.emplace_back(AssociatedExprPr, AssociatedDecl, IsNonContiguous);`


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12734
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false));
----------------
.`emplace_back(AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false);`


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12776-12777
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExpr, AssociatedDecl, /*IsNonContiguous*/ false));
   }
----------------
`.emplace_back(AssociatedExpr, AssociatedDecl, /*IsNonContiguous*/ false);`


================
Comment at: clang/lib/Serialization/ASTReader.cpp:12819-12820
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
-    Components.push_back(OMPClauseMappableExprCommon::MappableComponent(
-        AssociatedExpr, AssociatedDecl));
+    Components.emplace_back(OMPClauseMappableExprCommon::MappableComponent(
+        AssociatedExpr, AssociatedDecl, /*IsNonContiguous=*/false));
   }
----------------
`.emplace_back(AssociatedExpr, AssociatedDecl, /*IsNonContiguous=*/false));`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972