[MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[TIFitis]

Summary

This patch adds OpenMP IRBuilder support for the Target Data directives to allow lowering to LLVM IR.

The mlir::Translation is responsible for generating supporting code for processing the map_operands through the processMapOperand function, and also generate code for the region associated with any Target Data operation.

The OMPIRBuilder is responsible for generating the begin and end mapper function calls.

Limitations:

• use_device_ptr and use_device_addr clauses are NOT supported for Target Data operation.
• nowait clauses are NOT supported for Target Enter and Exit Data operations.
• Only LLVMPointerType is supported for map_operands.

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

Instead of copy pasting this from mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp can you extract it and put it in a common shared file so bith translation can use the same code without duplication?

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

@raghavendhra put up a patch some time back and he faced some issues. It might be good to check with him or may be he can comment here.
https://reviews.llvm.org/D127037
https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

Just moving the three functions should be trivial. I'm not talking about the processMapOperand.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

I've moved getSizeInBytes.

The other two functions make use of mlir::Location and thus can't be moved trivially.

I can still try to move them by individually passing the elements of mlir::Location but that might not be ideal. Is that what you'd like?

[TIFitis]

Moved getSizeInBytes to OMPIRBuilder.cpp

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

What about a new header file in mlir/include/mlir/Target/LLVMIR/Dialect/** shared by mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp and mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp. That should be doable.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp and mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp already have access to the common mlir::Location type.

Problem is that OMPIRBuilder.cpp is the only common file between them where I can move the two functions to. Currently there are no mlir/** include files in OMPIRBuilder.cpp and it seems to me like a strict design choice to have it that way.

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

The functions can be header only. Why do you need to put them in the OMPIRBuilder.cpp? I think it is better than duplicate the exact same code over.

[TIFitis]

Added new header file mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h, moved two common functions to the new file.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

Sorry, I misunderstood you earlier.
I've added a new header file mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h, this is my first attempt at adding a new header file, please let me know if you find any issues.

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

Thanks! That's what I had in mind. We might want to check with MLIR folks if mlir::utils is suited for that. I don't mind if it is mlir::omp::builder or smth similar since it is related to the OMPIRBuilder.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

Since the utils file is common to all the dialects I kept it as mlir::utils.

How do I get the opinion from people working in MLIR on this, can you suggest some reviewers whom I can add?

[clementval]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

It's only valid for translation to the llvmir dialect so that why mlir::utils seems to generic to me.

Maybe @ftynse has some thoughts on this.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

I agree with you on that, would perhaps renaming it to something like mlir::dialect-utils be a better option?

[kiranchandramohan]

Please add more description to the summary regarding the scope of this patch. If it is only able to lower specific llvm-dialect types, please call out that. Please also explain the split in work between mlir::Translation and OpenMPIRbuilder. You can also consider splitting this into two patches. One that just adds the changes to OpenMPIRBuilder and the child version of the patch adding the translation in MLIR.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

You can post in MLIR discourse MLIR section (https://discourse.llvm.org/c/mlir/31) to get an opinion.

open-directive-utils , ompbuilder-utils are other options. Simialr names could be considered for the file name as well.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1343–1383

I've created a discourse topic for this here.
https://discourse.llvm.org/t/rfc-adding-new-util-file-to-mlir-dialect-translation/68221?u=tifitis

[ftynse]

mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h

14

This tag is wrong. It should be MLIR_TARGET_LLVMIR_DIALECT_UTILS_H.

60

Nit: please add a newline.

[ftynse]

mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h

28

It's not great to have static functions in a header. I suppose this is done to avoid build dependencies, but it's better to get the dependencies right.

[TIFitis]

Changed namespace to mlir::LLVM for common files. Addressed reviewer comments.

[TIFitis]

Added explicit failure for use_device_ptr and use_device_addr clauses.

[jdoerfert]

Please avoid the MLIR style in any other subproject. I know some of the OpenMP Builder stuff already uses llvm:: and MLIR style names, but that is in itself bad, not something we should extend.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1092

Nit: Style, no llvm::

1573

Style, see above.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4093

Style, see above.

4149

Style, see above.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

4991

Style, see above.

[TIFitis]

Addressed reviewer comments

[TIFitis]

Fixing build errors. Added mlir/lib/Target/LLVMIR/Dialect/Utils.cpp, made it part of the MLIRTargetLLVMIRExport lib.

[raghavendhra]

LGTM

[TIFitis]

@kiranchandramohan @clementval can you please take a look and let me know if any more changes are required for this patch?

Thanks,
Akash

[kiranchandramohan]

Please add tests for the MLIR portion.

Could you also post the full LLVM IR for a construct with the map clause?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4140

Nit:Remove empty line.

mlir/lib/Target/LLVMIR/Dialect/Utils.cpp

1 ↗

(On Diff #497045)

Nit: I would prefer the OpenMPCommon.cpp name suggested in https://discourse.llvm.org/t/rfc-adding-new-util-file-to-mlir-dialect-translation/68221.

[TIFitis]

In D142914#4144475, @kiranchandramohan wrote:

Please add tests for the MLIR portion.

Can you please tell me where to add these?

Could you also post the full LLVM IR for a construct with the map clause?

The following is a simple example, let me know if you'd like a more complex example with loop :)

Flang:

subroutine openmp_target_data
    integer :: i
    !$omp target data map(tofrom: i)
        i = 99;
    !$omp end target data
end subroutine openmp_target_data

FIR:

func.func @_QPopenmp_target_data() {
  %0 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFopenmp_target_dataEi"}
  omp.target_data   map((tofrom -> %0 : !fir.ref<i32>)) {
    %c99_i32 = arith.constant 99 : i32
    fir.store %c99_i32 to %0 : !fir.ref<i32>
    omp.terminator
  }
  return
}

LLVMIR:

llvm.func @_QPopenmp_target_data() {
  %0 = llvm.mlir.constant(1 : i64) : i64
  %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array<i32: 0, 0>, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr<i32>
  omp.target_data   map((tofrom -> %1 : !llvm.ptr<i32>)) {
    %2 = llvm.mlir.constant(99 : i32) : i32
    llvm.store %2, %1 : !llvm.ptr<i32>
    omp.terminator
  }
  llvm.return
}

llvm IR .ll:

; ModuleID = 'LLVMDialectModule'
source_filename = "LLVMDialectModule"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "aarch64-unknown-linux-gnu"

%struct.ident_t = type { i32, i32, i32, i32, ptr }

@0 = private unnamed_addr constant [26 x i8] c";test.mlir;unknown;4;10;;\00", align 1
@1 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
@2 = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 22, ptr @1 }, align 8
@.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
@.offload_mapnames = private constant [1 x ptr] [ptr @0]

declare ptr @malloc(i64)

declare void @free(ptr)

define void @_QPopenmp_target_data() {
  %1 = alloca [1 x ptr], align 8
  %2 = alloca [1 x ptr], align 8
  %3 = alloca [1 x i64], align 8
  %4 = alloca i32, i64 1, align 4
  br label %entry

entry:                                            ; preds = %0
  %5 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
  store ptr %4, ptr %5, align 8
  %6 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
  store ptr %4, ptr %6, align 8
  %7 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
  store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %7, align 8
  %8 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
  %9 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
  %10 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
  call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %8, ptr %9, ptr %10, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
  br label %omp.data.region

omp.data.region:                                  ; preds = %entry
  store i32 99, ptr %4, align 4
  br label %omp.region.cont

omp.region.cont:                                  ; preds = %omp.data.region
  %11 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
  %12 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
  %13 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
  call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %11, ptr %12, ptr %13, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
  ret void
}

; Function Attrs: nounwind
declare void @__tgt_target_data_begin_mapper(ptr, i64, i32, ptr, ptr, ptr, ptr, ptr, ptr) #0

; Function Attrs: nounwind
declare void @__tgt_target_data_end_mapper(ptr, i64, i32, ptr, ptr, ptr, ptr, ptr, ptr) #0

attributes #0 = { nounwind }

!llvm.module.flags = !{!0}

!0 = !{i32 2, !"Debug Info Version", i32 3}

Cheers,
Akash

mlir/lib/Target/LLVMIR/Dialect/Utils.cpp

1 ↗

(On Diff #497045)

Would you also like me to move the file inside OpenMP ( mlir/lib/Target/LLVMIR/Dialect/OpenMP ) ?

[TIFitis]

Addressed reviewer comments, fixed error in MLIR translation.

[kiranchandramohan]

In D142914#4145224, @TIFitis wrote:

In D142914#4144475, @kiranchandramohan wrote:

Please add tests for the MLIR portion.

Can you please tell me where to add these?

In mlir/test/Target/LLVMIR/openmp-llvm.mlir. You can also consider starting a new file for target constructs in that directory.

mlir/lib/Target/LLVMIR/Dialect/Utils.cpp

1 ↗

(On Diff #497045)

No, the current location is fine.

[TIFitis]

Added MLIR test.

[TIFitis]

Rebased

[TIFitis]

@kiranchandramohan I've added the MLIR test as requested. Please let me know if any other changes are required :)

[kiranchandramohan]

Thanks for making the change.
I am still going through the patch, I have a few comments.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1567–1568

SmallVector -> SmallVectorImpl
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

There is some recent recommendation to not insert artificial instructions and remove them. The preferred approach is to use splitBB function(s) inside the OpenMPIRBuilder. This function works on blocks without terminators. You can consult the IfCondition code in createParallel.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

4983

Is this a debugging leftover?

4990

Call verifyModule to ensure the IR is well formed.

mlir/include/mlir/Target/LLVMIR/Dialect/OpenMPCommon.h

13–14 ↗

(On Diff #500807)

Nit: The header guards would need changing.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1458

Ideally we would not want to create an OpenMP runtime calls in the Translation. This is the job of the OpenMPIRbuilder. I would recommend passing a boolean to the OpenMPIRBuilder and let the IRBuilder pick up the runtime function.

1479

Same comment as above.

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

1 ↗

(On Diff #500807)

In MLIR it is preferred to test the minimal set of functionalities. So you will have to minimize the CHECK lines and write them manually. Focus on Checking the runtime call and any important IR inserted by the IRBuilder. Keep the contents of the region to a minimum.
https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices

[TIFitis]

Addressed reviewer comments

[TIFitis]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

Thanks a lot for taking the time to review this lengthy patch.

This one seems a bit tricky to do. At first glance createParallel seems to be doing something different where its calling different runtime functions based on the IfCondition instead of much in the way of Control Flow changes.

The unreachable inst helps out a lot here as it makes it really easy to keep trace of the resume point for adding instructions after the BodyGen codes are generated.

I am still looking into finding a way to do this elegantly without having to use the unreachable instruction, but would it be a major blocker if we had to keep it?

I have addressed all the other changes you requested.

[kiranchandramohan]

I have some more comments.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1549

Nit: In the following comments either use full stops for all.

It will be helpful if the comments below refer to the OpenMP construct or clauses rather than the MLIR representation. Forr e.g Map Clause instead of Map operand.

1554

Can't the presence/absence of the BodyGenCB indicate the presence/absence of a region?

1560

This should be expanded to say whether it means begin or end mapper based on the value of the boolean and probably also renamed to isBegin or isEnter or something like that.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

Thanks for explaining the need for the unreachable. I will leave it with you on whether to make the change.

You can see an instance of a past request for not using temporary instruction. That patch (if for createTask) addressed the issue one way.
https://reviews.llvm.org/D130615#inline-1257711

I gave a quick try and came up with the following code. I don't think it is very elegant, but it is one way to do it.

-  // LLVM utilities like blocks with terminators.
-  auto *UI = Builder.CreateUnreachable();
+  BasicBlock *ContBB = nullptr;
   if (IfCond) {
-    auto *ThenTI =
-        SplitBlockAndInsertIfThen(IfCond, UI, /* Unreachable */ false);
-    ThenTI->getParent()->setName("omp_if.then");
-    Builder.SetInsertPoint(ThenTI);
-  } else {
-    Builder.SetInsertPoint(UI);
+    BasicBlock *EntryBB = Builder.GetInsertBlock();
+    ContBB = splitBB(Builder, /*CreateBranch*/ false);
+    BasicBlock *ThenBB =
+    BasicBlock::Create(Builder.getContext(), EntryBB->getName() + ".if",
+                       ContBB->getParent(), ContBB);
+    ContBB->setName(EntryBB->getName() + ".if.end");
+    Builder.CreateCondBr(IfCond, ThenBB, ContBB);
+    Builder.SetInsertPoint(ThenBB);
+    Builder.CreateBr(ContBB);
+    Builder.SetInsertPoint(ThenBB->getTerminator());
   }
 
   ProcessMapOpCB(Builder.saveIP(), Builder.saveIP());
