[PATCH] D12871: [OpenMP] Target directive host codegen - rebased

[Alexey Bataev via cfe-commits]

ABataev 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);
+      }
+
----------------
You were using uint64_t ASTContext::getTypeSize(ElementType), but I'm talking about static llvm::Value *getTypeSize(CGF, ElementType), which is defined in CGOpenMPRuntime.cpp. It does exactly the same thing you're doing in this part of code.

================
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.
----------------
You can emit an empty else block, it will be optimized by backend

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:121-123
@@ -103,4 +120,5 @@
+            ArgLVal.getAddress(), ArgLVal.getType()->castAs<ReferenceType>());
       auto *ExprArg =
           EmitLoadOfLValue(ArgLVal, SourceLocation()).getScalarVal();
       auto VAT = FD->getCapturedVLAType();
       VLASizeMap[VAT->getSizeExpr()] = ExprArg;
----------------
sfantao wrote:
> I am actually doing `ArgLVal.getType()` instead  `FD->getType()`. FD doesn't have a reference type here.
Agree, thanks.


http://reviews.llvm.org/D12871