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