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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
