[mlir][openacc] Initial translation for DataOp to LLVM IR

[clementval]

Summary

Add basic translation of acc.data to LLVM IR with runtime calls.

[kiranchandramohan]

Apologies for the delay.

I had a quick look through the patch. I feel like a lot of the code here can be put into the OpenACC/OpenMP IRBuilder. As you know the usual way the function for a construct is written in the OpenMP IRBuilder is like there is a function (createData) for creating the runtime calls for the construct and there is a bodygen callback that can fill in the body of the construct. The createData function in the OpenACC IRBuilder can create the runtime calls for beginMapperFunc, endMapperFunc. Is it possible to rewrite the code like this?

mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp

275–315

Can these two functions be in the OpenACC IRBuilder?

418–424

This should be in the OpenACC IRBuilder I think.

428–464

This code can probably be the bodygen callback, which creates the mapping from mlir to llvm blocks and then converts them to LLVM IR.

467–475

This should be in the OpenACC IRBuilder I think.

[clementval]

In D104301#2837251, @kiranchandramohan wrote:

Apologies for the delay.

I had a quick look through the patch. I feel like a lot of the code here can be put into the OpenACC/OpenMP IRBuilder. As you know the usual way the function for a construct is written in the OpenMP IRBuilder is like there is a function (createData) for creating the runtime calls for the construct and there is a bodygen callback that can fill in the body of the construct. The createData function in the OpenACC IRBuilder can create the runtime calls for beginMapperFunc, endMapperFunc. Is it possible to rewrite the code like this?

Thanks for the comments. At first I was thinking to put this in the OpenXXIRBuilder like other functions you mentioned. The only reason I did not is that even it would be in the IRBuilder, the clang code gen is using the CodeGenFunction to create the allocas through call to CreateMemTemp and this is likely not do-able in our case from MLIR. But if it is fine that the function the IRBuilder is only (partially at least) used from Flang then I 100% fine following your suggestion.

[kiranchandramohan]

In D104301#2837561, @clementval wrote:

In D104301#2837251, @kiranchandramohan wrote:

Apologies for the delay.

I had a quick look through the patch. I feel like a lot of the code here can be put into the OpenACC/OpenMP IRBuilder. As you know the usual way the function for a construct is written in the OpenMP IRBuilder is like there is a function (createData) for creating the runtime calls for the construct and there is a bodygen callback that can fill in the body of the construct. The createData function in the OpenACC IRBuilder can create the runtime calls for beginMapperFunc, endMapperFunc. Is it possible to rewrite the code like this?

Thanks for the comments. At first I was thinking to put this in the OpenXXIRBuilder like other functions you mentioned. The only reason I did not is that even it would be in the IRBuilder, the clang code gen is using the CodeGenFunction to create the allocas through call to CreateMemTemp and this is likely not do-able in our case from MLIR. But if it is fine that the function the IRBuilder is only (partially at least) used from Flang then I 100% fine following your suggestion.

We can discuss during today's call.

[clementval]

Move code to OpenMPIRBuilder

[clementval]

@kiranchandramohan I moved some of the code to the OpenMPIRBuilder. I left the region conversion in the translation for the moment. This region is a bit special since it could even be just inline between the two calls.

[kiranchandramohan]

Thanks @clementval. I would request @jdoerfert, @Meinersbur, @fghanim to provide the feedback here.

[ftynse]

MLIR part LGTM with comments addressed.

mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp

132

We usually pass SmallVectorImpl & that avoids the need to fix the number of stack elements. SmallVector has a fixed number even if this number is computed by template deduction.

327

No need for template here.

333–338

Please expand auto here. MLIR uses auto when the type is obvious from the RHS, e.g., in a dyn_cast, or when it is too long or impossible to spell out, e.g. iterators and lambdas.

431–433

It doesn't look like the insertion point changed between where currIP was created and here.

[clementval]

Address review comments

[clementval]

Addressed @ftynse comments.

[clementval]

Ping

[clementval]

@jdoerfert @Meinersbur Any comment on the OpenMPIRBuilder part?

[jdoerfert]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

39 ↗

