ABataev 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) {
----------------
cchen wrote:
> 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)
It does not relate to the comments thread but I got it. Anyway, try to 
investigate why the compiler crashes if you try to cr4eate a constant ща 
]СПАюШте64Ен] directly.


================
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:
> > 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.
It is going to simplify the codegen. Currently, to get the offset, you need to 
dig through all the elements of the array section. If, instead, you use the 
pointers, you would not need to do this and you can rely on something like 
`CGF.EmitArraySectionLValue()`. At least, I hope so.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7910
+        const Expr *CountExpr = nullptr;
+        if (OASE)
+          CountExpr = OASE->getLength();
----------------
The check is not required, you already checked that the expression must be 
array section only.


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

Reply via email to