ABataev added inline comments.
================
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);
----------------
cchen wrote:
> ABataev wrote:
> > The code for `SizeV` must be under the control of the next `if`:
> > ```
> > if (DimSizes.size() < Components.size() - 1) {
> > ....
> > }
> > ```
> I don't think I understand this one. Why do you remove SizeV in the if
> condition?
This is for `SizeV`. You don't use it if `DimSizes.size() < Components.size() -
1` is `false`, looks like memory leak.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+ llvm::APInt Size = CAT->getSize();
+ SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+ } else if (VAT) {
----------------
cchen wrote:
> ABataev wrote:
> > Create directly as of `CGF.Int64Ty` type.
> Doing this I'll get assertion error in this exact line if on a 32-bits target.
Hmm, why, can you investigate?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+ } else {
+ Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+ CGF.Int64Ty,
+ /*isSigned=*/false);
----------------
cchen wrote:
> ABataev wrote:
> > Do you really to pass real offsets here? Can we use pointers instead?
> Do you mean I should set the type of Offset to Expr*?
Currently, you're passing offsets to the runtime. Can we pass pointers instead?
I mean, for `a[b]` you pass `b` to the runtime, can we pass `&a[b]` instead?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7334
+ OverlappedElements = llvm::None,
+ MapDimArrayTy *const Dims = nullptr) const {
// The following summarizes what has to be generated for each map and the
----------------
Can we encapsulate `Dims` into `StructNonContiguousInfo`?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7837-7838
+ Context.getTypeSizeInChars(ElementType).getQuantity();
+ } else if (VAT) {
+ ElementType = VAT->getElementType().getTypePtr();
+ ElementTypeSize =
----------------
If only `CAT` or `VAT` is allowed, then transform this if into:
```
else {
assert(VAT&& ...);
```
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7875-7878
+ const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
+
+ if (!OASE)
+ continue;
----------------
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:7917
+ if (DI != DimSizes.end()) {
+ CurStride = CGF.Builder.CreateNUWMul(CurStrides.back(), *DI++);
+ CurStrides.push_back(CurStride);
----------------
Avoid expressions with some side effects, like `*DI++`
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