(On Diff #355555)

If we have a struct with 3 alloca members with proper names this is not needed anymore.

2264 ↗

(On Diff #355555)

Also elsewhere.

2281 ↗

(On Diff #355555)

The types should all exist, Int8Ptr is a global in the omp namespace. If not, you can also add them to OMPKinds.def (top).

I don't think returning a SmallVector is great. I'd suggest a struct with 3 Alloca members with names or pass such a struct in and fill it.

Alloca IP has to be passed in from the caller. The entry point is not necessarily the right place, e.g., if this is called in a parallel region.

[clementval]

Address review comment

[clementval]

@jdoerfert Thanks for the review. I uploaded a new version.

[jdoerfert]

OpenMPIRBuilder parts looks good

[clementval]

Rebase

[clementval]

Fix OpenMPIRBuilder unit test

mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp

 using namespace mlir;
 
 using OpenACCIRBuilder = llvm::OpenMPIRBuilder;
 
 //===----------------------------------------------------------------------===//
 // Utility functions
 //===----------------------------------------------------------------------===//
 
-/// 0 = alloc/create
-static constexpr uint64_t kCreateFlag = 0;
-/// 1 = to/device/copyin
-static constexpr uint64_t kDeviceCopyinFlag = 1;
-/// 2 = from/copyout
-static constexpr uint64_t kHostCopyoutFlag = 2;
-/// 8 = delete
-static constexpr uint64_t kDeleteFlag = 8;
+/// Flag values are extracted from openmp/libomptarget/include/omptarget.h and
+/// mapped to corresponding OpenACC flags.
+static constexpr uint64_t kCreateFlag = 0x000;
+static constexpr uint64_t kDeviceCopyinFlag = 0x001;
+static constexpr uint64_t kHostCopyoutFlag = 0x002;
+static constexpr uint64_t kCopyFlag = kDeviceCopyinFlag | kHostCopyoutFlag;
+static constexpr uint64_t kPresentFlag = 0x1000;
+static constexpr uint64_t kDeleteFlag = 0x008;
 
 /// Default value for the device id
 static constexpr int64_t kDefaultDevice = -1;
 
+/// Position of the respective AllocInst* in a SmallVector.
+static constexpr int64_t kArgsBasePos = 0;
+static constexpr int64_t kArgsPos = 1;
+static constexpr int64_t kArgSizesPos = 2;
+
 /// Create a constant string location from the MLIR Location information.
 static llvm::Constant *createSourceLocStrFromLocation(Location loc,
                                                       OpenACCIRBuilder &builder,
                                                       StringRef name) {
   if (auto fileLoc = loc.dyn_cast<FileLineColLoc>()) {
     StringRef fileName = fileLoc.getFilename();
     unsigned lineNo = fileLoc.getLine();
     unsigned colNo = fileLoc.getColumn();

 /// Extract pointer, size and mapping information from operands
 /// to populate the future functions arguments.
 static LogicalResult
 processOperands(llvm::IRBuilderBase &builder,
                 LLVM::ModuleTranslation &moduleTranslation, Operation *op,
                 ValueRange operands, unsigned totalNbOperand,
                 uint64_t operandFlag, SmallVector<uint64_t> &flags,
                 SmallVector<llvm::Constant *> &names, unsigned &index,
-                llvm::AllocaInst *argsBase, llvm::AllocaInst *args,
-                llvm::AllocaInst *argSizes) {
+                SmallVector<llvm::AllocaInst *> &mapperAllocas) {
   OpenACCIRBuilder *accBuilder = moduleTranslation.getOpenMPBuilder();
   llvm::LLVMContext &ctx = builder.getContext();
   auto *i8PtrTy = llvm::Type::getInt8PtrTy(ctx);
   auto *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, totalNbOperand);
   auto *i64Ty = llvm::Type::getInt64Ty(ctx);
   auto *arrI64Ty = llvm::ArrayType::get(i64Ty, totalNbOperand);
 
   for (Value data : operands) {
if (DataDescriptor::isValid(data)) {
       return op->emitOpError()
              << "Data operand must be legalized before translation."
              << "Unsupported type: " << data.getType();
     }
 
     // Store base pointer extracted from operand into the i-th position of
     // argBase.
     llvm::Value *ptrBaseGEP = builder.CreateInBoundsGEP(
-        arrI8PtrTy, argsBase, {builder.getInt32(0), builder.getInt32(index)});
+        arrI8PtrTy, mapperAllocas[kArgsBasePos],
+        {builder.getInt32(0), builder.getInt32(index)});
     llvm::Value *ptrBaseCast = builder.CreateBitCast(
         ptrBaseGEP, dataPtrBase->getType()->getPointerTo());
     builder.CreateStore(dataPtrBase, ptrBaseCast);
 
     // Store pointer extracted from operand into the i-th position of args.
     llvm::Value *ptrGEP = builder.CreateInBoundsGEP(
-        arrI8PtrTy, args, {builder.getInt32(0), builder.getInt32(index)});
+        arrI8PtrTy, mapperAllocas[kArgsPos],
+        {builder.getInt32(0), builder.getInt32(index)});
     llvm::Value *ptrCast =
         builder.CreateBitCast(ptrGEP, dataPtr->getType()->getPointerTo());
     builder.CreateStore(dataPtr, ptrCast);
 
     // Store size extracted from operand into the i-th position of argSizes.
     llvm::Value *sizeGEP = builder.CreateInBoundsGEP(
-        arrI64Ty, argSizes, {builder.getInt32(0), builder.getInt32(index)});
+        arrI64Ty, mapperAllocas[kArgSizesPos],
+        {builder.getInt32(0), builder.getInt32(index)});
     builder.CreateStore(dataSize, sizeGEP);
 
     flags.push_back(operandFlag);
     llvm::Constant *mapName =
         createMappingInformation(data.getLoc(), *accBuilder);
     names.push_back(mapName);
     ++index;
   }
   return success();
 }
 
 /// Process data operands from acc::EnterDataOp
-static LogicalResult processDataOperands(
-    llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation,
+static LogicalResult
+processDataOperands(llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
                     acc::EnterDataOp op, SmallVector<uint64_t> &flags,
-    SmallVector<llvm::Constant *> &names, llvm::AllocaInst *argsBase,
-    llvm::AllocaInst *args, llvm::AllocaInst *argSizes) {
+                    SmallVector<llvm::Constant *> &names,
+                    SmallVector<llvm::AllocaInst *> &mapperAllocas) {
   // TODO add `create_zero` and `attach` operands
 
   unsigned index = 0;
 
   // Create operands are handled as `alloc` call.
   if (failed(processOperands(builder, moduleTranslation, op,
                              op.createOperands(), op.getNumDataOperands(),
-                             kCreateFlag, flags, names, index, argsBase, args,
-                             argSizes)))
+                             kCreateFlag, flags, names, index, mapperAllocas)))
     return failure();
 
   // Copyin operands are handled as `to` call.
   if (failed(processOperands(builder, moduleTranslation, op,
                              op.copyinOperands(), op.getNumDataOperands(),
-                             kDeviceCopyinFlag, flags, names, index, argsBase,
-                             args, argSizes)))
+                             kDeviceCopyinFlag, flags, names, index,
+                             mapperAllocas)))
     return failure();
 
   return success();
 }
 
 /// Process data operands from acc::ExitDataOp
