ABataev added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7327-7330
+ bool IsNonContiguous = false,
+ MapNonContiguousArrayTy *const Offsets = nullptr,
+ MapNonContiguousArrayTy *const Counts = nullptr,
+ MapNonContiguousArrayTy *const Strides = nullptr) const {
----------------
I would prefer to pack these 4 params into a single parameter (a struct). Also,
can we put `Dims` parameter into the list of the optional parameters?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7805
+ // should be [10, 10] and the first stride is 4 btyes.
+ for (const auto &Component : Components) {
+ const Expr *AssocExpr = Component.getAssociatedExpression();
----------------
Expand `auto` here to a real type
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7807
+ const Expr *AssocExpr = Component.getAssociatedExpression();
+ const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
+ if (OASE) {
----------------
Can we have anything else except for array section here? If not, use just
`cast`. If yes, use `continue` to simplify complexity:
```
if (!OASE)
continue;
...
```
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7821
+ Context.getTypeSizeInChars(ElementType).getQuantity();
+ } else if (VAT) {
+ ElementType = VAT->getElementType().getTypePtr();
----------------
What if the base is a pointer, not an array?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7831-7838
+ llvm::Value *SizeV = nullptr;
+ if (CAT) {
+ llvm::APInt Size = CAT->getSize();
+ SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+ } else if (VAT) {
+ const Expr *Size = VAT->getSizeExpr();
+ SizeV = CGF.EmitScalarExpr(Size);
----------------
The code for `SizeV` must be under the control of the next `if`:
```
if (DimSizes.size() < Components.size() - 1) {
....
}
```
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+ llvm::APInt Size = CAT->getSize();
+ SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+ } else if (VAT) {
----------------
Create directly as of `CGF.Int64Ty` type.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7859
+ // declaration in target update to/from clause.
+ for (const auto &Component : Components) {
+ const Expr *AssocExpr = Component.getAssociatedExpression();
----------------
Expand `auto` here to a real type
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7861-7864
+ const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
+
+ if (OASE) {
+ // Offset
----------------
Can we have anything else except for array section here? If not, use just
`cast`. If yes, use `continue` to simplify complexity:
```
if (!OASE)
continue;
...
```
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+ } else {
+ Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+ CGF.Int64Ty,
+ /*isSigned=*/false);
----------------
Do you really to pass real offsets here? Can we use pointers instead?
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
