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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 3 inline comments as done.
cchen added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+            llvm::APInt Size = CAT->getSize();
+            SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+          } else if (VAT) {
----------------
ABataev wrote:
> 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?
My comment was not accurate, I've updated it. What I want to convey is that we can only have `CAT, VAT, or pointer` here, since analysis in Sema has a restriction for it. (SemaOpenMP line 16623)


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+          } else {
+            Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+                                               CGF.Int64Ty,
+                                               /*isSigned=*/false);
----------------
ABataev wrote:
> 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?
Yes, I'm fine either passing index or passing address, though I'm curious why you're recommending passing address.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7875-7878
+        const auto *OASE = dyn_cast<OMPArraySectionExpr>(AssocExpr);
+
+        if (!OASE)
+          continue;
----------------
ABataev wrote:
> 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;
> ...
> ```
Not sure about this one, I've added:
```
if (!OASE)
  continue;
...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972