ABataev added a comment.
Also, what about compatibility with declare mapper? Can you add tests for it?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7809-7813
+ MapValuesArrayTy CurOffsets{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 0)};
+ MapValuesArrayTy CurCounts{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};
+ MapValuesArrayTy CurStrides;
+ SmallVector<llvm::Value *, 4> DimSizes{
+ llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};
----------------
DO not use braced initializer list
https://llvm.org/docs/CodingStandards.html#id26
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7839-7847
+ if (CAT) {
+ ElementType = CAT->getElementType().getTypePtr();
+ } else if (VAT) {
+ ElementType = VAT->getElementType().getTypePtr();
+ } else {
+ assert(&Component == &*Components.begin() &&
+ "Only expect pointer (non CAT or VAT) when this is the "
----------------
No need for braces here per coding standard
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7876
+
+ // Skip the dummy dimension since we have already have its information.
+ auto DI = DimSizes.begin() + 1;
----------------
have already have
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7892
+ const Expr *AssocExpr = Component.getAssociatedExpression();
+ AssocExpr->dump();
+ const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
----------------
debug code
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7893
+ AssocExpr->dump();
+ const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
+ const auto *AE = dyn_cast<ArraySubscriptExpr>(AssocExpr);
----------------
Move this after the first `if` statement
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7894-7896
+ const auto *AE = dyn_cast<ArraySubscriptExpr>(AssocExpr);
+
+ if (AE) {
----------------
Better to make it something like:
```
if (const auto *AE = dyn_cast<ArraySubscriptExpr>(AssocExpr))
```
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7944-7945
+ : llvm::ConstantInt::get(CGF.Int64Ty, /*V=*/1);
+ Count = CGF.Builder.CreateUDiv(CGF.Builder.CreateNUWSub(*DI, Offset),
+ Stride);
+ }
----------------
If no stride at all, why need to divide?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7967
+ DimProd = CGF.Builder.CreateNUWMul(DimProd, *(DI - 1));
+ CurStrides.push_back(CGF.Builder.CreateNUWMul(DimProd, Stride));
+ if (DI != DimSizes.end())
----------------
Same, if no stride at all, no need to mul.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16913-16919
+ if (OASE) {
+ if (!OASE->getLength())
+ SemaRef.Diag(ELoc, diag::err_array_section_does_not_specify_length)
+ << ERange;
+ else
+ break;
+ }
----------------
Better to transform it to something like:
```
if (!OASE || OASE->getLength())
break;
SemaRef.Diag(...)
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84192/new/
https://reviews.llvm.org/D84192
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