@@ -4087,9 +4090,10 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createTargetData(
     emitMapperCall(Builder.saveIP(), beginMapperFunc, srcLocInfo, MapTypesArg,
                    MapNamesArg, MapperAllocas, DeviceID, MapTypeFlags.size());
 
+    BasicBlock *DataExitBB = splitBB(Builder, true, "target_data.exit");
     BodyGenCB(Builder.saveIP(), Builder.saveIP());
 
-    Builder.SetInsertPoint(UI->getParent());
+    Builder.SetInsertPoint(DataExitBB, DataExitBB->begin());
     // Create call to end the data region.
     emitMapperCall(Builder.saveIP(), endMapperFunc, srcLocInfo, MapTypesArg,
                    MapNamesArg, MapperAllocas, DeviceID, MapTypeFlags.size());
@@ -4100,11 +4104,9 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createTargetData(
                    MapTypeFlags.size());
   }
 
-  // Update the insertion point and remove the terminator we introduced.
-  Builder.SetInsertPoint(UI->getParent());
-  if (IfCond)
-    UI->getParent()->setName("omp_if.end");
-  UI->eraseFromParent();
+  if (ContBB)
+    return {ContBB, ContBB->begin()};
+

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

4915

In general, we have to test more in these unit tests. At the moment this only tests the enter data variant.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Isn't it possible to sink this whole function into the OpenMPIRBuilder by passing it a list of mapOpValue and mapTypeFlags?
lvm::Value *mapOpValue = moduleTranslation.lookupValue(mapOp);

Did i miss something? Or is this in anticipation of more processing required for other types?

1489–1497

Can all this go into the OpenMP IRBuilder? And be passed back if necessary via the callback.

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

49–80 ↗

(On Diff #502175)

Nit: Reduce this region to the minimum. One or two instructions in the region is sufficient.

[TIFitis]

Adrresed reviewer comments.

[TIFitis]

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1554

The BodyGenCB is always present as an argument right? Passing a nullptr when its not required doesn't seem possible at least when using the BodyGenCallbackTy. Is there a way to selectively pass the BodyGenCB?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

I saw the other directives were making use of an explicit ExitBB, but for those directives they always needed to splice the directive into multiple BB's anyways.

Since for target data we don't need to splice the BB unless using the if clause or target region are present, I was trying to find a way to do it without having to add a new BB at all times. IMO adding an unreachable inst that we always remove soon after is better than adding a new BB, but I'd defer to you in this case.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

4915

The IRBuilder part of the code doesn't really generate much code in itself. It mainly calls the BodyGenCB and ProcessMapCB, and makes use of the existing emitMapperCall function.

Test for emitMapperCall is already present in the unit test and the CB functors are tested through the MLIR test. Thus I've only added a single unit test for the IRBuilder, would you like me to add a few more?

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

I'm not fully sure but we might need more MLIR related things when supporting types other than LLVMPointerType. Also there is a call to mlir::LLVM::createMappingInformation.

I guess it might still be possible to move most of it to the IRBuilder, would you like me to do that?

1489–1497

If we decide to move processMapOperand then these can be moved along with it.

[kiranchandramohan]

Thanks for the update and the replies. See comments inline.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1554

The only optional CallBack seems to be the FinalizeCallbackTy. It is defined as an std::function whereas BodyGenCallBackTy is defined as a function_ref. But the definition of function_ref looks like it can be used to check whether it is a nullptr or not. Did you face any issues in trying to make it optional with a default parameter value of nullptr?

https://llvm.org/doxygen/STLFunctionalExtras_8h_source.html#l00036

void emitCancelationCheckImpl(Value *CancelFlag,
                              omp::Directive CanceledDirective,
                              FinalizeCallbackTy ExitCB = {});

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

OK. You can retain the unreachable for now.

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

4915

Yes, please add a test with a region and for exit data as well.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Callbacks are useful when there is frontend-specific handling that is required. If more types require to be handled then it is better to have the callback. We can revisit this after all types are handled. I assume, the current handling is for scalars and arrays of known-size.

[TIFitis]

Addressed comments. Rebased.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

1554

Sorry, I was being stupid and trying to pass nullptr as an argument instead of making the actual parameter optional. BodyGenCallBackTy works.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

4035–4043

Thanks. I've added a small comment in the code to explain what the UI is used for.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

I am a novice at FORTRAN so I'm not aware of all the types and scenarios.

I've tested the following cases and they work end-to-end:

Fortran:

subroutine openmp_target_data_region(a)
    real :: a(*)
    integer :: b(1024)
    character :: c
    integer, pointer :: p
    !$omp target enter data map(to: a, b, c, p)
end subroutine openmp_target_data_region

LLVM IR( flang-new -fc1 -emit-llvm -fopenmp test.f90 -o test.ll ):

; ModuleID = 'FIRModule'
source_filename = "FIRModule"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%struct.ident_t = type { i32, i32, i32, i32, ptr }

@0 = private unnamed_addr constant [13 x i8] c"loc(unknown)\00", align 1
@1 = private unnamed_addr constant [56 x i8] c";/home/akash/Documents/scratch/test2.f90;unknown;3;16;;\00", align 1
@2 = private unnamed_addr constant [56 x i8] c";/home/akash/Documents/scratch/test2.f90;unknown;4;18;;\00", align 1
@3 = private unnamed_addr constant [56 x i8] c";/home/akash/Documents/scratch/test2.f90;unknown;5;25;;\00", align 1
@4 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
@5 = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 22, ptr @4 }, align 8
@.offload_maptypes = private unnamed_addr constant [4 x i64] [i64 1, i64 1, i64 1, i64 1]
@.offload_mapnames = private constant [4 x ptr] [ptr @0, ptr @1, ptr @2, ptr @3]

declare ptr @malloc(i64)

declare void @free(ptr)

define void @openmp_target_data_region_(ptr %0) {
  %2 = alloca [4 x ptr], align 8
  %3 = alloca [4 x ptr], align 8
  %4 = alloca [4 x i64], align 8
  %5 = alloca [1024 x i32], i64 1, align 4
  %6 = alloca [1 x i8], i64 1, align 1
  %7 = alloca { ptr, i64, i32, i8, i8, i8, i8 }, i64 1, align 8
  %8 = alloca ptr, i64 1, align 8
  store ptr null, ptr %8, align 8
  br label %entry

entry:                                            ; preds = %1
  %9 = getelementptr inbounds [4 x ptr], ptr %2, i32 0, i32 0
  store ptr %0, ptr %9, align 8
  %10 = getelementptr inbounds [4 x ptr], ptr %3, i32 0, i32 0
  store ptr %0, ptr %10, align 8
  %11 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 0
  store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %11, align 8
  %12 = getelementptr inbounds [4 x ptr], ptr %2, i32 0, i32 1
  store ptr %5, ptr %12, align 8
  %13 = getelementptr inbounds [4 x ptr], ptr %3, i32 0, i32 1
  store ptr %5, ptr %13, align 8
  %14 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 1
  store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %14, align 8
  %15 = getelementptr inbounds [4 x ptr], ptr %2, i32 0, i32 2
  store ptr %6, ptr %15, align 8
  %16 = getelementptr inbounds [4 x ptr], ptr %3, i32 0, i32 2
  store ptr %6, ptr %16, align 8
  %17 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 2
  store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %17, align 8
  %18 = getelementptr inbounds [4 x ptr], ptr %2, i32 0, i32 3
  store ptr %7, ptr %18, align 8
  %19 = getelementptr inbounds [4 x ptr], ptr %3, i32 0, i32 3
  store ptr %7, ptr %19, align 8
  %20 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 3
  store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %20, align 8
  %21 = getelementptr inbounds [4 x ptr], ptr %2, i32 0, i32 0
  %22 = getelementptr inbounds [4 x ptr], ptr %3, i32 0, i32 0
  %23 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 0
  call void @__tgt_target_data_begin_mapper(ptr @5, i64 -1, i32 4, ptr %21, ptr %22, ptr %23, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
  ret void
}

; Function Attrs: nounwind
declare void @__tgt_target_data_begin_mapper(ptr, i64, i32, ptr, ptr, ptr, ptr, ptr, ptr) #0

; Function Attrs: nounwind
declare void @__tgt_target_data_end_mapper(ptr, i64, i32, ptr, ptr, ptr, ptr, ptr, ptr) #0

attributes #0 = { nounwind }

!llvm.module.flags = !{!0}

!0 = !{i32 2, !"Debug Info Version", i32 3}

If I am missing some important types here then please let me know, I'll try to see if they work and if not I'll add support for them in further patches.

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

In general how are you passing the size of the fortran variable/type to the OpenMP runtime? For scalars and arrays with sizes known at compile time, this comes from the type itself. But for other types like assumed-shape arrays, variable length arrays this information comes from the descriptor or from other fields. My question is how is this being collected and passed to the runtime?

For all the types, I see the following code in the IR you gave above for generating the ArgSizes argument of __tgt_target_data_begin_mapper. I don't understand how the code (and size) be the same for all the types.

...
%11 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 0
store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %11, align 8
...
%14 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 1
store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %14, align 8
...
%17 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 2
store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %17, align 8
...
%20 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 3
store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %20, align 8
...
%23 = getelementptr inbounds [4 x i64], ptr %4, i32 0, i32 0
call void @__tgt_target_data_begin_mapper(ptr @5, i64 -1, i32 4, ptr %21, ptr %22, ptr %23, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)

I would like some more clarity on this before proceeding. Clang generates different code for this and I see that it is appropriately filling the ArgSizes field.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

OpenMPIRBuilder::getSizeInBytes is the function responsible for calculating the ArgSizes.

For the Value : %1 = alloca i64, i64 1, align 8 it returns size as i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64) and TBH I don't understand this. This function was taken from OpenACC.

I will re-implement this function and update the patch.

[TIFitis]

Added names for offload mapper variables.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Actually, the generated code seems correct. The first answer here gives insight into how OpenMPIRBuilder::getSizeInBytes is calculating the size of the type.

Opaque pointers make it look the same for all the different types, disabling opaque pointers you get something like the following:

integer(8) :: a :

%7 = getelementptr inbounds [1 x i64], [1 x i64]* %.offload_sizes, i32 0, i32 0
store i64 ptrtoint (i64** getelementptr (i64*, i64** null, i32 1) to i64), i64* %7, align 4

integer :: b(1024) :

%8 = getelementptr inbounds [1 x i64], [1 x i64]* %.offload_sizes, i32 0, i32 0
store i64 ptrtoint ([1024 x i32]** getelementptr ([1024 x i32]*, [1024 x i32]** null, i32 1) to i64), i64* %8, align 4

Let me know if this makes sense.

Thanks,
Akash

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Ahh OK. It does make it a bit harder to read.

But going to back to my general question:

In general how are you passing the size of the fortran variable/type to the OpenMP runtime? For scalars and arrays with sizes known at compile time, this comes from the type itself. But for other types like assumed-shape arrays, variable length arrays this information comes from the descriptor or from other fields. My question is how is this being collected and passed to the runtime

Consider the following two subroutines, It has two assumed shape arrays. In sb0 it is a rank-1 array, in sb1 it is a rank-2 array. At the llvm dialect layer, these two will be represented by struct equivalents of Fortran descriptors as given below. If we now find the size of the types, it would get the size of the descriptor struct rather than it memory it is referring to. I guess this is not what we want to do. I believe this would require some special processing, unless the patch also does something for this.

omp.target_data   map((to -> %arg0 : !llvm.ptr<struct<(ptr<i32>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>>))

omp.target_data   map((to -> %arg0 : !llvm.ptr<struct<(ptr<i32>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>>))

subroutine sb0(a)
   integer :: a(:)
   !$omp target data map(to: a)
   a(10) = 20
   !$omp end target data
end subroutine

subroutine sb1(a)
   integer :: a(:,:)
   !$omp target data map(to: a)
   a(5,6) = 20
   !$omp end target data
end subroutine

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Just want to clarify that I am not expecting a fix here. But just a statement about what is supported and what is not supported.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

In line 1382 you can see how the size is determined. It is done simply by passing the map variable to the getSizeInBytes function.

I haven't added any special handling for assumed-shape arrays or any other special cases. I am not sure if it is equivalent to an int* in C, but looking at the llvm IR, if the map variable is an i32* then clang sets the size to 4, and for assumed-shape arrays in your examples sb0 and sb1 it also creates i32* in the llvm IR and the size for these calculated in the same way.

I am not sure if this is the correct behaviour. Do you know what is the correct size that should be passed in your examples to the runtime? If it is relatively straightforward then I can look into adding support for these, otherwise maybe we can add a TODO for handling assumed-shape arrays in a future patch.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Sorry, made a mistake in previous comment.

Clang sets size to 8 and not 4 for i32*.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

Also, the standard specifies assumed-size arrays are not supported. I am not sure if the arrays in your example are assumed-shape or assumed-size.

[kiranchandramohan]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

They are assumed-shape arrays.

We are probably missing a semantic check for the assumed-size case. But good to know that they need not be supported. In one of the examples, you provided, it was mentioned to be supported. If you can create a ticket for this that would be great. Gfortran catches this error.

subroutine openmp_target_data_region(a)
    real :: a(*)
    !$omp target enter data map(to: a)
end subroutine openmp_target_data_region

In general, besides assumed-shape, there are various cases where a descriptor can come in. Like for pointers and allocatable arrays.

subroutine sb4
   integer, pointer :: a(:)
   allocate(a(10))
   !$omp target data map(to: a)
   a(10) = 20
   !$omp end target data
end subroutine

For array sections:

subroutine sb4
   integer :: a(10)
   !$omp target data map(to: a(3:10))
   a(10) = 20
   !$omp end target data
end subroutine

I believe the size should be passed to the runtime, then only it can compute how many bytes should be mapped.

From your answers,

1. The following types are supported:

scalars, constant sized arrays

2. The following types are probably supported:

variable length arrays, derived types with elements of type in (1) or (2)

3. The following types are not supported:

assumed-size arrays, pointers, allocatables, derived type with elements of type in (3), array-sections (this segfaults now).

Would it be OK to add types in (2) and (3) as TODO failures in flang?

[TIFitis]

Added TODO for unsupported map operand types in Flang frontend

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1344

I've added the TODO. Please let me know if I missed any scenario there.

Thanks :)

[TIFitis]

Fix test

[kiranchandramohan]

LGTM. Please wait a day before submission. Move the flang portion to a separate commit.

flang/lib/Lower/OpenMP.cpp

727 ↗

(On Diff #505456)

Nit: expand auto.

727–738 ↗

(On Diff #505456)

These changes should go in a separate patch.

737 ↗

(On Diff #505456)

Since TODO will print a "Not yet implemented" message, I think we can remove the trailing "not supported suffix".

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1489–1497

Now that we decided not to move processMapOperand, can this be moved to the OpenMPIRBuilder?

1510

Nit: I think this should be an assertion.

[TIFitis]

Addresed reviewer comments. Removed flang changes.

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1489–1497

Hi, I am working on a patch to add use_device_ptr and use_deice_addr clauses. As part of that I have restructured how mapOperands are processed and separated the mlir and codegen part of it. These are already moved to OMPIRBuilder in that patch.

1510

AFAIK the mlir op for this only allows for one region.

[TIFitis]

Rebased.

[mehdi_amini]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1520

mapperFunc is used uninitialized here which is UB, can you look into this?

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1520

Thanks for pointing out, I'll push a patch to fix this.

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1520

https://github.com/llvm/llvm-project/commit/9ea3fcfa380c6097fddd0d9a9b2c13f0f20bc41a

This fixes it.

[mehdi_amini]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1478

This line is not needed after the fix you pushed right?

Seems like we could also just set bool mapperFunc = isa<omp::EnterDataOp>(op); or something like that.

1520

Thanks for the quick fix!

[TIFitis]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

1478

I've already removed this line in my fix.

Yes, I think we can get rid of the variable altogether and just pass the isa as an argument when calling. I'll add this change in one of my ongoing patches.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

enum OpenMPOffloadingRequiresDirFlags : int64_t {
   /// unified_address clause.
   OMP_REQ_UNIFIED_ADDRESS         = 0x004,
   /// unified_shared_memory clause.
   OMP_REQ_UNIFIED_SHARED_MEMORY   = 0x008,
   /// dynamic_allocators clause.
   OMP_REQ_DYNAMIC_ALLOCATORS      = 0x010,
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_REQ_DYNAMIC_ALLOCATORS)
 };
-
-enum OpenMPOffloadingReservedDeviceIDs {
-  /// Device ID if the device was not defined, runtime should get it
-  /// from environment variables in the spec.
-  OMP_DEVICEID_UNDEF = -1,
-};
 } // anonymous namespace
 
 /// Describes ident structure that describes a source location.
 /// All descriptions are taken from
 /// https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/kmp.h
 /// Original structure:
 /// typedef struct ident {
 ///    kmp_int32 reserved_1;   /**<  might be used in Fortran;

llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

enum class OpenMPOffloadMappingFlags : uint64_t {
   /// non-contiguous list items in target update directive
   OMP_MAP_NON_CONTIG = 0x100000000000,
   /// The 16 MSBs of the flags indicate whether the entry is member of some
   /// struct/class.
   OMP_MAP_MEMBER_OF = 0xffff000000000000,
   LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */ OMP_MAP_MEMBER_OF)
 };
 
+enum OpenMPOffloadingReservedDeviceIDs {
+  /// Device ID if the device was not defined, runtime should get it
+  /// from environment variables in the spec.
+  OMP_DEVICEID_UNDEF = -1
+};
+
 enum class AddressSpace : unsigned {
   Generic = 0,
   Global = 1,
   Shared = 3,
   Constant = 4,
   Local = 5,
 };
 

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

public:
 
   /// An ordered map of auto-generated variables to their unique names.
   /// It stores variables with the following names: 1) ".gomp_critical_user_" +
   /// <critical_section_name> + ".var" for "omp critical" directives; 2)
   /// <mangled_name_for_global_var> + ".cache." for cache for threadprivate
   /// variables.
   StringMap<Constant*, BumpPtrAllocator> InternalVars;
 
+  /// Computes the size of type in bytes.
+  Value *getSizeInBytes(Value *BasePtr);
+
   /// Create the global variable holding the offload mappings information.
   GlobalVariable *createOffloadMaptypes(SmallVectorImpl<uint64_t> &Mappings,
                                         std::string VarName);
 
   /// Create the global variable holding the offload names information.
   GlobalVariable *
   createOffloadMapnames(SmallVectorImpl<llvm::Constant *> &Names,
                         std::string VarName);
public:
   /// \param NumThreads Number default threads
   Constant *registerTargetRegionFunction(OffloadEntriesInfoManager &InfoManager,
                                          TargetRegionEntryInfo &EntryInfo,
                                          Function *OutlinedFunction,
                                          StringRef EntryFnName,
                                          StringRef EntryFnIDName,
                                          int32_t NumTeams, int32_t NumThreads);
 
+  /// Generator for '#omp target data'
+  ///
+  /// \param Loc The location where the target data construct was encountered.
+  /// \param CodeGenIP The insertion point at which the target directive code
+  /// should be placed.
+  /// \param HasRegion True if the op has a region associated with it, false
+  /// otherwise
+  /// \param MapTypeFlags BitVector storing the mapType flags for the
+  /// mapOperands
+  /// \param MapNames Names for the mapOperands
+  /// \param MapperAllocas Pointers to the AllocInsts for the map clause
+  /// \param MapperFunc Mapper Fucntion to be called for the Target Data op
+  /// \param DeviceID Stores the DeviceID from the device clause
+  /// \param IfCond Value which corresponds to the if clause condition.
+  /// \param ProcessMapOpCB Callback that will generate the mapOperand code.
+  /// \param BodyGenCB Callback that will generate the region code.
+  OpenMPIRBuilder::InsertPointTy createTargetData(
+      const LocationDescription &Loc, OpenMPIRBuilder::InsertPointTy CodeGenIP,
+      bool HasRegion, SmallVector<uint64_t> &MapTypeFlags,
+      SmallVector<Constant *> &MapNames, struct MapperAllocas &MapperAllocas,
+      Function *MapperFunc, int64_t DeviceID, Value *IfCond,
+      BodyGenCallbackTy ProcessMapOpCB, BodyGenCallbackTy BodyGenCB);
+
   /// Declarations for LLVM-IR types (simple, array, function and structure) are
   /// generated below. Their names are defined and used in OpenMPKinds.def. Here
   /// we provide the declarations, the initializeTypes function will provide the
   /// values.
   ///
   ///{
 #define OMP_TYPE(VarName, InitValue) Type *VarName = nullptr;
 #define OMP_ARRAY_TYPE(VarName, ElemTy, ArraySize)                             \
   ArrayType *VarName##Ty = nullptr;                                            \
   PointerType *VarName##PtrTy = nullptr;

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Constant *OpenMPIRBuilder::registerTargetRegionFunction(
   auto OutlinedFnID = createOutlinedFunctionID(OutlinedFn, EntryFnIDName);
   auto EntryAddr = createTargetRegionEntryAddr(OutlinedFn, EntryFnName);
   InfoManager.registerTargetRegionEntryInfo(
       EntryInfo, EntryAddr, OutlinedFnID,
       OffloadEntriesInfoManager::OMPTargetRegionEntryTargetRegion);
   return OutlinedFnID;
 }
 
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createTargetData(
+    const LocationDescription &Loc, OpenMPIRBuilder::InsertPointTy CodeGenIP,
+    bool HasRegion, SmallVector<uint64_t> &MapTypeFlags,
+    SmallVector<Constant *> &MapNames, struct MapperAllocas &MapperAllocas,
+    Function *MapperFunc, int64_t DeviceID, Value *IfCond,
+    BodyGenCallbackTy ProcessMapOpCB, BodyGenCallbackTy BodyGenCB) {
+  if (!updateToLocation(Loc))
+    return InsertPointTy();
+
+  Builder.restoreIP(CodeGenIP);
+
+  // LLVM utilities like blocks with terminators.
+  auto *UI = Builder.CreateUnreachable();
+
+  Instruction *ThenTI = UI, *ElseTI = nullptr;
+  if (IfCond) {
+    SplitBlockAndInsertIfThenElse(IfCond, UI, &ThenTI, &ElseTI);
+    ThenTI->getParent()->setName("omp_if.then");
+    ElseTI->getParent()->setName("omp_if.else");
+  }
+  Builder.SetInsertPoint(ThenTI);
+
+  ProcessMapOpCB(Builder.saveIP(), Builder.saveIP());
+
+  uint32_t SrcLocStrSize;
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc, SrcLocStrSize);
+  Value *srcLocInfo = getOrCreateIdent(SrcLocStr, SrcLocStrSize);
+
+  GlobalVariable *MapTypesGV =
+      createOffloadMaptypes(MapTypeFlags, ".offload_maptypes");
+  Value *MapTypesArg = Builder.CreateConstInBoundsGEP2_32(
+      ArrayType::get(Builder.getInt64Ty(), MapTypeFlags.size()), MapTypesGV,
+      /*Idx0=*/0, /*Idx1=*/0);
+
+  GlobalVariable *MapNamesGV =
+      createOffloadMapnames(MapNames, ".offload_mapnames");
+  Value *MapNamesArg = Builder.CreateConstInBoundsGEP2_32(
+      ArrayType::get(Builder.getInt8PtrTy(), MapNames.size()), MapNamesGV,
+      /*Idx0=*/0, /*Idx1=*/0);
+
+  if (HasRegion) {
+    Function *beginMapperFunc = getOrCreateRuntimeFunctionPtr(
+        omp::OMPRTL___tgt_target_data_begin_mapper);
+    Function *endMapperFunc =
+        getOrCreateRuntimeFunctionPtr(omp::OMPRTL___tgt_target_data_end_mapper);
+
+    // Create call to start the data region.
+    emitMapperCall(Builder.saveIP(), beginMapperFunc, srcLocInfo, MapTypesArg,
+                   MapNamesArg, MapperAllocas, DeviceID, MapTypeFlags.size());
+
+    ProcessMapOpCB(Builder.saveIP(), Builder.saveIP());
+
+    // Create call to end the data region.
+    emitMapperCall(Builder.saveIP(), endMapperFunc, srcLocInfo, MapTypesArg,
+                   MapNamesArg, MapperAllocas, DeviceID, MapTypeFlags.size());
+  } else {
+    emitMapperCall(Builder.saveIP(), MapperFunc, srcLocInfo, MapTypesArg,
+                   MapNamesArg, MapperAllocas, DeviceID, MapTypeFlags.size());
+  }
+
+  // Update the insertion point and remove the terminator we introduced.
+  Builder.SetInsertPoint(UI->getParent());
+  if (IfCond)
+    UI->getParent()->setName("omp_if.end");
+  UI->eraseFromParent();
+  return Builder.saveIP();
+}
+
 std::string OpenMPIRBuilder::getNameWithSeparators(ArrayRef<StringRef> Parts,
                                                    StringRef FirstSeparator,
                                                    StringRef Separator) {
   SmallString<128> Buffer;
   llvm::raw_svector_ostream OS(Buffer);
   StringRef Sep = FirstSeparator;
   for (StringRef Part : Parts) {
     OS << Sep << Part;
     Sep = Separator;
   }

 }
 
 Value *OpenMPIRBuilder::getOMPCriticalRegionLock(StringRef CriticalName) {
   std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
   std::string Name = getNameWithSeparators({Prefix, "var"}, ".", ".");
   return getOrCreateInternalVariable(KmpCriticalNameTy, Name);
 }
 
+Value *OpenMPIRBuilder::getSizeInBytes(Value *BasePtr) {
+
+  LLVMContext &Ctx = Builder.getContext();
+  Value *Null = Constant::getNullValue(BasePtr->getType()->getPointerTo());
+  Value *SizeGep =
+      Builder.CreateGEP(BasePtr->getType(), Null, Builder.getInt32(1));
+  Value *SizePtrToInt = Builder.CreatePtrToInt(SizeGep, Type::getInt64Ty(Ctx));
+  return SizePtrToInt;
+}
+
 GlobalVariable *
 OpenMPIRBuilder::createOffloadMaptypes(SmallVectorImpl<uint64_t> &Mappings,
                                        std::string VarName) {
   llvm::Constant *MaptypesArrayInit =
       llvm::ConstantDataArray::get(M.getContext(), Mappings);
   auto *MaptypesArrayGlobal = new llvm::GlobalVariable(
       M, MaptypesArrayInit->getType(),
       /*isConstant=*/true, llvm::GlobalValue::PrivateLinkage, MaptypesArrayInit,
       VarName);

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

TEST_F(OpenMPIRBuilderTest, EmitMapperCall) {
   EXPECT_TRUE(MapperCall->getOperand(1)->getType()->isIntegerTy(64));
   EXPECT_TRUE(MapperCall->getOperand(2)->getType()->isIntegerTy(32));
 
   EXPECT_EQ(MapperCall->getOperand(6), MaptypesArg);
   EXPECT_EQ(MapperCall->getOperand(7), MapnamesArg);
   EXPECT_TRUE(MapperCall->getOperand(8)->getType()->isPointerTy());
 }
 
+TEST_F(OpenMPIRBuilderTest, TargetEnterData) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  unsigned NumDataOperands = 1;
+  int64_t DeviceID = 2;
+  struct OpenMPIRBuilder::MapperAllocas MapperAllocas;
+  SmallVector<uint64_t> MapTypeflags = {5};
+  SmallVector<Constant *> MapNames;
+  auto *I8PtrTy = Builder.getInt8PtrTy();
+  auto *ArrI8PtrTy = ArrayType::get(I8PtrTy, NumDataOperands);
+  auto *I64Ty = Builder.getInt64Ty();
+  auto *ArrI64Ty = ArrayType::get(I64Ty, NumDataOperands);
+
+  AllocaInst *Val1 =
+      Builder.CreateAlloca(Builder.getInt32Ty(), Builder.getInt64(1));
+  ASSERT_NE(Val1, nullptr);
+
+  IRBuilder<>::InsertPoint AllocaIP(&F->getEntryBlock(),
+                                    F->getEntryBlock().getFirstInsertionPt());
+  OMPBuilder.createMapperAllocas(Builder.saveIP(), AllocaIP, NumDataOperands,
+                                 MapperAllocas);
+
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  auto ProcessMapOpCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP) {
+    Value *DataValue = Val1;
+    Value *DataPtrBase;
+    Value *DataPtr;
+    DataPtrBase = DataValue;
+    DataPtr = DataValue;
+    Builder.restoreIP(CodeGenIP);
+
+    Value *Null = Constant::getNullValue(DataValue->getType()->getPointerTo());
+    Value *SizeGep =
+        Builder.CreateGEP(DataValue->getType(), Null, Builder.getInt32(1));
+    Value *SizePtrToInt = Builder.CreatePtrToInt(SizeGep, I64Ty);
+
+    Value *PtrBaseGEP =
+        Builder.CreateInBoundsGEP(ArrI8PtrTy, MapperAllocas.ArgsBase,
+                                  {Builder.getInt32(0), Builder.getInt32(0)});
+    Value *PtrBaseCast = Builder.CreateBitCast(
+        PtrBaseGEP, DataPtrBase->getType()->getPointerTo());
+    Builder.CreateStore(DataPtrBase, PtrBaseCast);
+    Value *PtrGEP =
+        Builder.CreateInBoundsGEP(ArrI8PtrTy, MapperAllocas.Args,
+                                  {Builder.getInt32(0), Builder.getInt32(0)});
+    Value *PtrCast =
+        Builder.CreateBitCast(PtrGEP, DataPtr->getType()->getPointerTo());
+    Builder.CreateStore(DataPtr, PtrCast);
+    Value *SizeGEP =
+        Builder.CreateInBoundsGEP(ArrI64Ty, MapperAllocas.ArgSizes,
+                                  {Builder.getInt32(0), Builder.getInt32(0)});
+    Builder.CreateStore(SizePtrToInt, SizeGEP);
+  };
+  auto BodyCB = [&](InsertPointTy AllocaIP, InsertPointTy codeGenIP) {};
+
+  Builder.restoreIP(OMPBuilder.createTargetData(
+      Loc, Builder.saveIP(), /*hasRegion=*/false, MapTypeflags, MapNames,
+      MapperAllocas,
+      OMPBuilder.getOrCreateRuntimeFunctionPtr(
+          OMPRTL___tgt_target_data_begin_mapper),
+      DeviceID, /*ifCond=*/nullptr, ProcessMapOpCB, BodyCB));
+
+  CallInst *TargetDataCall = dyn_cast<CallInst>(&BB->back());
+  BB->dump();
+  EXPECT_NE(TargetDataCall, nullptr);
+  EXPECT_EQ(TargetDataCall->arg_size(), 9U);
+  EXPECT_EQ(TargetDataCall->getCalledFunction()->getName(),
+            "__tgt_target_data_begin_mapper");
+  EXPECT_TRUE(TargetDataCall->getOperand(1)->getType()->isIntegerTy(64));
+  EXPECT_TRUE(TargetDataCall->getOperand(2)->getType()->isIntegerTy(32));
+  EXPECT_TRUE(TargetDataCall->getOperand(8)->getType()->isPointerTy());
+}
+
 TEST_F(OpenMPIRBuilderTest, CreateTask) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
   F->setName("func");
   IRBuilder<> Builder(BB);
 
   AllocaInst *ValPtr32 = Builder.CreateAlloca(Builder.getInt32Ty());

mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h

+//===- Utils.h - General Utilities for translating MLIR dialect to LLVM IR-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines general utilities for MLIR Dialect translations to LLVM IR.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_TARGET_LLVMIR_DIALECT_UTILS_H
+#define MLIR_TARGET_LLVMIR_DIALECT_UTILS_H
+
+#include "mlir/IR/BuiltinAttributes.h"
+#include "mlir/IR/Location.h"
+#include "mlir/Support/LLVM.h"
+
+#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
+#include "llvm/IR/IRBuilder.h"
+
+namespace mlir {
+namespace LLVM {
+
+/// Create a constant string location from the MLIR Location information.
+llvm::Constant *createSourceLocStrFromLocation(Location loc,
+                                               llvm::OpenMPIRBuilder &builder,
+                                               StringRef name,
+                                               uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast<FileLineColLoc>()) {
+    StringRef fileName = fileLoc.getFilename();
+    unsigned lineNo = fileLoc.getLine();
+    unsigned colNo = fileLoc.getColumn();
+    return builder.getOrCreateSrcLocStr(name, fileName, lineNo, colNo, strLen);
+  }
+  std::string locStr;
+  llvm::raw_string_ostream locOS(locStr);
+  locOS << loc;
+  return builder.getOrCreateSrcLocStr(locOS.str(), strLen);
+}
+
+/// Create a constant string representing the mapping information extracted from
+/// the MLIR location information.
+llvm::Constant *createMappingInformation(Location loc,
+                                         llvm::OpenMPIRBuilder &builder) {
+  uint32_t strLen;
+  if (auto nameLoc = loc.dyn_cast<NameLoc>()) {
+    StringRef name = nameLoc.getName();
+    return createSourceLocStrFromLocation(nameLoc.getChildLoc(), builder, name,
+                                          strLen);
+  }
+  return createSourceLocStrFromLocation(loc, builder, "unknown", strLen);
+}
+
+} // namespace LLVM
+} // namespace mlir
+
+#endif // MLIR_TARGET_LLVMIR_DIALECT_UTILS_H

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

 
 #include "mlir/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.h"
 #include "mlir/Conversion/OpenACCToLLVM/ConvertOpenACCToLLVM.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/Dialect/OpenACC/OpenACC.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/Support/LLVM.h"
+#include "mlir/Target/LLVMIR/Dialect/Utils.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace mlir;
 

 static constexpr uint64_t kPresentFlag = 0x1000;
 static constexpr uint64_t kDeleteFlag = 0x008;
 // Runtime extension to implement the OpenACC second reference counter.
 static constexpr uint64_t kHoldFlag = 0x2000;
 
 /// Default value for the device id
 static constexpr int64_t kDefaultDevice = -1;
 
-/// Create a constant string location from the MLIR Location information.
-static llvm::Constant *createSourceLocStrFromLocation(Location loc,
-                                                      OpenACCIRBuilder &builder,
-                                                      StringRef name,
-                                                      uint32_t &strLen) {
-  if (auto fileLoc = loc.dyn_cast<FileLineColLoc>()) {
-    StringRef fileName = fileLoc.getFilename();
-    unsigned lineNo = fileLoc.getLine();
-    unsigned colNo = fileLoc.getColumn();
-    return builder.getOrCreateSrcLocStr(name, fileName, lineNo, colNo, strLen);
-  }
-  std::string locStr;
-  llvm::raw_string_ostream locOS(locStr);
-  locOS << loc;
-  return builder.getOrCreateSrcLocStr(locOS.str(), strLen);
-}
-
 /// Create the location struct from the operation location information.
 static llvm::Value *createSourceLocationInfo(OpenACCIRBuilder &builder,
                                              Operation *op) {
   auto loc = op->getLoc();
   auto funcOp = op->getParentOfType<LLVM::LLVMFuncOp>();
   StringRef funcName = funcOp ? funcOp.getName() : "unknown";
   uint32_t strLen;
-  llvm::Constant *locStr =
-      createSourceLocStrFromLocation(loc, builder, funcName, strLen);
+  llvm::Constant *locStr = mlir::LLVM::createSourceLocStrFromLocation(
+      loc, builder, funcName, strLen);
   return builder.getOrCreateIdent(locStr, strLen);
 }
 
-/// Create a constant string representing the mapping information extracted from
-/// the MLIR location information.
-static llvm::Constant *createMappingInformation(Location loc,
-                                                OpenACCIRBuilder &builder) {
-  uint32_t strLen;
-  if (auto nameLoc = loc.dyn_cast<NameLoc>()) {
-    StringRef name = nameLoc.getName();
-    return createSourceLocStrFromLocation(nameLoc.getChildLoc(), builder, name,
-                                          strLen);
-  }
-  return createSourceLocStrFromLocation(loc, builder, "unknown", strLen);
-}
-
 /// Return the runtime function used to lower the given operation.
 static llvm::Function *getAssociatedFunction(OpenACCIRBuilder &builder,
                                              Operation *op) {
   return llvm::TypeSwitch<Operation *, llvm::Function *>(op)
       .Case([&](acc::EnterDataOp) {
         return builder.getOrCreateRuntimeFunctionPtr(
             llvm::omp::OMPRTL___tgt_target_data_begin_mapper);
       })
       .Case([&](acc::ExitDataOp) {
         return builder.getOrCreateRuntimeFunctionPtr(
             llvm::omp::OMPRTL___tgt_target_data_end_mapper);
       })
       .Case([&](acc::UpdateOp) {
         return builder.getOrCreateRuntimeFunctionPtr(
             llvm::omp::OMPRTL___tgt_target_data_update_mapper);
       });
   llvm_unreachable("Unknown OpenACC operation");
 }
 
-/// Computes the size of type in bytes.
-static llvm::Value *getSizeInBytes(llvm::IRBuilderBase &builder,
-                                   llvm::Value *basePtr) {
-  llvm::LLVMContext &ctx = builder.getContext();
-  llvm::Value *null =
-      llvm::Constant::getNullValue(basePtr->getType()->getPointerTo());
-  llvm::Value *sizeGep =
-      builder.CreateGEP(basePtr->getType(), null, builder.getInt32(1));
-  llvm::Value *sizePtrToInt =
-      builder.CreatePtrToInt(sizeGep, llvm::Type::getInt64Ty(ctx));
-  return sizePtrToInt;
-}
-
 /// 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,
                 SmallVectorImpl<llvm::Constant *> &names, unsigned &index,
if (DataDescriptor::isValid(data)) {
       dataPtrBase =
           builder.CreateExtractValue(dataValue, kPtrBasePosInDataDescriptor);
       dataPtr = builder.CreateExtractValue(dataValue, kPtrPosInDataDescriptor);
       dataSize =
           builder.CreateExtractValue(dataValue, kSizePosInDataDescriptor);
     } else if (data.getType().isa<LLVM::LLVMPointerType>()) {
       dataPtrBase = dataValue;
       dataPtr = dataValue;
-      dataSize = getSizeInBytes(builder, dataValue);
+      dataSize = accBuilder->getSizeInBytes(dataValue);
     } else {
       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.
for (Value data : operands) {
     // Store size extracted from operand into the i-th position of argSizes.
     llvm::Value *sizeGEP = builder.CreateInBoundsGEP(
         arrI64Ty, mapperAllocas.ArgSizes,
         {builder.getInt32(0), builder.getInt32(index)});
     builder.CreateStore(dataSize, sizeGEP);
 
     flags.push_back(operandFlag);
     llvm::Constant *mapName =
-        createMappingInformation(data.getLoc(), *accBuilder);
+        mlir::LLVM::createMappingInformation(data.getLoc(), *accBuilder);
     names.push_back(mapName);
     ++index;
   }
   return success();
 }
 
 /// Process data operands from acc::EnterDataOp
 static LogicalResult

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

 // IR.
 //
 //===----------------------------------------------------------------------===//
 #include "mlir/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/IRMapping.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/Support/LLVM.h"
+#include "mlir/Target/LLVMIR/Dialect/Utils.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/TypeSwitch.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/IRBuilder.h"
 
 using namespace mlir;
 
 namespace {
 static llvm::omp::ScheduleKind
convertOmpThreadprivate(Operation &opInst, llvm::IRBuilderBase &builder,
   llvm::Value *callInst =
       moduleTranslation.getOpenMPBuilder()->createCachedThreadPrivate(
           ompLoc, data, size, cacheName);
   llvm::Value *result = builder.CreateBitCast(callInst, globalValue->getType());
   moduleTranslation.mapValue(opInst.getResult(0), result);
   return success();
 }
 
+/// Process MapOperands for Target Data directives.
+static LogicalResult processMapOperand(
+    llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation,
+    const SmallVector<Value> &mapOperands, const ArrayAttr &mapTypes,
+    SmallVector<uint64_t> &mapTypeFlags,
+    SmallVectorImpl<llvm::Constant *> &mapNames,
+    struct llvm::OpenMPIRBuilder::MapperAllocas &mapperAllocas) {
+  auto numMapOperands = mapOperands.size();
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  llvm::PointerType *i8PtrTy = builder.getInt8PtrTy();
+  llvm::ArrayType *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, numMapOperands);
+  llvm::IntegerType *i64Ty = builder.getInt64Ty();
+  llvm::ArrayType *arrI64Ty = llvm::ArrayType::get(i64Ty, numMapOperands);
+
+  unsigned index = 0;
+  for (const auto &mapOp : mapOperands) {
+    const auto &mapTypeOp = mapTypes[index];
+
+    llvm::Value *mapOpValue = moduleTranslation.lookupValue(mapOp);
+    llvm::Value *mapOpPtrBase;
+    llvm::Value *mapOpPtr;
+    llvm::Value *mapOpSize;
+
+    if (mapOp.getType().isa<LLVM::LLVMPointerType>()) {
+      mapOpPtrBase = mapOpValue;
+      mapOpPtr = mapOpValue;
+      mapOpSize = ompBuilder->getSizeInBytes(mapOpValue);
+    } else {
+      return failure();
+    }
+
+    // Store base pointer extracted from operand into the i-th position of
+    // argBase.
+    llvm::Value *ptrBaseGEP = builder.CreateInBoundsGEP(
+        arrI8PtrTy, mapperAllocas.ArgsBase,
+        {builder.getInt32(0), builder.getInt32(index)});
+    llvm::Value *ptrBaseCast = builder.CreateBitCast(
+        ptrBaseGEP, mapOpPtrBase->getType()->getPointerTo());
+    builder.CreateStore(mapOpPtrBase, ptrBaseCast);
+
+    // Store pointer extracted from operand into the i-th position of args.
+    llvm::Value *ptrGEP = builder.CreateInBoundsGEP(
+        arrI8PtrTy, mapperAllocas.Args,
+        {builder.getInt32(0), builder.getInt32(index)});
+    llvm::Value *ptrCast =
+        builder.CreateBitCast(ptrGEP, mapOpPtr->getType()->getPointerTo());
+    builder.CreateStore(mapOpPtr, ptrCast);
+
+    // Store size extracted from operand into the i-th position of argSizes.
+    llvm::Value *sizeGEP = builder.CreateInBoundsGEP(
+        arrI64Ty, mapperAllocas.ArgSizes,
+        {builder.getInt32(0), builder.getInt32(index)});
+    builder.CreateStore(mapOpSize, sizeGEP);
+
+    mapTypeFlags.push_back(mapTypeOp.dyn_cast<mlir::IntegerAttr>().getInt());
+    llvm::Constant *mapName =
+        mlir::LLVM::createMappingInformation(mapOp.getLoc(), *ompBuilder);
+    mapNames.push_back(mapName);
+    ++index;
+  }
+
+  return success();
+}
+
+static LogicalResult
+convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
+                     LLVM::ModuleTranslation &moduleTranslation) {
+  unsigned numMapOperands;
+  llvm::Function *mapperFunc;
+  llvm::Value *ifCond = nullptr;
+  int64_t deviceID = llvm::omp::OMP_DEVICEID_UNDEF;
+  SmallVector<Value> mapOperands;
+  ArrayAttr mapTypes;
+
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+
+  LogicalResult result =
+      llvm::TypeSwitch<Operation *, LogicalResult>(op)
+          .Case([&](omp::DataOp dataOp) {
+            if (dataOp.getUseDeviceAddr().size() ||
+                dataOp.getUseDevicePtr().size())
+              return failure();
+
+            if (auto ifExprVar = dataOp.getIfExpr())
+              ifCond = moduleTranslation.lookupValue(ifExprVar);
+
+            if (auto devId = dataOp.getDevice())
+              if (auto constOp = mlir::dyn_cast<mlir::LLVM::ConstantOp>(
+                      devId.getDefiningOp()))
+                if (auto intAttr =
+                        constOp.getValue().dyn_cast<mlir::IntegerAttr>())
+                  deviceID = intAttr.getInt();
+
+            numMapOperands = dataOp.getMapOperands().size();
+            mapOperands = dataOp.getMapOperands();
+            mapTypes = dataOp.getMapTypes();
+            return success();
+          })
+          .Case([&](omp::EnterDataOp enterDataOp) {
+            if (enterDataOp.getNowait())
+              return failure();
+
+            if (auto ifExprVar = enterDataOp.getIfExpr())
+              ifCond = moduleTranslation.lookupValue(ifExprVar);
+
+            if (auto devId = enterDataOp.getDevice())
+              if (auto constOp = mlir::dyn_cast<mlir::LLVM::ConstantOp>(
+                      devId.getDefiningOp()))
+                if (auto intAttr =
+                        constOp.getValue().dyn_cast<mlir::IntegerAttr>())
+                  deviceID = intAttr.getInt();
+
+            numMapOperands = enterDataOp.getMapOperands().size();
+            mapOperands = enterDataOp.getMapOperands();
+            mapTypes = enterDataOp.getMapTypes();
+            mapperFunc = ompBuilder->getOrCreateRuntimeFunctionPtr(
+                llvm::omp::OMPRTL___tgt_target_data_begin_mapper);
+            return success();
+          })
+          .Case([&](omp::ExitDataOp exitDataOp) {
+            if (exitDataOp.getNowait())
+              return failure();
+
+            if (auto ifExprVar = exitDataOp.getIfExpr())
+              ifCond = moduleTranslation.lookupValue(ifExprVar);
+
+            if (auto devId = exitDataOp.getDevice())
+              if (auto constOp = mlir::dyn_cast<mlir::LLVM::ConstantOp>(
+                      devId.getDefiningOp()))
+                if (auto intAttr =
+                        constOp.getValue().dyn_cast<mlir::IntegerAttr>())
+                  deviceID = intAttr.getInt();
+
+            numMapOperands = exitDataOp.getMapOperands().size();
+            mapOperands = exitDataOp.getMapOperands();
+            mapTypes = exitDataOp.getMapTypes();
+            mapperFunc = ompBuilder->getOrCreateRuntimeFunctionPtr(
+                llvm::omp::OMPRTL___tgt_target_data_end_mapper);
+            return success();
+          })
+          .Default([&](Operation *op) {
+            return op->emitError("unsupported OpenMP operation: ")
+                   << op->getName();
+          });
+
+  if (failed(result))
+    return failure();
+
+  llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
+  llvm::OpenMPIRBuilder::InsertPointTy allocaIP =
+      findAllocaInsertPoint(builder, moduleTranslation);
+
+  struct llvm::OpenMPIRBuilder::MapperAllocas mapperAllocas;
+  SmallVector<uint64_t> mapTypeFlags;
+  SmallVector<llvm::Constant *> mapNames;
+  ompBuilder->createMapperAllocas(builder.saveIP(), allocaIP, numMapOperands,
+                                  mapperAllocas);
+
+  using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+  LogicalResult processMapOpStatus = success();
+  auto processMapOpCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
+    builder.restoreIP(codeGenIP);
+    processMapOpStatus =
+        processMapOperand(builder, moduleTranslation, mapOperands, mapTypes,
+                          mapTypeFlags, mapNames, mapperAllocas);
+  };
+
+  LogicalResult bodyGenStatus = success();
+  auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
+    // DataOp has only one region associated with it.
+    auto &region = cast<omp::DataOp>(op).getRegion();
+    builder.restoreIP(codeGenIP);
+    convertOmpOpRegions(region, "omp.data.region", builder, moduleTranslation,
+                        bodyGenStatus);
+  };
+
+  builder.restoreIP(ompBuilder->createTargetData(
+      ompLoc, builder.saveIP(), isa<omp::DataOp>(op), mapTypeFlags, mapNames,
+      mapperAllocas, mapperFunc, deviceID, ifCond, processMapOpCB, bodyCB));
+
+  if (failed(processMapOpStatus))
+    return processMapOpStatus;
+  return bodyGenStatus;
+}
+
 namespace {
 
 /// Implementation of the dialect interface that converts operations belonging
 /// to the OpenMP dialect to LLVM IR.
 class OpenMPDialectLLVMIRTranslationInterface
     : public LLVMTranslationDialectInterface {
 public:
   using LLVMTranslationDialectInterface::LLVMTranslationDialectInterface;
return llvm::TypeSwitch<Operation *, LogicalResult>(op)
         // sections which will be used by `critical` ops and hence can be
         // ignored for lowering. The OpenMP IRBuilder will create unique
         // name for critical section names.
         return success();
       })
       .Case([&](omp::ThreadprivateOp) {
         return convertOmpThreadprivate(*op, builder, moduleTranslation);
       })
+      .Case<omp::DataOp, omp::EnterDataOp, omp::ExitDataOp>([&](auto op) {
+        return convertOmpTargetData(op, builder, moduleTranslation);
+      })
       .Default([&](Operation *inst) {
         return inst->emitError("unsupported OpenMP operation: ")
                << inst->getName();
       });
 }
 
 void mlir::registerOpenMPDialectTranslation(DialectRegistry &registry) {
   registry.insert<omp::OpenMPDialect>();