-static LogicalResult processDataOperands(
-    llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation,
+static LogicalResult
+processDataOperands(llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
                     acc::ExitDataOp op, SmallVector<uint64_t> &flags,
-    SmallVector<llvm::Constant *> &names, llvm::AllocaInst *argsBase,
-    llvm::AllocaInst *args, llvm::AllocaInst *argSizes) {
+                    SmallVector<llvm::Constant *> &names,
+                    SmallVector<llvm::AllocaInst *> &mapperAllocas) {
   // TODO add `detach` operands
 
   unsigned index = 0;
 
   // Delete operands are handled as `delete` call.
   if (failed(processOperands(builder, moduleTranslation, op,
                              op.deleteOperands(), op.getNumDataOperands(),
-                             kDeleteFlag, flags, names, index, argsBase, args,
-                             argSizes)))
+                             kDeleteFlag, flags, names, index, mapperAllocas)))
     return failure();
 
   // Copyout operands are handled as `from` call.
   if (failed(processOperands(builder, moduleTranslation, op,
                              op.copyoutOperands(), op.getNumDataOperands(),
-                             kHostCopyoutFlag, flags, names, index, argsBase,
-                             args, argSizes)))
+                             kHostCopyoutFlag, flags, names, index,
+                             mapperAllocas)))
     return failure();
 
   return success();
 }
 
 /// Process data operands from acc::UpdateOp
-static LogicalResult processDataOperands(
-    llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation,
+static LogicalResult
+processDataOperands(llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
                     acc::UpdateOp op, SmallVector<uint64_t> &flags,
-    SmallVector<llvm::Constant *> &names, llvm::AllocaInst *argsBase,
-    llvm::AllocaInst *args, llvm::AllocaInst *argSizes) {
+                    SmallVector<llvm::Constant *> &names,
+                    SmallVector<llvm::AllocaInst *> &mapperAllocas) {
   unsigned index = 0;
 
   // Host operands are handled as `from` call.
   if (failed(processOperands(builder, moduleTranslation, op, op.hostOperands(),
                              op.getNumDataOperands(), kHostCopyoutFlag, flags,
-                             names, index, argsBase, args, argSizes)))
+                             names, index, mapperAllocas)))
     return failure();
 
   // Device operands are handled as `to` call.
   if (failed(processOperands(builder, moduleTranslation, op,
                              op.deviceOperands(), op.getNumDataOperands(),
-                             kDeviceCopyinFlag, flags, names, index, argsBase,
-                             args, argSizes)))
+                             kDeviceCopyinFlag, flags, names, index,
+                             mapperAllocas)))
     return failure();
 
   return success();
 }
 
+static SmallVector<llvm::AllocaInst *>
+createMapperAllocas(llvm::IRBuilderBase &builder,
+                    llvm::Function *enclosingFunction,
+                    unsigned totalNbOperand) {
+  llvm::LLVMContext &ctx = builder.getContext();
+  auto *i8PtrTy = llvm::Type::getInt8PtrTy(ctx);
+  auto *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, totalNbOperand);
+  auto *i64Ty = llvm::Type::getInt64Ty(ctx);
+  auto *arrI64Ty = llvm::ArrayType::get(i64Ty, totalNbOperand);
+  llvm::IRBuilder<>::InsertPoint allocaIP(
+      &enclosingFunction->getEntryBlock(),
+      enclosingFunction->getEntryBlock().getFirstInsertionPt());
+  llvm::IRBuilder<>::InsertPoint currentIP = builder.saveIP();
+  builder.restoreIP(allocaIP);
+  llvm::AllocaInst *argsBase = builder.CreateAlloca(arrI8PtrTy);
+  llvm::AllocaInst *args = builder.CreateAlloca(arrI8PtrTy);
+  llvm::AllocaInst *argSizes = builder.CreateAlloca(arrI64Ty);
+  builder.restoreIP(currentIP);
+  return {argsBase, args, argSizes};
+}
+
+static SmallVector<llvm::Value *>
+createMapperAllocasGEP(llvm::IRBuilderBase &builder,
+                       SmallVector<llvm::AllocaInst *> &mapperAllocas,
+                       unsigned totalNbOperand) {
+  llvm::LLVMContext &ctx = builder.getContext();
+  auto *i8PtrTy = llvm::Type::getInt8PtrTy(ctx);
+  auto *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, totalNbOperand);
+  auto *i64Ty = llvm::Type::getInt64Ty(ctx);
+  auto *arrI64Ty = llvm::ArrayType::get(i64Ty, totalNbOperand);
+  llvm::Value *argsBaseGEP =
+      builder.CreateInBoundsGEP(arrI8PtrTy, mapperAllocas[kArgsBasePos],
+                                {builder.getInt32(0), builder.getInt32(0)});
+  llvm::Value *argsGEP =
+      builder.CreateInBoundsGEP(arrI8PtrTy, mapperAllocas[kArgsPos],
+                                {builder.getInt32(0), builder.getInt32(0)});
+  llvm::Value *argSizesGEP =
+      builder.CreateInBoundsGEP(arrI64Ty, mapperAllocas[kArgSizesPos],
+                                {builder.getInt32(0), builder.getInt32(0)});
+  return {argsBaseGEP, argsGEP, argSizesGEP};
+}
+
 //===----------------------------------------------------------------------===//
 // Conversion functions
 //===----------------------------------------------------------------------===//
 
+/// Converts an OpenACC data operation into LLVM IR.
+static LogicalResult convertDataOp(acc::DataOp &op,
+                                   llvm::IRBuilderBase &builder,
+                                   LLVM::ModuleTranslation &moduleTranslation) {
+  llvm::LLVMContext &ctx = builder.getContext();
+  auto enclosingFuncOp =
+      op.getOperation()->template getParentOfType<LLVM::LLVMFuncOp>();
+  llvm::Function *enclosingFunction =
+      moduleTranslation.lookupFunction(enclosingFuncOp.getName());
+
+  OpenACCIRBuilder *accBuilder = moduleTranslation.getOpenMPBuilder();
+
+  auto *srcLocInfo = createSourceLocationInfo(*accBuilder, op);
+
+  auto *beginMapperFunc = accBuilder->getOrCreateRuntimeFunctionPtr(
+      llvm::omp::OMPRTL___tgt_target_data_begin_mapper);
+
+  auto *endMapperFunc = accBuilder->getOrCreateRuntimeFunctionPtr(
+      llvm::omp::OMPRTL___tgt_target_data_end_mapper);
+
+  // Number of arguments in the data operation.
+  unsigned totalNbOperand = op.getNumDataOperands();
+
+  SmallVector<llvm::AllocaInst *> mapperAllocas =
+      createMapperAllocas(builder, enclosingFunction, totalNbOperand);
+
+  SmallVector<uint64_t> flags;
+  SmallVector<llvm::Constant *> names;
+  unsigned index = 0;
+
+  // TODO handle no_create, deviceptr and attach operands.
+
+  if (failed(processOperands(builder, moduleTranslation, op, op.copyOperands(),
+                             totalNbOperand, kCopyFlag, flags, names, index,
+                             mapperAllocas)))
+    return failure();
+
+  if (failed(processOperands(
+          builder, moduleTranslation, op, op.copyinOperands(), totalNbOperand,
+          kDeviceCopyinFlag, flags, names, index, mapperAllocas)))
+    return failure();
+
+  // TODO copyin readonly currenlty handled as copyin. Update when extension
+  // available.
+  if (failed(processOperands(builder, moduleTranslation, op,
+                             op.copyinReadonlyOperands(), totalNbOperand,
+                             kDeviceCopyinFlag, flags, names, index,
+                             mapperAllocas)))
+    return failure();
+
+  if (failed(processOperands(
+          builder, moduleTranslation, op, op.copyoutOperands(), totalNbOperand,
+          kHostCopyoutFlag, flags, names, index, mapperAllocas)))
+    return failure();
+
+  // TODO copyout zero currenlty handled as copyout. Update when extension
+  // available.
+  if (failed(processOperands(builder, moduleTranslation, op,
+                             op.copyoutZeroOperands(), totalNbOperand,
+                             kHostCopyoutFlag, flags, names, index,
+                             mapperAllocas)))
+    return failure();
+
+  if (failed(processOperands(builder, moduleTranslation, op,
+                             op.createOperands(), totalNbOperand, kCreateFlag,
+                             flags, names, index, mapperAllocas)))
+    return failure();
+
+  // TODO create zero currenlty handled as create. Update when extension
+  // available.
+  if (failed(processOperands(builder, moduleTranslation, op,
+                             op.createZeroOperands(), totalNbOperand,
+                             kCreateFlag, flags, names, index, mapperAllocas)))
+    return failure();
+
+  if (failed(processOperands(builder, moduleTranslation, op,
+                             op.presentOperands(), totalNbOperand, kPresentFlag,
+                             flags, names, index, mapperAllocas)))
+    return failure();
+
+  llvm::GlobalVariable *maptypes =
+      accBuilder->createOffloadMaptypes(flags, ".offload_maptypes");
+  llvm::Value *maptypesArg = builder.CreateConstInBoundsGEP2_32(
+      llvm::ArrayType::get(llvm::Type::getInt64Ty(ctx), totalNbOperand),
+      maptypes, /*Idx0=*/0, /*Idx1=*/0);
+
+  llvm::GlobalVariable *mapnames =
+      accBuilder->createOffloadMapnames(names, ".offload_mapnames");
+  llvm::Value *mapnamesArg = builder.CreateConstInBoundsGEP2_32(
+      llvm::ArrayType::get(llvm::Type::getInt8PtrTy(ctx), totalNbOperand),
+      mapnames, /*Idx0=*/0, /*Idx1=*/0);
+
+  SmallVector<llvm::Value *> mapperAllocasGEP =
+      createMapperAllocasGEP(builder, mapperAllocas, totalNbOperand);
+  llvm::Value *nullPtr = llvm::Constant::getNullValue(
+      llvm::Type::getInt8PtrTy(ctx)->getPointerTo());
+
+  // Create call to start the data region.
+  builder.CreateCall(
+      beginMapperFunc,
+      {srcLocInfo, builder.getInt64(kDefaultDevice),
+       builder.getInt32(totalNbOperand), mapperAllocasGEP[kArgsBasePos],
+       mapperAllocasGEP[kArgsPos], mapperAllocasGEP[kArgSizesPos], maptypesArg,
+       mapnamesArg, nullPtr});
+
+  // Convert the region.
+  auto currIP = builder.saveIP();
+  llvm::BasicBlock *entryBlock = nullptr;
+
+  for (Block &bb : op.region()) {
+    llvm::BasicBlock *llvmBB = llvm::BasicBlock::Create(
+        ctx, "acc.data", builder.GetInsertBlock()->getParent());
+    if (entryBlock == nullptr)
+      entryBlock = llvmBB;
+    moduleTranslation.mapBlock(&bb, llvmBB);
+  }
+
+  auto afterDataRegion = builder.saveIP();
+
+  builder.restoreIP(currIP);
+  llvm::BranchInst *sourceTerminator = builder.CreateBr(entryBlock);
+
+  builder.restoreIP(afterDataRegion);
+  llvm::BasicBlock *endDataBlock = llvm::BasicBlock::Create(
+      ctx, "acc.end_data", builder.GetInsertBlock()->getParent());
+
+  SetVector<Block *> blocks =
+      LLVM::detail::getTopologicallySortedBlocks(op.region());
+  for (Block *bb : blocks) {
+    llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
+    if (bb->isEntryBlock()) {
+      assert(sourceTerminator->getNumSuccessors() == 1 &&
+             "provided entry block has multiple successors");
+      sourceTerminator->setSuccessor(0, llvmBB);
+    }
+
+    if (failed(
+            moduleTranslation.convertBlock(*bb, bb->isEntryBlock(), builder))) {
+      return failure();
+    }
+
+    if (isa<acc::TerminatorOp, acc::YieldOp>(bb->getTerminator()))
+      builder.CreateBr(endDataBlock);
+  }
+
+  // Create call to end the data region.
+  builder.SetInsertPoint(endDataBlock);
+  mapperAllocasGEP =
+      createMapperAllocasGEP(builder, mapperAllocas, totalNbOperand);
+  builder.CreateCall(
+      endMapperFunc,
+      {srcLocInfo, builder.getInt64(kDefaultDevice),
+       builder.getInt32(totalNbOperand), mapperAllocasGEP[kArgsBasePos],
+       mapperAllocasGEP[kArgsPos], mapperAllocasGEP[kArgSizesPos], maptypesArg,
+       mapnamesArg, nullPtr});
+
+  return success();
+}
+
 /// Converts an OpenACC standalone data operation into LLVM IR.
 template <typename OpTy>
 static LogicalResult
 convertStandaloneDataOp(OpTy &op, llvm::IRBuilderBase &builder,
                         LLVM::ModuleTranslation &moduleTranslation) {
   auto enclosingFuncOp =
       op.getOperation()->template getParentOfType<LLVM::LLVMFuncOp>();
   llvm::Function *enclosingFunction =
       moduleTranslation.lookupFunction(enclosingFuncOp.getName());
 
   OpenACCIRBuilder *accBuilder = moduleTranslation.getOpenMPBuilder();
 
   auto *srcLocInfo = createSourceLocationInfo(*accBuilder, op);
   auto *mapperFunc = getAssociatedFunction(*accBuilder, op);
 
   // Number of arguments in the enter_data operation.
   unsigned totalNbOperand = op.getNumDataOperands();
 
-  // TODO could be moved to OpenXXIRBuilder?
   llvm::LLVMContext &ctx = builder.getContext();
-  auto *i8PtrTy = llvm::Type::getInt8PtrTy(ctx);
-  auto *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, totalNbOperand);
-  auto *i64Ty = llvm::Type::getInt64Ty(ctx);
-  auto *arrI64Ty = llvm::ArrayType::get(i64Ty, totalNbOperand);
-  llvm::IRBuilder<>::InsertPoint allocaIP(
-      &enclosingFunction->getEntryBlock(),
-      enclosingFunction->getEntryBlock().getFirstInsertionPt());
-  llvm::IRBuilder<>::InsertPoint currentIP = builder.saveIP();
-  builder.restoreIP(allocaIP);
-  llvm::AllocaInst *argsBase = builder.CreateAlloca(arrI8PtrTy);
-  llvm::AllocaInst *args = builder.CreateAlloca(arrI8PtrTy);
-  llvm::AllocaInst *argSizes = builder.CreateAlloca(arrI64Ty);
-  builder.restoreIP(currentIP);
+
+  SmallVector<llvm::AllocaInst *> mapperAllocas =
+      createMapperAllocas(builder, enclosingFunction, totalNbOperand);
 
   SmallVector<uint64_t> flags;
   SmallVector<llvm::Constant *> names;
 
   if (failed(processDataOperands(builder, moduleTranslation, op, flags, names,
-                                 argsBase, args, argSizes)))
+                                 mapperAllocas)))
     return failure();
 
   llvm::GlobalVariable *maptypes =
       accBuilder->createOffloadMaptypes(flags, ".offload_maptypes");
   llvm::Value *maptypesArg = builder.CreateConstInBoundsGEP2_32(
       llvm::ArrayType::get(llvm::Type::getInt64Ty(ctx), totalNbOperand),
       maptypes, /*Idx0=*/0, /*Idx1=*/0);
 
   llvm::GlobalVariable *mapnames =
       accBuilder->createOffloadMapnames(names, ".offload_mapnames");
   llvm::Value *mapnamesArg = builder.CreateConstInBoundsGEP2_32(
       llvm::ArrayType::get(llvm::Type::getInt8PtrTy(ctx), totalNbOperand),
       mapnames, /*Idx0=*/0, /*Idx1=*/0);
 
