ABataev added a comment. Do you have a test for mapping of something like `arr[0][:n]`, where the base is an array subscript and the remaining part is an array section?
================ Comment at: clang/include/clang/AST/OpenMPClause.h:4680-4681 + : AssociatedExpressionNonContiguousPr( + llvm::PointerIntPair<Expr *, 1, bool>(AssociatedExpression, + IsNonContiguous)), AssociatedDeclaration( ---------------- I think you can initialize `AssociatedExpressionNonContiguousPr` using just `AssociatedExpressionNonContiguousPr(AssociatedExpression, IsNonContiguous)` form, no? ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7890 OpenMPMapClauseKind MapType, - ArrayRef<OpenMPMapModifierKind> MapModifiers, - bool ReturnDevicePointer, bool IsImplicit) + ArrayRef<OpenMPMapModifierKind> MapModifiers, bool ReturnDevicePointer, + bool IsImplicit) ---------------- Restore original formatting ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8375-8376 bool IsFinalArraySection = - isFinalArraySectionExpression(I->getAssociatedExpression()); + isFinalArraySectionExpression(I->getAssociatedExpression()) && + (!IsNonContiguous); ---------------- Better to convert it to `!IsNonContiguous && isFinalArraySectionExpression(I->getAssociatedExpression())`. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8396 + if (OASE) + DimSize++; ---------------- Use prefix form `++DimSize`. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8453 + /*AddIsTargetParamFlag=*/false, + /*IsNonContiguous=*/IsNonContiguous); LB = BP; ---------------- No need for parameter name comment here, it is required only if the `true|false` constants are used ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8508 + IsCaptureFirstInfo && !RequiresReference, + /*IsNonContiguous=*/IsNonContiguous); ---------------- Same, comment not required ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8563-8577 + /// Generate the base pointers, section pointers, sizes , map type bits, + /// dimension size, offset, count, and strides for the provided map type, map + /// modifier, and expression components. \a IsFirstComponent should be set to + /// true if the provided set of components is the first associated with a + /// capture. + void generateInfoForTargetDataComponentList( + OpenMPMapClauseKind MapType, ArrayRef<OpenMPMapModifierKind> MapModifiers, ---------------- Can we merge the functionality in this new function with the existing ones somehow? It is not the best idea to duplicate functionality using copy-paste if any. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9353-9355 -/// Emit the arrays used to pass the captures and map information to the -/// offloading runtime library. If there is no map or capture information, -/// return nullptr by reference. ---------------- Why removed the comment? ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9681 +/// return nullptr by reference. +static void emitTargetDataOffloadingArrays( + CodeGenFunction &CGF, ---------------- Same question as before - can we merge this functionality with the existing functions? ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:18383-18385 MVLI.VarComponents.back().push_back( - OMPClauseMappableExprCommon::MappableComponent(SimpleRefExpr, D)); + OMPClauseMappableExprCommon::MappableComponent(SimpleRefExpr, D, + false)); ---------------- Use `.emplace_back(SimpleRefExpr, D, false);` ================ Comment at: clang/lib/Serialization/ASTReader.cpp:12481-12482 auto *AssociatedDecl = Record.readDeclAs<ValueDecl>(); Components.push_back(OMPClauseMappableExprCommon::MappableComponent( - AssociatedExpr, AssociatedDecl)); + AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false)); } ---------------- `.emplace_back(AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false);` ================ Comment at: clang/lib/Serialization/ASTReader.cpp:12599-12600 auto *AssociatedDecl = Record.readDeclAs<ValueDecl>(); Components.push_back(OMPClauseMappableExprCommon::MappableComponent( - AssociatedExpr, AssociatedDecl)); + AssociatedExprPr, AssociatedDecl, IsNonContiguous)); } ---------------- Same, use `emplace_back()` ================ Comment at: clang/lib/Serialization/ASTReader.cpp:12650 auto *AssociatedDecl = Record.readDeclAs<ValueDecl>(); Components.push_back(OMPClauseMappableExprCommon::MappableComponent( + AssociatedExprPr, AssociatedDecl, IsNonContiguous)); ---------------- Same, use `emplace_back()` ================ Comment at: clang/lib/Serialization/ASTReader.cpp:12700 auto *AssociatedDecl = Record.readDeclAs<ValueDecl>(); Components.push_back(OMPClauseMappableExprCommon::MappableComponent( + AssociatedExprPr, AssociatedDecl, /*IsNonContiguous=*/false)); ---------------- Same, use `emplace_back()` ================ Comment at: clang/lib/Serialization/ASTReader.cpp:12744 Components.push_back(OMPClauseMappableExprCommon::MappableComponent( - AssociatedExpr, AssociatedDecl)); + AssociatedExpr, AssociatedDecl, /*IsNonContiguous*/ false)); } ---------------- Same, use `emplace_back()` ================ Comment at: clang/test/OpenMP/target_update_to_messages.cpp:147 } + return 0; ---------------- Delete this extra line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79972/new/ https://reviews.llvm.org/D79972 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits