sfantao added inline comments.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3044-3054
@@ +3043,13 @@
+
+ if (auto *VAT = dyn_cast<VariableArrayType>(ElementType.getTypePtr())) {
+ auto VATInfo = CGF.getVLASize(VAT);
+ Size = llvm::ConstantInt::get(
+ CGM.SizeTy,
+ CGM.getContext().getTypeSizeInChars(VATInfo.second).getQuantity());
+ Size = CGF.Builder.CreateNUWMul(Size, VATInfo.first);
+ } else {
+ uint64_t ElementTypeSize =
+ CGM.getContext().getTypeSizeInChars(ElementType).getQuantity();
+ Size = llvm::ConstantInt::get(CGM.SizeTy, ElementTypeSize);
+ }
+
----------------
ABataev wrote:
> Use `getTypeSize(CGF, ElementType)` instead
In some previous review of this patch there was a suggestion to change from
`getTypeSize(CGF, ElementType)/8` to
`getTypeSizeInChars(ElementType).getQuantity()`. Just double checking if you
really want me to revert that.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3082-3128
@@ +3081,49 @@
+
+ // Generate the code to launch the target region. The pattern is the
+ // following:
+ //
+ // ...
+ // br IfCond (if any), omp_offload, omp_offload_fail
+ //
+ // omp_offload.try:
+ // ; create arrays for offloading
+ // error = __tgt_target(...)
+ // br error, omp_offload_fail, omp_offload_end
+ //
+ // omp_offload.fail:
+ // host_version(...)
+ //
+ // omp_offload.end:
+ // ...
+ //
+
+ auto OffloadTryBlock = CGF.createBasicBlock("omp_offload.try");
+ auto OffloadFailBlock = CGF.createBasicBlock("omp_offload.fail");
+ auto ContBlock = CGF.createBasicBlock("omp_offload.end");
+
+ if (IfCond)
+ CGF.EmitBranchOnBoolExpr(IfCond, OffloadTryBlock, OffloadFailBlock,
+ /*TrueCount=*/0);
+
+ CGF.EmitBlock(OffloadTryBlock);
+
+ unsigned PointerNumVal = BasePointers.size();
+ llvm::Value *PointerNum = CGF.Builder.getInt32(PointerNumVal);
+ llvm::Value *BasePointersArray;
+ llvm::Value *PointersArray;
+ llvm::Value *SizesArray;
+ llvm::Value *MapTypesArray;
+
+ if (PointerNumVal) {
+ llvm::APInt PointerNumAP(32, PointerNumVal, /*isSigned=*/true);
+ QualType PointerArrayType = CGF.getContext().getConstantArrayType(
+ CGF.getContext().VoidPtrTy, PointerNumAP, ArrayType::Normal,
+ /*IndexTypeQuals=*/0);
+
+ BasePointersArray =
+ CGF.CreateMemTemp(PointerArrayType, ".offload_baseptrs").getPointer();
+ PointersArray =
+ CGF.CreateMemTemp(PointerArrayType, ".offload_ptrs").getPointer();
+
+ // If we don't have any VLA types, we can use a constant array for the map
+ // sizes, otherwise we need to fill up the arrays as we do for the
pointers.
----------------
ABataev wrote:
> Could you use `emitOMPIfClause()` function instead of this long block?
I don't think I can reuse the current implementation. The reason is that the
"else" basic block is always emitted and the "if" basic block needs to have a
branch to the "else" basic block, and all these basic blocks are only visible
inside `emitOMPIfClause()`. Basically, the if clause codegen would have to be
combined with the testing of the return code of the offloading call.
================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:38
@@ +37,3 @@
+ if (UseOnlyReferences) {
+ LValue LV = MakeNaturalAlignAddrLValue(
+ CreateMemTemp(CurField->getType(), "__vla_size_ref").getPointer(),
----------------
ABataev wrote:
> Use `MakeAddrLValue(CreateMemTemp(CurField->getType(), "__vla_size_ref"),
> CurField->getType())` instead.
Done!
================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:117-122
@@ -102,4 +116,8 @@
if (FD->hasCapturedVLAType()) {
auto *ExprArg =
EmitLoadOfLValue(ArgLVal, SourceLocation()).getScalarVal();
+ if (UseOnlyReferences) {
+ auto ExprArgRef = MakeNaturalAlignAddrLValue(ExprArg, FD->getType());
+ ExprArg = EmitLoadOfLValue(ExprArgRef,
SourceLocation()).getScalarVal();
+ }
auto VAT = FD->getCapturedVLAType();
----------------
ABataev wrote:
> ```
> if (UseOnlyReferences)
> ArgLVal = CGF.EmitLoadOfReferenceLValue(ArgLVal.getAddress(),
> FD->getType()->casAs<ReferenceType>());
> auto *ExprArg = EmitLoadOfLValue(ArgLVal, SourceLocation()).getScalarVal();
> ```
Done!
http://reviews.llvm.org/D12871
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