-  llvm::Value *argsBaseGEP = builder.CreateInBoundsGEP(
-      arrI8PtrTy, argsBase, {builder.getInt32(0), builder.getInt32(0)});
-  llvm::Value *argsGEP = builder.CreateInBoundsGEP(
-      arrI8PtrTy, args, {builder.getInt32(0), builder.getInt32(0)});
-  llvm::Value *argSizesGEP = builder.CreateInBoundsGEP(
-      arrI64Ty, argSizes, {builder.getInt32(0), builder.getInt32(0)});
+  SmallVector<llvm::Value *> mapperAllocasGEP =
+      createMapperAllocasGEP(builder, mapperAllocas, totalNbOperand);
   llvm::Value *nullPtr = llvm::Constant::getNullValue(
       llvm::Type::getInt8PtrTy(ctx)->getPointerTo());
 
-  builder.CreateCall(mapperFunc,
-                     {srcLocInfo, builder.getInt64(kDefaultDevice),
-                      builder.getInt32(totalNbOperand), argsBaseGEP, argsGEP,
-                      argSizesGEP, maptypesArg, mapnamesArg, nullPtr});
+  builder.CreateCall(mapperFunc, {srcLocInfo, builder.getInt64(kDefaultDevice),
+                                  builder.getInt32(totalNbOperand),
+                                  mapperAllocasGEP[kArgsBasePos],
+                                  mapperAllocasGEP[kArgsPos],
+                                  mapperAllocasGEP[kArgSizesPos], maptypesArg,
+                                  mapnamesArg, nullPtr});
 
   return success();
 }
 
 namespace {
 
 /// Implementation of the dialect interface that converts operations belonging
 /// to the OpenACC dialect to LLVM IR.

 
 /// Given an OpenACC MLIR operation, create the corresponding LLVM IR
 /// (including OpenACC runtime calls).
 LogicalResult OpenACCDialectLLVMIRTranslationInterface::convertOperation(
     Operation *op, llvm::IRBuilderBase &builder,
     LLVM::ModuleTranslation &moduleTranslation) const {
 
   return llvm::TypeSwitch<Operation *, LogicalResult>(op)
+      .Case([&](acc::DataOp dataOp) {
+        return convertDataOp(dataOp, builder, moduleTranslation);
+      })
       .Case([&](acc::EnterDataOp enterDataOp) {
         return convertStandaloneDataOp<acc::EnterDataOp>(enterDataOp, builder,
                                                          moduleTranslation);
       })
       .Case([&](acc::ExitDataOp exitDataOp) {
         return convertStandaloneDataOp<acc::ExitDataOp>(exitDataOp, builder,
                                                         moduleTranslation);
       })
       .Case([&](acc::UpdateOp updateOp) {
         return convertStandaloneDataOp<acc::UpdateOp>(updateOp, builder,
                                                       moduleTranslation);
       })
+      .Case<acc::TerminatorOp, acc::YieldOp>([](auto op) {
+        // `yield` and `terminator` can be just omitted. The block structure was
+        // created in the function that handles their parent operation.
+        assert(op->getNumOperands() == 0 &&
+               "unexpected OpenACC terminator with operands");
+        return success();
+      })
       .Default([&](Operation *op) {
         return op->emitError("unsupported OpenACC operation: ")
                << op->getName();
       });
 }
 
 void mlir::registerOpenACCDialectTranslation(DialectRegistry &registry) {
   registry.insert<acc::OpenACCDialect>();

mlir/test/Target/LLVMIR/openacc-llvm.mlir

 
 // CHECK: [[ARGBASE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARGBASE_ALLOCA]], i32 0, i32 0
 // CHECK: [[ARG_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARG_ALLOCA]], i32 0, i32 0
 // CHECK: [[SIZE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZE_ALLOCA]], i32 0, i32 0
 
 // CHECK: call void @__tgt_target_data_update_mapper(%struct.ident_t* [[LOCGLOBAL]], i64 -1, i32 2, i8** [[ARGBASE_ALLOCA_GEP]], i8** [[ARG_ALLOCA_GEP]], i64* [[SIZE_ALLOCA_GEP]], i64* getelementptr inbounds ([{{[0-9]*}} x i64], [{{[0-9]*}} x i64]* [[MAPTYPES]], i32 0, i32 0), i8** getelementptr inbounds ([{{[0-9]*}} x i8*], [{{[0-9]*}} x i8*]* [[MAPNAMES]], i32 0, i32 0), i8** null)
 
 // CHECK: declare void @__tgt_target_data_update_mapper(%struct.ident_t*, i64, i32, i8**, i8**, i64*, i64*, i8**, i8**) #0
+
+// -----
+
+llvm.func @testdataop(%arg0: !llvm.struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, %arg1: !llvm.ptr<f32>, %arg2: !llvm.ptr<i32>) {
+  %0 = llvm.mlir.constant(10 : index) : i64
+  %1 = llvm.mlir.null : !llvm.ptr<f32>
+  %2 = llvm.getelementptr %1[%0] : (!llvm.ptr<f32>, i64) -> !llvm.ptr<f32>
+  %3 = llvm.ptrtoint %2 : !llvm.ptr<f32> to i64
+  %4 = llvm.extractvalue %arg0[1] : !llvm.struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>
+  %5 = llvm.mlir.undef : !llvm.struct<"openacc_data", (struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, ptr<f32>, i64)>
+  %6 = llvm.insertvalue %arg0, %5[0] : !llvm.struct<"openacc_data", (struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, ptr<f32>, i64)>
+  %7 = llvm.insertvalue %4, %6[1] : !llvm.struct<"openacc_data", (struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, ptr<f32>, i64)>
+  %8 = llvm.insertvalue %3, %7[2] : !llvm.struct<"openacc_data", (struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, ptr<f32>, i64)>
+  acc.data copy(%8 : !llvm.struct<"openacc_data", (struct<(ptr<f32>, ptr<f32>, i64, array<1 x i64>, array<1 x i64>)>, ptr<f32>, i64)>) copyout(%arg1 : !llvm.ptr<f32>) {
+    %9 = llvm.mlir.constant(2 : i32) : i32
+    llvm.store %9, %arg2 : !llvm.ptr<i32>
+    acc.terminator
+  }
+  llvm.return
+}
+
+// CHECK: %struct.ident_t = type { i32, i32, i32, i32, i8* }
+// CHECK: [[LOCSTR:@.*]] = private unnamed_addr constant [{{[0-9]*}} x i8] c";{{.*}};testdataop;{{[0-9]*}};{{[0-9]*}};;\00", align 1
+// CHECK: [[LOCGLOBAL:@.*]] = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([{{[0-9]*}} x i8], [{{[0-9]*}} x i8]* [[LOCSTR]], i32 0, i32 0) }, align 8
+// CHECK: [[MAPNAME1:@.*]] = private unnamed_addr constant [{{[0-9]*}} x i8] c";{{.*}};unknown;{{[0-9]*}};{{[0-9]*}};;\00", align 1
+// CHECK: [[MAPNAME2:@.*]] = private unnamed_addr constant [{{[0-9]*}} x i8] c";{{.*}};unknown;{{[0-9]*}};{{[0-9]*}};;\00", align 1
+// CHECK: [[MAPTYPES:@.*]] = private unnamed_addr constant [{{[0-9]*}} x i64] [i64 3, i64 2]
+// CHECK: [[MAPNAMES:@.*]] = private constant [{{[0-9]*}} x i8*] [i8* getelementptr inbounds ([{{[0-9]*}} x i8], [{{[0-9]*}} x i8]* [[MAPNAME1]], i32 0, i32 0), i8* getelementptr inbounds ([{{[0-9]*}} x i8], [{{[0-9]*}} x i8]* [[MAPNAME2]], i32 0, i32 0)]
+
+// CHECK: define void @testdataop({ float*, float*, i64, [1 x i64], [1 x i64] } %{{.*}}, float* [[SIMPLEPTR:%.*]], i32* %{{.*}})
+// CHECK: [[ARGBASE_ALLOCA:%.*]] = alloca [{{[0-9]*}} x i8*], align 8
+// CHECK: [[ARG_ALLOCA:%.*]] = alloca [{{[0-9]*}} x i8*], align 8
+// CHECK: [[SIZE_ALLOCA:%.*]] = alloca [{{[0-9]*}} x i64], align 8
+
+// CHECK: [[ARGBASE:%.*]] = extractvalue %openacc_data %{{.*}}, 0
+// CHECK: [[ARG:%.*]] = extractvalue %openacc_data %{{.*}}, 1
+// CHECK: [[ARGSIZE:%.*]] = extractvalue %openacc_data %{{.*}}, 2
+// CHECK: [[ARGBASEGEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARGBASE_ALLOCA]], i32 0, i32 0
+// CHECK: [[ARGBASEGEPCAST:%.*]] = bitcast i8** [[ARGBASEGEP]] to { float*, float*, i64, [1 x i64], [1 x i64] }*
+// CHECK: store { float*, float*, i64, [1 x i64], [1 x i64] } [[ARGBASE]], { float*, float*, i64, [1 x i64], [1 x i64] }* [[ARGBASEGEPCAST]], align 8
+// CHECK: [[ARGGEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARG_ALLOCA]], i32 0, i32 0
+// CHECK: [[ARGGEPCAST:%.*]] = bitcast i8** [[ARGGEP]] to float**
+// CHECK: store float* [[ARG]], float** [[ARGGEPCAST]], align 8
+// CHECK: [[SIZEGEP:%.*]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZE_ALLOCA]], i32 0, i32 0
+// CHECK: store i64 [[ARGSIZE]], i64* [[SIZEGEP]], align 4
+
+// CHECK: [[ARGBASEGEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARGBASE_ALLOCA]], i32 0, i32 1
+// CHECK: [[ARGBASEGEPCAST:%.*]] = bitcast i8** [[ARGBASEGEP]] to float**
+// CHECK: store float* [[SIMPLEPTR]], float** [[ARGBASEGEPCAST]], align 8
+// CHECK: [[ARGGEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARG_ALLOCA]], i32 0, i32 1
+// CHECK: [[ARGGEPCAST:%.*]] = bitcast i8** [[ARGGEP]] to float**
+// CHECK: store float* [[SIMPLEPTR]], float** [[ARGGEPCAST]], align 8
+// CHECK: [[SIZEGEP:%.*]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZE_ALLOCA]], i32 0, i32 1
+// CHECK: store i64 ptrtoint (i1** getelementptr (i1*, i1** null, i32 1) to i64), i64* [[SIZEGEP]], align 4
+
+// CHECK: [[ARGBASE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARGBASE_ALLOCA]], i32 0, i32 0
+// CHECK: [[ARG_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARG_ALLOCA]], i32 0, i32 0
+// CHECK: [[SIZE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZE_ALLOCA]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* [[LOCGLOBAL]], i64 -1, i32 2, i8** [[ARGBASE_ALLOCA_GEP]], i8** [[ARG_ALLOCA_GEP]], i64* [[SIZE_ALLOCA_GEP]], i64* getelementptr inbounds ([{{[0-9]*}} x i64], [{{[0-9]*}} x i64]* [[MAPTYPES]], i32 0, i32 0), i8** getelementptr inbounds ([{{[0-9]*}} x i8*], [{{[0-9]*}} x i8*]* [[MAPNAMES]], i32 0, i32 0), i8** null)
+// CHECK: br label %acc.data
+
+// CHECK:      acc.data:
+// CHECK-NEXT:   store i32 2, i32* %{{.*}}
+// CHECK-NEXT:   br label %acc.end_data
+
+// CHECK: acc.end_data:
+// CHECK:   [[ARGBASE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARGBASE_ALLOCA]], i32 0, i32 0
+// CHECK:   [[ARG_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[ARG_ALLOCA]], i32 0, i32 0
+// CHECK:   [[SIZE_ALLOCA_GEP:%.*]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZE_ALLOCA]], i32 0, i32 0
+// CHECK:   call void @__tgt_target_data_end_mapper(%struct.ident_t* [[LOCGLOBAL]], i64 -1, i32 2, i8** [[ARGBASE_ALLOCA_GEP]], i8** [[ARG_ALLOCA_GEP]], i64* [[SIZE_ALLOCA_GEP]], i64* getelementptr inbounds ([{{[0-9]*}} x i64], [{{[0-9]*}} x i64]* [[MAPTYPES]], i32 0, i32 0), i8** getelementptr inbounds ([{{[0-9]*}} x i8*], [{{[0-9]*}} x i8*]* [[MAPNAMES]], i32 0, i32 0), i8** null)
+
+// CHECK: declare void @__tgt_target_data_begin_mapper(%struct.ident_t*, i64, i32, i8**, i8**, i64*, i64*, i8**, i8**)
+// CHECK: declare void @__tgt_target_data_end_mapper(%struct.ident_t*, i64, i32, i8**, i8**, i64*, i64*, i8**, i8**)