[OpenMP][MLIR] Refactor and extend current map support by adding MapInfoOp and DataBoundsOp operations to the OpenMP Dialect

[agozillon]

Summary

This patch adds two new operations:

The first is the DataBoundsOp, which is based on OpenACC's DataBoundsOp,
which holds stride, index, extent, lower bound and upper bounds
which will be used in future follow up patches to perform initial
array sectioning of mapped arrays, and Fortran pointer and
allocatable mapping. Similarly to OpenACC, this new OpenMP
DataBoundsOp also comes with a new OpenMP type, which
helps to restrict operations to accepting only
DataBoundsOp as an input or output where necessary
(or other related operations that implement this type as
a return).

The patch also adds the MapInfoOp which rolls up some of
the old map information stored in target
operations into this new operation, and adds new
information that will be utilised in the lowering of mapped
variables, e.g. the aforementioned DataBoundsOp, but also a
new ByCapture OpenMP MLIR attribute, and isImplicit boolean
attribute. Both the ByCapture and isImplicit arguments will
affect the lowering from the OpenMP dialect to LLVM-IR in
minor but important ways, such as shifting the final maptype
or generating different load/store combinations to maintain
semantics with the OpenMP standard and alignment with the
current Clang OpenMP output as best as possible.

This MapInfoOp operation is slightly based on OpenACC's
DataEntryOp, the main difference other than some slightly
different fields (e,g, isImplicit/MapType/ByCapture) is that
OpenACC's data operations "inherit" (the MLIR ODS
equivalent) from this operation, whereas in OpenMP operations
that utilise MapInfoOp's are composed of/contain them.

A series of these MapInfoOp (one per map clause list item) is
now held by target operations that represent OpenMP
directives that utilise map clauses, e.g. TargetOp. MapInfoOp's
do not have their own specialised lowering to LLVM-IR, instead
the lowering is dependent on the particular container of the
MapInfoOp's, e.g. TargetOp has its own lowering to LLVM-IR
which utilised the information stored inside of MapInfoOp's to
affect it's lowering and the end result of the LLVM-IR generated,
which in turn can differ for host and device.

This patch contains these operations, minor changes to the
printing and parsing to support them, changes to tests (only
those relevant to this segment of the patch, other test
additions and changes are in other dependent
patches in this series) and some alterations to the OpenMPToLLVM
rewriter to support the new OpenMP type and operations.

This patch is one in a series that are dependent on each
other:

https://reviews.llvm.org/D158734
https://reviews.llvm.org/D158735
https://reviews.llvm.org/D158737

[agozillon]

Trying to take a page from @skatrak's playbook and build a patch stack (a bit late to be learning this considering we're swapping to LLVM PR's soon I believe, but it felt useful with these patches). So this is patch 1/4 in a series that will depend on each other, with the intent that the 4th will pass CI via phabricators "Depends on" support, if I don't manage to make it work, it is still possible to review each patch separately and I can open up a larger patch that contains all of the segments to test that it passes CI before final commit once we are happy with each individual component. As unfortunately this patch series is very interlinked and rather large.

[agozillon]

Patch 1/4 of the map changes from the patch series made by: https://reviews.llvm.org/D158732, https://reviews.llvm.org/D158735, https://reviews.llvm.org/D158737 and https://reviews.llvm.org/D158734 that aims to expand the current map support of the OpenMP dialect and lower from Fortran -> MLIR -> LLVM IR, future support on expanding the lowering from the OpenMP dialect to LLVM IR for map on TargetOp (omp target) will be forthcoming for declare target and explicit map variables hopefully in the near future (implicit likely a little while after that).

This patch should not pass CI on its own, patch 4: https://reviews.llvm.org/D158737, should pass provided the dependencies are setup correctly.

[agozillon]

A small ping for some attention on this patch if a reviewer can spare some time, thank you very much!

[kiranchandramohan]

Please write an RFC in the MLIR discourse channel for this update.

[agozillon]

In D158732#4630432, @kiranchandramohan wrote:

Please write an RFC in the MLIR discourse channel for this update.

No problem @kiranchandramohan, I have created one here:

https://discourse.llvm.org/t/rfc-updating-extending-the-openmp-dialects-map-to-support-array-boundaries-capture-type-and-a-refactoring/73149

Please do add to it if you wish to add further discussion points.

[clementval]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1038

Would it make sense to have a common interface (BoundLikeOp) so lowering can be shared more easily.

[razvanlupusoru]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1098

Are the variable operand methods used anywhere?

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

196–204

It would improve readability if map_type_value was also printed as a string here.

199

I cannot tell if both the target operation and the map_entry operation hold the same information about the mapping flags. If yes, it might be a good idea to relegate it to a single spot - namely the map_entry operation.

[razvanlupusoru]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1038

I personally agree with Valentin's comment. I am in the process of creating a new folder for sharing interfaces between OpenMP and OpenACC. And the interface BoundLikeOp likely will find a good home there. By sharing the interfaces we are not coupling the two dialects and we can still have separate implementation with different semantics.

I also don't think this suggestion needs anything done in this patch - I am OK with omp dialect introducing its own DataBoundsOp at this time and attaching the interface later.

[agozillon]

Thank you both very much for your feedback, I appreciate it a lot, I believe I've answered all of the comments inline!

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1038

I'd certainly be happy to try and share as much as possible across the two implementations.

Although, as @razvanlupusoru mentions I would prefer to do this in a future patch if at all possible after the initial patch to land the existing shared folder lands (or if someone else would like to take up the task of creating a shared interface that'd be perfectly fine as well)!

1098

Yes, inside of OpenMPToLLVM.cpp's lowering process, the call RegionLessOpWithVarOperandsConversion uses these to generically iterate over any operation that implements these functions operands. It's likely possible to create a specialised lowering, but the lowering at the moment is just the default, i.e. it's effectively not lowered to LLVM just updated with alterations to the operands from my current understanding at least.

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

196–204

I am a little hesitant to do so. Primarily because; as you've noticed in the comment below, the information is the same as the one displayed in the primary holding operation, however, the stored information is actually a lot more than just from/to and friends, the textual pretty printing only shows a small subset of the underlying information stored in the number. It's a bit-shifted integer composed of these flags: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h#L193C38-L193C38 and the number presented here is the number that'll inevitably be in the LLVM-IR, so it makes it simple at a glance to compare the MLIR to the LLVM-IR. But perhaps that's just me and I've spent too long debugging map entries in LLVM-IR!

But yes, if we wanted to pretty print it fully and be able to retain proper MLIR parsing for these map entries I think we'd need to fully represent every flag in that enumerator which the current map implementation does not do. For example it's possible for something to be OMP_MAP_TARGET_PARAM | OMP_MAP_LITERAL | OMP_MAP_IMPLICIT, which would read something like (param, literal, implicit -> map_var) and there's a fair number of other combinations from what I can gather from Clang, which we only scratch the surface on at the moment in Fortran. What the current pretty printing seems to do is filter out the implementation specific details and present the reader with the specification defined wording for things.

So in this case I'd prefer to keep it as is, but if we do wish to change it I am also happy to do so.

199

They do currently, albeit this one filters out a lot of the implementation details stored in the information to present the specification defined wording for readers to understand a little easier. The other is just a bit-shifted series of flags, some that I believe are not specification defined and are implementation details. I gave a little more background in the prior comment.

However, aside from that it's tied to the operations printing as I tried to maintain the status quo of the original map pretty printing which uses the current operations type to pretty print some of it's values e.g. if it's a target_enter_data vs a target_exit_data we'll print the same information differently, e.g. release vs alloc. We can get around this by not making this operation specific distinction anymore in the pretty printing though and have some kind of shared printing e.g. release_or_alloc, but I'm not sure if that'd be confusing?

[razvanlupusoru]

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

196–204

OK - I understand! I think that ideally only the map entry operations hold the semantics of what needs done in the map operation. So that means eventually one would want this operation to also print with words the bit fields. I think it is OK to have both the numerical value and the words. Would you mind doing this in follow-up work?

199

From a design perspective, it would be better if the mapping operation was in a single spot - otherwise we run into possibility of mismatch.

[clementval]

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

199

As @razvanlupusoru pointed out, this design feels weird. The information is duplicated now and it doesn't feel right.

[agozillon]

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

199

I'm happy to do it in this patch, I'll move the pretty printing to be only part of the map_entry, remove the numeric value and only have a string representation for it, this will involve introducing more wording to represent the entirety of the bitfield and we'll have to combine the release and alloc keywords into a single keyword, perhaps exit_release_or_enter_alloc?

I like the number but that's more from an it's easier to debug things perspective, but I understand the opportunity for mismatches in the written IR make it a little problematic, so I'm fine with a single string representation of it if we're happy with that!

This change will likely result in some other alterations across the patch series as a lot of the tests will need some alteration (and perhaps a subsequent accept), so I apologies for the noise ahead of time.

[agozillon]

Make there be only one source of truth for map type on the map_entry operations and stringify all elements of the bitfield, adjust tests accordingly (tests will also be adjusted on other patches in the series)

[razvanlupusoru]

Thank you!

[agozillon]

@razvanlupusoru @clementval thank you both very much for your reviews!

I will leave the patch series open until Thursday to give time for further comments/feedback from other reviewers (in particular @kiranchandramohan and @jsjodin) at that point if no further comments/change requests are received I will push it upstream!

[kiranchandramohan]

Thanks @agozillon for this update.

The patch also adds a new MapEntryOp operation which rolls up some of the old map information stored in target operations into this new operation, and adds new information that will be utilised in the lowering of mapped variables, e.g. the aforementioned BoundsOp, but also a new ByCapture OpenMP MLIR attribute

Is this Operation only present for collecting information? Or does it also manage the mapping of the data? I believe OpenACC has separate operations for creating copies both into and out of an OpenACC region (Small snippet from the proposal below). If it is only for collecting the information, would it be better to call it MapInfo instead of MapEntry?

Data clause operations which have semantics at both entry and exit of region will be decomposed:
acc copy => acc.copyin (before region) + acc.copyout (after region)

An alternative proposal was to continue to take in a reference to the FIR descriptor, and collect the relevant information from the descriptor while lowering from FIR to LLVM. Did you try this?
https://github.com/flang-compiler/f18-llvm-project/pull/915

I will leave the patch series open until Thursday to give time for further comments/feedback from other reviewers (in particular @kiranchandramohan and @jsjodin) at that point if no further comments/change requests are received I will push it upstream!

We will need a review from @TIFitis (who did the initial work on the map clause).

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1038

It is probably better to define this in terms of the terminology used in the OpenMP standard. I guess this will be Array Sections in OpenMP.
https://www.openmp.org/spec-html/5.0/openmpsu21.html

1121

The description does not say what this represents or how this will be used.
Can you add a summary as well?

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

1555

Nit: newline

[agozillon]

In D158732#4644114, @kiranchandramohan wrote:

Thanks @agozillon for this update.

No problem! Thank you for the review.

The patch also adds a new MapEntryOp operation which rolls up some of the old map information stored in target operations into this new operation, and adds new information that will be utilised in the lowering of mapped variables, e.g. the aforementioned BoundsOp, but also a new ByCapture OpenMP MLIR attribute

Is this Operation only present for collecting information? Or does it also manage the mapping of the data? I believe OpenACC has separate operations for creating copies both into and out of an OpenACC region (Small snippet from the proposal below). If it is only for collecting the information, would it be better to call it MapInfo instead of MapEntry?

Data clause operations which have semantics at both entry and exit of region will be decomposed:
acc copy => acc.copyin (before region) + acc.copyout (after region)

It would probably be better to call it MapInfo in that case, as the Target related operations are in charge of the mapping via processMapInfo/genMapInfo and utilise these for information rather than a direct lowering.

An alternative proposal was to continue to take in a reference to the FIR descriptor, and collect the relevant information from the descriptor while lowering from FIR to LLVM. Did you try this?
https://github.com/flang-compiler/f18-llvm-project/pull/915

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

However, I am currently looking into a reasonable way to access the descriptors for lowering pointers and pointer contained in a derived type in the OMPIRBuilder, so perhaps it'd be useful for that, so thank you very much!

For pointers it appears you can still use the bounds information as it's generated (and it just accesses the descriptor), but for pointers contained within a derived type you cannot; at least out-of-the-box it seems, but perhaps I just need to teach the initial PFT lowering of the map to handle derived-types a bit differently (although I'm not sure the original intention for bounds was to extend further than array sectioning), but that means implicit captures of derived types won't be able to map contained pointers implicitly I think (at least trivially). So I am still leaning towards indicating to the OMPIRBuilder in someway that it's lowering a Fortran pointer with a descriptor so it can directly access related information for offloading, but I am not sure of the best way to do that unfortunately, I have a few ideas that I am thinking about, but I am not sure what the best or most reasonable way to do it would be. So if anyone has any feedback or thoughts on this I'd love to hear about it.

I will leave the patch series open until Thursday to give time for further comments/feedback from other reviewers (in particular @kiranchandramohan and @jsjodin) at that point if no further comments/change requests are received I will push it upstream!

We will need a review from @TIFitis (who did the initial work on the map clause).

Sounds good, more than happy to wait for @TIFitis to review the series! I'll wait for @TIFitis's feedback as well before I address the renaming of MapEntry to MapInfo and the addition of improved/more OpenMP specific documentation!

[razvanlupusoru]

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

I have given a lot of thought about Fortran array slices in the context of OpenACC. :) Which is why in the end I ended up deciding to use a data bounds operation. I believe the same will apply to OpenMP.

I ended up explaining a few of the points around the need of the bounds operation here (in the 3rd part of my long response):
https://discourse.llvm.org/t/rfc-openacc-dialect-data-operation-improvements/69825/6?u=razvan.lupusoru
Here are the relevant pieces of the points I made there:

• OpenACC slice semantics are not the same as slice semantics on the source language.
• In both cases, the varPtr is the loaded address from the same descriptor.
• The FIR inside the region is unaffected by the slicing operation. The original descriptor is still used.

So why is this relevant? Because the question was asked why not obtain the information from the FIR descriptor? Well, array slicing operations in directive models is not the same as slicing operation in the source language. So we cannot always use descriptor. Thus the bounds operation adds a uniform way to create data operations.

So I am still leaning towards indicating to the OMPIRBuilder in someway that it's lowering a Fortran pointer with a descriptor so it can directly access related information for offloading,

I am not convinced this additional complexity is needed. We can decompose the descriptor data action and the data action into two mapping operations. And the data action can indeed use bounds operation that loads from descriptor.

For pointers it appears you can still use the bounds information as it's generated (and it just accesses the descriptor), but for pointers contained within a derived type you cannot;

To me this seems like an implementation limitation, not a design one. Is this the case you had in mind?

program test_derived_ptr
  type container
    integer :: spacer
    integer, pointer, dimension(:) :: ptrvar
  end type
  
  type(container) :: base
  integer, pointer, dimension(:) :: secondptrvar
  integer, dimension(10), target :: array
  
  base%ptrvar => array
  secondptrvar => array
  
  !$acc data copy(base%ptrvar,secondptrvar)
  !$acc end data
end program

In this case, I indeed see that one of the operations has bounds and one does not:

%26 = acc.copyin varPtr(%25 : !fir.ptr<!fir.array<?xi32>>)   -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "base%ptrvar"}
...
%32 = acc.bounds   lowerbound(%c0_3 : index) upperbound(%31 : index) stride(%30#2 : index) startIdx(%29#0 : index) {strideInBytes = true}
%33 = fir.box_addr %27 : (!fir.box<!fir.ptr<!fir.array<?xi32>>>) -> !fir.ptr<!fir.array<?xi32>>
%34 = acc.copyin varPtr(%33 : !fir.ptr<!fir.array<?xi32>>)   bounds(%32) -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "secondptrvar"}

I don't see though why we couldn't generate a bounds operation for the derived type case. @clementval

[clementval]

In D158732#4644451, @razvanlupusoru wrote:

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

I have given a lot of thought about Fortran array slices in the context of OpenACC. :) Which is why in the end I ended up deciding to use a data bounds operation. I believe the same will apply to OpenMP.

I ended up explaining a few of the points around the need of the bounds operation here (in the 3rd part of my long response):
https://discourse.llvm.org/t/rfc-openacc-dialect-data-operation-improvements/69825/6?u=razvan.lupusoru
Here are the relevant pieces of the points I made there:

• OpenACC slice semantics are not the same as slice semantics on the source language.
• In both cases, the varPtr is the loaded address from the same descriptor.
• The FIR inside the region is unaffected by the slicing operation. The original descriptor is still used.

So why is this relevant? Because the question was asked why not obtain the information from the FIR descriptor? Well, array slicing operations in directive models is not the same as slicing operation in the source language. So we cannot always use descriptor. Thus the bounds operation adds a uniform way to create data operations.

So I am still leaning towards indicating to the OMPIRBuilder in someway that it's lowering a Fortran pointer with a descriptor so it can directly access related information for offloading,

I am not convinced this additional complexity is needed. We can decompose the descriptor data action and the data action into two mapping operations. And the data action can indeed use bounds operation that loads from descriptor.

For pointers it appears you can still use the bounds information as it's generated (and it just accesses the descriptor), but for pointers contained within a derived type you cannot;

To me this seems like an implementation limitation, not a design one. Is this the case you had in mind?

program test_derived_ptr
  type container
    integer :: spacer
    integer, pointer, dimension(:) :: ptrvar
  end type
  
  type(container) :: base
  integer, pointer, dimension(:) :: secondptrvar
  integer, dimension(10), target :: array
  
  base%ptrvar => array
  secondptrvar => array
  
  !$acc data copy(base%ptrvar,secondptrvar)
  !$acc end data
end program

In this case, I indeed see that one of the operations has bounds and one does not:

%26 = acc.copyin varPtr(%25 : !fir.ptr<!fir.array<?xi32>>)   -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "base%ptrvar"}
...
%32 = acc.bounds   lowerbound(%c0_3 : index) upperbound(%31 : index) stride(%30#2 : index) startIdx(%29#0 : index) {strideInBytes = true}
%33 = fir.box_addr %27 : (!fir.box<!fir.ptr<!fir.array<?xi32>>>) -> !fir.ptr<!fir.array<?xi32>>
%34 = acc.copyin varPtr(%33 : !fir.ptr<!fir.array<?xi32>>)   bounds(%32) -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "secondptrvar"}

I don't see though why we couldn't generate a bounds operation for the derived type case. @clementval

That's probably a missing case in the code generating the bound op. I'll look into this.

[agozillon]

In D158732#4644461, @clementval wrote:

In D158732#4644451, @razvanlupusoru wrote:

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

I have given a lot of thought about Fortran array slices in the context of OpenACC. :) Which is why in the end I ended up deciding to use a data bounds operation. I believe the same will apply to OpenMP.

I ended up explaining a few of the points around the need of the bounds operation here (in the 3rd part of my long response):
https://discourse.llvm.org/t/rfc-openacc-dialect-data-operation-improvements/69825/6?u=razvan.lupusoru
Here are the relevant pieces of the points I made there:

• OpenACC slice semantics are not the same as slice semantics on the source language.
• In both cases, the varPtr is the loaded address from the same descriptor.
• The FIR inside the region is unaffected by the slicing operation. The original descriptor is still used.

So why is this relevant? Because the question was asked why not obtain the information from the FIR descriptor? Well, array slicing operations in directive models is not the same as slicing operation in the source language. So we cannot always use descriptor. Thus the bounds operation adds a uniform way to create data operations.

So I am still leaning towards indicating to the OMPIRBuilder in someway that it's lowering a Fortran pointer with a descriptor so it can directly access related information for offloading,

I am not convinced this additional complexity is needed. We can decompose the descriptor data action and the data action into two mapping operations. And the data action can indeed use bounds operation that loads from descriptor.

For pointers it appears you can still use the bounds information as it's generated (and it just accesses the descriptor), but for pointers contained within a derived type you cannot;

To me this seems like an implementation limitation, not a design one. Is this the case you had in mind?

program test_derived_ptr
  type container
    integer :: spacer
    integer, pointer, dimension(:) :: ptrvar
  end type
  
  type(container) :: base
  integer, pointer, dimension(:) :: secondptrvar
  integer, dimension(10), target :: array
  
  base%ptrvar => array
  secondptrvar => array
  
  !$acc data copy(base%ptrvar,secondptrvar)
  !$acc end data
end program

In this case, I indeed see that one of the operations has bounds and one does not:

%26 = acc.copyin varPtr(%25 : !fir.ptr<!fir.array<?xi32>>)   -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "base%ptrvar"}
...
%32 = acc.bounds   lowerbound(%c0_3 : index) upperbound(%31 : index) stride(%30#2 : index) startIdx(%29#0 : index) {strideInBytes = true}
%33 = fir.box_addr %27 : (!fir.box<!fir.ptr<!fir.array<?xi32>>>) -> !fir.ptr<!fir.array<?xi32>>
%34 = acc.copyin varPtr(%33 : !fir.ptr<!fir.array<?xi32>>)   bounds(%32) -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "secondptrvar"}

I don't see though why we couldn't generate a bounds operation for the derived type case. @clementval

That's probably a missing case in the code generating the bound op. I'll look into this.

I think there actually is a semi-related TODO in there, as I was wondering if it works for pointers, how hard would it be to extend it for derived types, and I was just looking into seeing if I could extend it myself! :-) But as you're the expert in this area, I'll just leave looking into this particular mapping case for further down the line and focus on just individual pointer mapping for the moment until you've had time to look into it further (although, I'm happy to look into it instead, if desired). As I am more than happy to utilise the bounds once it supports it, if it was intended/is fine to support this use-case.

[agozillon]

In D158732#4644451, @razvanlupusoru wrote:

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

I have given a lot of thought about Fortran array slices in the context of OpenACC. :) Which is why in the end I ended up deciding to use a data bounds operation. I believe the same will apply to OpenMP.

I ended up explaining a few of the points around the need of the bounds operation here (in the 3rd part of my long response):
https://discourse.llvm.org/t/rfc-openacc-dialect-data-operation-improvements/69825/6?u=razvan.lupusoru
Here are the relevant pieces of the points I made there:

• OpenACC slice semantics are not the same as slice semantics on the source language.
• In both cases, the varPtr is the loaded address from the same descriptor.
• The FIR inside the region is unaffected by the slicing operation. The original descriptor is still used.

So why is this relevant? Because the question was asked why not obtain the information from the FIR descriptor? Well, array slicing operations in directive models is not the same as slicing operation in the source language. So we cannot always use descriptor. Thus the bounds operation adds a uniform way to create data operations.

Thank you very much for providing further context behind the original decision! I believe OpenMP has similar concerns as you mention.

So I am still leaning towards indicating to the OMPIRBuilder in someway that it's lowering a Fortran pointer with a descriptor so it can directly access related information for offloading,

I am not convinced this additional complexity is needed. We can decompose the descriptor data action and the data action into two mapping operations. And the data action can indeed use bounds operation that loads from descriptor.

You're very likely correct, it is additional unnecessary complexity if the bounds operation can support this case.

For pointers it appears you can still use the bounds information as it's generated (and it just accesses the descriptor), but for pointers contained within a derived type you cannot;

To me this seems like an implementation limitation, not a design one. Is this the case you had in mind?

program test_derived_ptr
  type container
    integer :: spacer
    integer, pointer, dimension(:) :: ptrvar
  end type
  
  type(container) :: base
  integer, pointer, dimension(:) :: secondptrvar
  integer, dimension(10), target :: array
  
  base%ptrvar => array
  secondptrvar => array
  
  !$acc data copy(base%ptrvar,secondptrvar)
  !$acc end data
end program

That is one of the cases I was thinking of, but I was also thinking of the case (which I now think after some further reading is illegal) where the clause specifies just the derived type so:

$acc data copy(base)

and then the underlying pointers in the derived type (ptrvar in this case) would be implicitly mapped, similarly to the member spacer. However, on further re-reading of the OpenMP specification (hence the long time for a reply, sorry for that) I don't think that's actually legal in OpenMP. I think you'd have to specify the derived type and the pointer member to map individually, so something like:

!$acc data copy(base, base%ptrvar)

Although, I think with Flang's descriptors you could technically implicitly map the pointers contained in the structure, but that's against the OpenMP specification it seems (but on the off-chance I misunderstood again, if someone knows it's legal please do mention it)! Perhaps because it'd result in more non-user specified data mapping than is necessary in a lot of cases, or because C/C++ lacks descriptors or other complex reasons I've not thought about :-)

In this case, I indeed see that one of the operations has bounds and one does not:

%26 = acc.copyin varPtr(%25 : !fir.ptr<!fir.array<?xi32>>)   -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "base%ptrvar"}
...
%32 = acc.bounds   lowerbound(%c0_3 : index) upperbound(%31 : index) stride(%30#2 : index) startIdx(%29#0 : index) {strideInBytes = true}
%33 = fir.box_addr %27 : (!fir.box<!fir.ptr<!fir.array<?xi32>>>) -> !fir.ptr<!fir.array<?xi32>>
%34 = acc.copyin varPtr(%33 : !fir.ptr<!fir.array<?xi32>>)   bounds(%32) -> !fir.ptr<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_copy>, name = "secondptrvar"}

I don't see though why we couldn't generate a bounds operation for the derived type case. @clementval

That would be very useful, as I'd very much like to be able to use bounds to map pointer elements of derived types if at all possible! (when explicitly mapped :-) )

[kiranchandramohan]

In D158732#4644451, @razvanlupusoru wrote:

I've not, at least not for array sectioning, the initial bounds work by OpenACC seemed more apt/appealing, especially to maintain some similarities across the implementations.

I have given a lot of thought about Fortran array slices in the context of OpenACC. :) Which is why in the end I ended up deciding to use a data bounds operation. I believe the same will apply to OpenMP.

I ended up explaining a few of the points around the need of the bounds operation here (in the 3rd part of my long response):
https://discourse.llvm.org/t/rfc-openacc-dialect-data-operation-improvements/69825/6?u=razvan.lupusoru
Here are the relevant pieces of the points I made there:

• OpenACC slice semantics are not the same as slice semantics on the source language.
• In both cases, the varPtr is the loaded address from the same descriptor.
• The FIR inside the region is unaffected by the slicing operation. The original descriptor is still used.

So why is this relevant? Because the question was asked why not obtain the information from the FIR descriptor? Well, array slicing operations in directive models is not the same as slicing operation in the source language. So we cannot always use descriptor. Thus the bounds operation adds a uniform way to create data operations.

Thanks for the explanation.
-> Is your point based on the assumption that we will use the Array Section Box in the region? Rather if we just reuse the FIR representation for Array Sections (i.e a new FIR Box B2 (for array(5:10)) created from the box for the whole input array B1 (for array) for only the map clause (or MLIR operand), I am assuming we can extract the relevant info (base_ptr, lower-bound, etc) from it.
-> Is capturing the base_ptr required, if we are using the same descriptor for the Array?
Note: I am not asking for a change but to understand the issues in the other representation.

[agozillon]

Update patch to address reviewer comments, more doucmentation for ops,
changed name of MapEntry->MapInfo, regress the clause parsing/printing to (almost) original state and fixed tests

[agozillon]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

1038

It's likely it will be used for a little more than just array sectioning, but that is one of its uses, it can also be utilised for pointer mapping and hopefully pointers that are member of derived-types/structs (the former I have an initial implementation of downstream, the latter I'm waiting on some future adjustments to the initial data bounds generation).

In at least Fortran for the latter, the bounds gives access to the relevant sections of the lowered descriptor information but hopefully it'll give other dialects (or languages if/when the C++ MLIR layer becomes available) the ability to lower this type of information as well for OpenMP.

Since the intent is to cover slightly more cases than just array sectioning I've stuck with the DataBoundsOp name, but I've attempted to clarify the operation a little more and make it a little more OpenMP specific. I am happy to change the name though if you think something else is better suited!

[kiranchandramohan]

LG. Nice work extracting the bits from the OpenACC proposal. Also thanks @razvanlupusoru @clementval.

Could you update the Summary and title as well? It might also be good to document in the Summary where it differs from OpenACC, and say that it is the target and target data operations that manages the mapping.

[agozillon]

In D158732#4646017, @kiranchandramohan wrote:

LG. Nice work extracting the bits from the OpenACC proposal. Also thanks @razvanlupusoru @clementval.

Thank you very much. And yes it's awesome work from @razvanlupusoru @clementval so thank you again (and thank you for the reviews as well).

Could you update the Summary and title as well? It might also be good to document in the Summary where it differs from OpenACC, and say that it is the target and target data operations that manages the mapping.

No problem, happy to do so.

Would it be possible to get an acceptance on the other patch segments in the series if you're happy with there current state as well (sorry for the bother)? After that I'll wait for @TIFitis to give his seal of approval as well and then I'll perhaps give a day or twos grace for final input before I land the patch series if that sounds reasonable?

[agozillon]

rebase to keep level with the other patches in the series

mlir/include/mlir/Dialect/OpenMP/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend/OpenMP/OMP.td)
 mlir_tablegen(OmpCommon.td --gen-directive-decl --directives-dialect=OpenMP)
 add_public_tablegen_target(omp_common_td)
 
 set(LLVM_TARGET_DEFINITIONS OpenMPOps.td)
 mlir_tablegen(OpenMPOpsDialect.h.inc -gen-dialect-decls -dialect=omp)
 mlir_tablegen(OpenMPOpsDialect.cpp.inc -gen-dialect-defs -dialect=omp)
 mlir_tablegen(OpenMPOps.h.inc -gen-op-decls)
 mlir_tablegen(OpenMPOps.cpp.inc -gen-op-defs)
+mlir_tablegen(OpenMPOpsTypes.h.inc -gen-typedef-decls -typedefs-dialect=omp)
+mlir_tablegen(OpenMPOpsTypes.cpp.inc -gen-typedef-defs -typedefs-dialect=omp)
 mlir_tablegen(OpenMPOpsEnums.h.inc -gen-enum-decls)
 mlir_tablegen(OpenMPOpsEnums.cpp.inc -gen-enum-defs)
 mlir_tablegen(OpenMPOpsAttributes.h.inc -gen-attrdef-decls -attrdefs-dialect=omp)
 mlir_tablegen(OpenMPOpsAttributes.cpp.inc -gen-attrdef-defs -attrdefs-dialect=omp)
 add_mlir_doc(OpenMPOps OpenMPDialect Dialects/ -gen-dialect-doc -dialect=omp)
 add_public_tablegen_target(MLIROpenMPOpsIncGen)
 add_dependencies(OpenMPDialectDocGen omp_common_td)
 add_mlir_interface(OpenMPOpsInterfaces)
 
 set(LLVM_TARGET_DEFINITIONS OpenMPTypeInterfaces.td)
 mlir_tablegen(OpenMPTypeInterfaces.h.inc -gen-type-interface-decls)
 mlir_tablegen(OpenMPTypeInterfaces.cpp.inc -gen-type-interface-defs)
 add_public_tablegen_target(MLIROpenMPTypeInterfacesIncGen)
 add_dependencies(mlir-generic-headers MLIROpenMPTypeInterfacesIncGen)

mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h

 #include "mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.h"
 #include "mlir/IR/Dialect.h"
 #include "mlir/IR/OpDefinition.h"
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/IR/SymbolTable.h"
 #include "mlir/Interfaces/ControlFlowInterfaces.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 
+#define GET_TYPEDEF_CLASSES
+#include "mlir/Dialect/OpenMP/OpenMPOpsTypes.h.inc"
+
 #include "mlir/Dialect/OpenMP/OpenMPOpsDialect.h.inc"
 #include "mlir/Dialect/OpenMP/OpenMPOpsEnums.h.inc"
 #include "mlir/Dialect/OpenMP/OpenMPTypeInterfaces.h.inc"
 
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.h.inc"
 
 #include "mlir/Dialect/OpenMP/OpenMPInterfaces.h"
 
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOps.h.inc"
 
 #endif // MLIR_DIALECT_OPENMP_OPENMPDIALECT_H_

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

 include "mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td"
 include "mlir/Dialect/OpenMP/OpenMPTypeInterfaces.td"
 
 def OpenMP_Dialect : Dialect {
   let name = "omp";
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect, ::mlir::func::FuncDialect"];
   let useDefaultAttributePrinterParser = 1;
+  let useDefaultTypePrinterParser = 1;
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
 include "mlir/Dialect/OpenMP/OmpCommon.td"
 
 //===----------------------------------------------------------------------===//
 //  OpenMP Attributes
 //===----------------------------------------------------------------------===//
class OpenMP_Op<string mnemonic, list<Trait> traits = []> :
       Op<OpenMP_Dialect, mnemonic, traits>;
 
 // Type which can be constraint accepting standard integers and indices.
 def IntLikeType : AnyTypeOf<[AnyInteger, Index]>;
 
 def OpenMP_PointerLikeType : TypeAlias<OpenMP_PointerLikeTypeInterface,
   "OpenMP-compatible variable type">;
 
+class OpenMP_Type<string name, string typeMnemonic> : TypeDef<OpenMP_Dialect, name> {
+  let mnemonic = typeMnemonic;
+}
+
 //===----------------------------------------------------------------------===//
 //  2.12.7 Declare Target Directive
 //===----------------------------------------------------------------------===//
 
 def DeviceTypeAny : I32EnumAttrCase<"any", 0>;
 def DeviceTypeHost   : I32EnumAttrCase<"host", 1>;
 def DeviceTypeNoHost   : I32EnumAttrCase<"nohost", 2>;
 
let extraClassDeclaration = [{
     }
     /// The i-th variable operand passed.
     Value getVariableOperand(unsigned i) {
       return getOperand(i);
     }
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// Map related constructs
+//===----------------------------------------------------------------------===//
+
+def CaptureThis : I32EnumAttrCase<"This", 0>;
+def CaptureByRef : I32EnumAttrCase<"ByRef", 1>;
+def CaptureByCopy : I32EnumAttrCase<"ByCopy", 2>;
+def CaptureVLAType : I32EnumAttrCase<"VLAType", 3>;
+
+def VariableCaptureKind : I32EnumAttr<
+    "VariableCaptureKind",
+    "variable capture kind",
+    [CaptureThis, CaptureByRef, CaptureByCopy, CaptureVLAType]> {
+  let genSpecializedAttr = 0;
+  let cppNamespace = "::mlir::omp";
+}
+
+def VariableCaptureKindAttr : EnumAttr<OpenMP_Dialect, VariableCaptureKind,
+                                       "variable_capture_kind"> {
+  let assemblyFormat = "`(` $value `)`";
+}
+
+def DataBoundsType : OpenMP_Type<"DataBounds", "data_bounds_ty"> {
+  let summary = "Type for representing omp data clause bounds information";
+}
+
+def DataBoundsOp : OpenMP_Op<"bounds",
+    [AttrSizedOperandSegments, NoMemoryEffect]> {
+  let summary = "Represents normalized bounds information for map and data clauses.";
+
+  let description = [{
+    This operation is a variation on the OpenACC dialects DataBoundsOp 
+    it is used to record bounds used in map and data clauses in a
+    normalized fashion (zero-based). This works well with the `PointerLikeType`
+    requirement in data clauses - since a `lower_bound` of 0 means looking
+    at data at the zero offset from pointer.
+
+    The operation must have an `upper_bound` or `extent` (or both are allowed -
+    but not checked for consistency). When the source language's arrays are
+    not zero-based, the `start_idx` must specify the zero-position index.
+
+    Examples below show copying a slice of 10-element array except first element.
+    To simplify examples, the constants are used directly in the acc.bounds 
+    operands - this is not the syntax of operation.
+
+    C++:
+    ```
+    int array[10];
+    #pragma target map(array[1:9])
+    ```
+    =>
+    ```mlir
+    omp.bounds lower_bound(1) upper_bound(9) extent(9) start_idx(0)
+    ```
+
+    Fortran:
+    ```
+    integer :: array(1:10)
+    !$target map(array(2:10))
+    ```
+    =>
+    ```mlir
+    omp.bounds lower_bound(1) upper_bound(9) extent(9) start_idx(1)
+    ```
+  }];
+
+  let arguments = (ins Optional<IntLikeType>:$lower_bound,
+                       Optional<IntLikeType>:$upper_bound,
+                       Optional<IntLikeType>:$extent,
+                       Optional<IntLikeType>:$stride,
+                       DefaultValuedAttr<BoolAttr, "false">:$stride_in_bytes,
+                       Optional<IntLikeType>:$start_idx);
+  let results = (outs DataBoundsType:$result);
+
+  let assemblyFormat = [{
+    oilist(
+        `lower_bound` `(` $lower_bound `:` type($lower_bound) `)`
+      | `upper_bound` `(` $upper_bound `:` type($upper_bound) `)`
+      | `extent` `(` $extent `:` type($extent) `)`
+      | `stride` `(` $stride `:` type($stride) `)`
+      | `start_idx` `(` $start_idx `:` type($start_idx) `)`
+    ) attr-dict
+  }];
+
+  let extraClassDeclaration = [{
+    /// The number of variable operands.
+    unsigned getNumVariableOperands() {
+      return getNumOperands();
+    }
+
+    /// The i-th variable operand passed.
+    Value getVariableOperand(unsigned i) {
+      return getOperands()[i];
+    }
+  }];
+
+  let hasVerifier = 1;
+}
+
+def MapEntryOp : OpenMP_Op<"map_entry", [AttrSizedOperandSegments]> {
+  let arguments = (ins OpenMP_PointerLikeType:$var_ptr,
+                       Optional<OpenMP_PointerLikeType>:$var_ptr_ptr,
+                       Variadic<DataBoundsType>:$bounds, /* rank-0 to rank-{n-1} */
+                       OptionalAttr<UI64Attr>:$map_type,
+                       OptionalAttr<VariableCaptureKindAttr>:$map_capture_type,
+                       DefaultValuedAttr<BoolAttr, "false">:$implicit,
+                       OptionalAttr<StrAttr>:$name);
+  let results = (outs OpenMP_PointerLikeType:$omp_ptr);
+
+  let description = [{
+    Description of arguments:
+    - `var_ptr`: The address of variable to copy.
+    - `var_ptr_ptr`: Specifies the address of varPtr - only used when the variable 
+    copied is a field in a struct.
+    - `bounds`: Used when copying just slice of array or array's bounds are not
+    encoded in type. They are in rank order where rank 0 is inner-most dimension.
+    - `implicit`: specified explicitly in a map clause or captured implicitly by 
+      being used in a target region with no map or other data mapping construct.
+    - 'map_clauses': OpenMP map type for this map capture, for example: from, to and 
+      always, usually mixed in with other map type signifiers such as if it's implicit
+      or a parameter. It's a bitfield composed of the OpenMP runtime flags 
+      stored in OpenMPOffloadMappingFlags.
+    - 'map_capture_type': Capture type for the variable e.g. byref, byvalue, byvla this 
+       can affect lowering.  
+    - `name`: Holds the name of variable as specified in user clause (including bounds).
+  }];
+
+  let assemblyFormat = [{
+    `var_ptr` `(` $var_ptr `:` type($var_ptr) `)`
+    oilist(
+        `var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `)`
+      | `map_clauses` `(` custom<MapClause>($map_type) `)`
+      | `capture` `(` custom<CaptureType>($map_capture_type) `)`
+      | `bounds` `(` $bounds `)`
+    ) `->` type($omp_ptr) attr-dict
+  }];
+
+  let extraClassDeclaration = [{
+    /// The number of variable operands.
+    unsigned getNumVariableOperands() {
+      return getNumOperands();
+    }
+
+    /// The i-th variable operand passed.
+    Value getVariableOperand(unsigned i) {
+      return getOperands()[i];
+    }
+  }];
+}
+
 //===---------------------------------------------------------------------===//
 // 2.14.2 target data Construct
 //===---------------------------------------------------------------------===//
 
 def Target_DataOp: OpenMP_Op<"target_data", [AttrSizedOperandSegments]>{
   let summary = "target data construct";
   let description = [{
     Map variables to a device data environment for the extent of the region.
let description = [{
 
     TODO:  depend clause and map_type_modifier values iterator and mapper.
   }];
 
   let arguments = (ins Optional<I1>:$if_expr,
                        Optional<AnyInteger>:$device,
                        Variadic<OpenMP_PointerLikeType>:$use_device_ptr,
                        Variadic<OpenMP_PointerLikeType>:$use_device_addr,
-                       Variadic<OpenMP_PointerLikeType>:$map_operands,
-                       OptionalAttr<I64ArrayAttr>:$map_types);
+                       Variadic<OpenMP_PointerLikeType>:$map_operands);
 
   let regions = (region AnyRegion:$region);
 
   let assemblyFormat = [{
     oilist(`if` `(` $if_expr `:` type($if_expr) `)`
     | `device` `(` $device `:` type($device) `)`
-    | `map`
-      `(` custom<MapClause>($map_operands, type($map_operands), $map_types) `)`
+    | `map_entries` `(` $map_operands `:` type($map_operands) `)`
     | `use_device_ptr` `(` $use_device_ptr `:` type($use_device_ptr) `)`
     | `use_device_addr` `(` $use_device_addr `:` type($use_device_addr) `)`)
     $region attr-dict
   }];
 
   let hasVerifier = 1;
 }
 
let description = [{
     The $map_types specifies the types and modifiers for the map clause.
 
     TODO:  depend clause and map_type_modifier values iterator and mapper.
   }];
 
   let arguments = (ins Optional<I1>:$if_expr,
                        Optional<AnyInteger>:$device,
                        UnitAttr:$nowait,
-                       Variadic<OpenMP_PointerLikeType>:$map_operands,
-                       I64ArrayAttr:$map_types);
+                       Variadic<OpenMP_PointerLikeType>:$map_operands);
 
   let assemblyFormat = [{
     oilist(`if` `(` $if_expr `:` type($if_expr) `)`
     | `device` `(` $device `:` type($device) `)`
-    | `nowait` $nowait)
-    `map` `(` custom<MapClause>($map_operands, type($map_operands), $map_types) `)`
-    attr-dict
+    | `nowait` $nowait
+    | `map_entries` `(` $map_operands `:` type($map_operands) `)`
+    ) attr-dict
    }];
 
   let hasVerifier = 1;
 }
 
 //===---------------------------------------------------------------------===//
 // 2.14.4 target exit data Construct
 //===---------------------------------------------------------------------===//
let description = [{
     The $map_types specifies the types and modifiers for the map clause.
 
     TODO:  depend clause and map_type_modifier values iterator and mapper.
   }];
 
   let arguments = (ins Optional<I1>:$if_expr,
                        Optional<AnyInteger>:$device,
                        UnitAttr:$nowait,
-                       Variadic<OpenMP_PointerLikeType>:$map_operands,
-                       I64ArrayAttr:$map_types);
+                       Variadic<OpenMP_PointerLikeType>:$map_operands);
 
   let assemblyFormat = [{
     oilist(`if` `(` $if_expr `:` type($if_expr) `)`
     | `device` `(` $device `:` type($device) `)`
-    | `nowait` $nowait)
-    `map` `(` custom<MapClause>($map_operands, type($map_operands), $map_types) `)`
-    attr-dict
+    | `nowait` $nowait
+    | `map_entries` `(` $map_operands `:` type($map_operands) `)`
+    ) attr-dict
    }];
 
   let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // 2.14.5 target construct
 //===----------------------------------------------------------------------===//
let description = [{
     TODO:  is_device_ptr, depend, defaultmap, in_reduction
 
   }];
 
   let arguments = (ins Optional<I1>:$if_expr,
                        Optional<AnyInteger>:$device,
                        Optional<AnyInteger>:$thread_limit,
                        UnitAttr:$nowait,
-                       Variadic<OpenMP_PointerLikeType>:$map_operands,
-                       OptionalAttr<I64ArrayAttr>:$map_types);
+                       Variadic<OpenMP_PointerLikeType>:$map_operands);
 
   let regions = (region AnyRegion:$region);
 
   let assemblyFormat = [{
     oilist( `if` `(` $if_expr `)`
     | `device` `(` $device `:` type($device) `)`
     | `thread_limit` `(` $thread_limit `:` type($thread_limit) `)`
     | `nowait` $nowait
-    | `map` `(` custom<MapClause>($map_operands, type($map_operands), $map_types) `)`
+    | `map_entries` `(` $map_operands `:` type($map_operands) `)`
     ) $region attr-dict
   }];
 
   let hasVerifier = 1;
 }
 
 
 //===----------------------------------------------------------------------===//

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

target.addDynamicallyLegalOp<
       mlir::omp::DataOp, mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
       mlir::omp::SimdLoopOp, mlir::omp::MasterOp, mlir::omp::SectionOp,
       mlir::omp::SectionsOp, mlir::omp::SingleOp, mlir::omp::TaskGroupOp,
       mlir::omp::TaskOp>([&](Operation *op) {
     return typeConverter.isLegal(&op->getRegion(0)) &&
            typeConverter.isLegal(op->getOperandTypes()) &&
            typeConverter.isLegal(op->getResultTypes());
   });
-  target.addDynamicallyLegalOp<mlir::omp::AtomicReadOp,
-                               mlir::omp::AtomicWriteOp, mlir::omp::FlushOp,
-                               mlir::omp::ThreadprivateOp, mlir::omp::YieldOp,
-                               mlir::omp::EnterDataOp, mlir::omp::ExitDataOp>(
+  target.addDynamicallyLegalOp<
+      mlir::omp::AtomicReadOp, mlir::omp::AtomicWriteOp, mlir::omp::FlushOp,
+      mlir::omp::ThreadprivateOp, mlir::omp::YieldOp, mlir::omp::EnterDataOp,
+      mlir::omp::ExitDataOp, mlir::omp::DataBoundsOp, mlir::omp::MapEntryOp>(
       [&](Operation *op) {
         return typeConverter.isLegal(op->getOperandTypes()) &&
                typeConverter.isLegal(op->getResultTypes());
       });
   target.addDynamicallyLegalOp<mlir::omp::ReductionOp>([&](Operation *op) {
     return typeConverter.isLegal(op->getOperandTypes());
   });
   target.addDynamicallyLegalOp<mlir::omp::ReductionDeclareOp>(
       [&](Operation *op) {
         return typeConverter.isLegal(&op->getRegion(0)) &&
                typeConverter.isLegal(&op->getRegion(1)) &&
                typeConverter.isLegal(&op->getRegion(2)) &&
                typeConverter.isLegal(op->getOperandTypes()) &&
                typeConverter.isLegal(op->getResultTypes());
       });
 }
 
 void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
                                                   RewritePatternSet &patterns) {
+  // This type is allowed when converting OpenMP to LLVM Dialect, it carries
+  // bounds information for map clauses and the operation and type are
+  // discarded on lowering to LLVM-IR from the OpenMP dialect.
+  converter.addConversion(
+      [&](omp::DataBoundsType type) -> Type { return type; });
+
   patterns.add<
       AtomicReadOpConversion, ReductionOpConversion,
       ReductionDeclareOpConversion, RegionOpConversion<omp::CriticalOp>,
       RegionOpConversion<omp::MasterOp>, ReductionOpConversion,
       RegionOpConversion<omp::ParallelOp>, RegionOpConversion<omp::WsLoopOp>,
       RegionOpConversion<omp::SectionsOp>, RegionOpConversion<omp::SectionOp>,
       RegionOpConversion<omp::SimdLoopOp>, RegionOpConversion<omp::SingleOp>,
       RegionOpConversion<omp::TaskGroupOp>, RegionOpConversion<omp::TaskOp>,
       RegionOpConversion<omp::DataOp>, RegionOpConversion<omp::TargetOp>,
       RegionLessOpWithVarOperandsConversion<omp::AtomicWriteOp>,
       RegionOpWithVarOperandsConversion<omp::AtomicUpdateOp>,
       RegionLessOpWithVarOperandsConversion<omp::FlushOp>,
       RegionLessOpWithVarOperandsConversion<omp::ThreadprivateOp>,
       RegionLessOpConversion<omp::YieldOp>,
       RegionLessOpConversion<omp::EnterDataOp>,
-      RegionLessOpConversion<omp::ExitDataOp>>(converter);
+      RegionLessOpConversion<omp::ExitDataOp>,
+      RegionLessOpWithVarOperandsConversion<omp::DataBoundsOp>,
+      RegionLessOpWithVarOperandsConversion<omp::MapEntryOp>>(converter);
 }
 
 namespace {
 struct ConvertOpenMPToLLVMPass
     : public impl::ConvertOpenMPToLLVMPassBase<ConvertOpenMPToLLVMPass> {
   using Base::Base;
 
   void runOnOperation() override;

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

void OpenMPDialect::initialize() {
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"
       >();
   addAttributes<
 #define GET_ATTRDEF_LIST
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
       >();
+  addTypes<
+#define GET_TYPEDEF_LIST
+#include "mlir/Dialect/OpenMP/OpenMPOpsTypes.cpp.inc"
+      >();
 
   addInterface<OpenMPDialectFoldInterface>();
   LLVM::LLVMPointerType::attachInterface<
       PointerLikeModel<LLVM::LLVMPointerType>>(*getContext());
   MemRefType::attachInterface<PointerLikeModel<MemRefType>>(*getContext());
   LLVM::LLVMPointerType::attachInterface<
       PointerLikeModel<LLVM::LLVMPointerType>>(*getContext());
 
if (nonspeculative && speculative)
     return op->emitOpError() << "the hints omp_sync_hint_nonspeculative and "
                                 "omp_sync_hint_speculative cannot be combined.";
   return success();
 }
 
 //===----------------------------------------------------------------------===//
 // Parser, printer and verifier for Target
 //===----------------------------------------------------------------------===//
-/// Parses a Map Clause.
+
+// Helper function to get bitwise AND of `value` and 'flag'
+uint64_t mapTypeToBitFlag(uint64_t value,
+                          llvm::omp::OpenMPOffloadMappingFlags flag) {
+  return value &
+         static_cast<
+             std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+             flag);
+}
+
+/// Parses a map_entries map type from a string format back into its numeric
+/// value.
 ///
-/// map-clause = `map (` ( `(` `always, `? `close, `? `present, `? ( `to` |
-/// `from` | `delete` ) ` -> ` symbol-ref ` : ` type(symbol-ref) `)` )+ `)`
-/// Eg: map((release -> %1 : !llvm.ptr<array<1024 x i32>>), (always, close, from
-/// -> %2 : !llvm.ptr<array<1024 x i32>>))
-static ParseResult
-parseMapClause(OpAsmParser &parser,
-               SmallVectorImpl<OpAsmParser::UnresolvedOperand> &map_operands,
-               SmallVectorImpl<Type> &map_operand_types, ArrayAttr &map_types) {
-  StringRef mapTypeMod;
-  OpAsmParser::UnresolvedOperand arg1;
-  Type arg1Type;
-  IntegerAttr arg2;
-  SmallVector<IntegerAttr> mapTypesVec;
-  llvm::omp::OpenMPOffloadMappingFlags mapTypeBits;
+/// map-clause = `map_clauses ( `always, `? `close, `? `present, `? (
+/// `to` | `from` | `delete` | `delete` | `exit_release_or_enter_alloc` )`
+/// `, `? `, `? `ptr_and_obj, `? `target_param, `? `return_param, `?
+/// `private, `? `literal, `? `implicit, `? `ompx_hold, `? `non_contig, `?
+/// `member_of`? `)`
+static ParseResult parseMapClause(OpAsmParser &parser, IntegerAttr &mapType) {
+  llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE;
 
+  // This simply verifies the correct keyword is read in, the
+  // keyword itself is stored inside of the operation
   auto parseTypeAndMod = [&]() -> ParseResult {
+    StringRef mapTypeMod;
     if (parser.parseKeyword(&mapTypeMod))
       return failure();
 
     if (mapTypeMod == "always")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS;
+
     if (mapTypeMod == "close")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE;
+
     if (mapTypeMod == "present")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRESENT;
 
     if (mapTypeMod == "to")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
+
     if (mapTypeMod == "from")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+
     if (mapTypeMod == "tofrom")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
                      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+
     if (mapTypeMod == "delete")
       mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE;
-    return success();
-  };
 
-  auto parseMap = [&]() -> ParseResult {
-    mapTypeBits = llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE;
+    if (mapTypeMod == "ptr_and_obj")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+    if (mapTypeMod == "target_param")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM;
+
+    if (mapTypeMod == "return_param")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_RETURN_PARAM;
+
+    if (mapTypeMod == "private")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRIVATE;
+
+    if (mapTypeMod == "literal")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL;
+
+    if (mapTypeMod == "implicit")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT;
+
+    if (mapTypeMod == "ompx_hold")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_OMPX_HOLD;
+
+    if (mapTypeMod == "non_contig")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NON_CONTIG;
+
+    if (mapTypeMod == "member_of")
+      mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF;
 
-    if (parser.parseLParen() ||
-        parser.parseCommaSeparatedList(parseTypeAndMod) ||
-        parser.parseArrow() || parser.parseOperand(arg1) ||
-        parser.parseColon() || parser.parseType(arg1Type) ||
-        parser.parseRParen())
-      return failure();
-    map_operands.push_back(arg1);
-    map_operand_types.push_back(arg1Type);
-    arg2 = parser.getBuilder().getIntegerAttr(
-        parser.getBuilder().getI64Type(),
-        static_cast<
-            std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-            mapTypeBits));
-    mapTypesVec.push_back(arg2);
     return success();
   };
 
-  if (parser.parseCommaSeparatedList(parseMap))
+  if (parser.parseCommaSeparatedList(parseTypeAndMod))
     return failure();
 
-  SmallVector<Attribute> mapTypesAttr(mapTypesVec.begin(), mapTypesVec.end());
-  map_types = ArrayAttr::get(parser.getContext(), mapTypesAttr);
+  mapType = parser.getBuilder().getIntegerAttr(
+      parser.getBuilder().getIntegerType(64, /*isSigned=*/false),
+      static_cast<std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+          mapTypeBits));
+
   return success();
 }
 
+/// Prints a map_entries map type from its numeric value out into its string
+/// format.
 static void printMapClause(OpAsmPrinter &p, Operation *op,
-                           OperandRange map_operands,
-                           TypeRange map_operand_types, ArrayAttr map_types) {
+                           IntegerAttr mapType) {
+  uint64_t mapTypeBits = mapType.getUInt();
 
-  // Helper function to get bitwise AND of `value` and 'flag'
-  auto bitAnd = [](int64_t value,
-                   llvm::omp::OpenMPOffloadMappingFlags flag) -> bool {
-    return value &
-           static_cast<
-               std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-               flag);
-  };
-
-  assert(map_operands.size() == map_types.size());
+  bool emitAllocRelease = true;
+  llvm::SmallVector<std::string, 4> mapTypeStrs;
 
-  for (unsigned i = 0, e = map_operands.size(); i < e; i++) {
-    int64_t mapTypeBits = 0x00;
-    Value mapOp = map_operands[i];
-    Attribute mapTypeOp = map_types[i];
-
-    assert(llvm::isa<mlir::IntegerAttr>(mapTypeOp));
-    mapTypeBits = llvm::cast<mlir::IntegerAttr>(mapTypeOp).getInt();
-
-    bool always = bitAnd(mapTypeBits,
-                         llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS);
-    bool close = bitAnd(mapTypeBits,
-                        llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE);
-    bool present = bitAnd(
-        mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRESENT);
-
-    bool to =
-        bitAnd(mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO);
-    bool from =
-        bitAnd(mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM);
-    bool del = bitAnd(mapTypeBits,
-                      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE);
-
-    std::string typeModStr, typeStr;
-    llvm::raw_string_ostream typeMod(typeModStr), type(typeStr);
-
-    if (always)
-      typeMod << "always, ";
-    if (close)
-      typeMod << "close, ";
-    if (present)
-      typeMod << "present, ";
-
-    if (to)
-      type << "to";
-    if (from)
-      type << "from";
-    if (del)
-      type << "delete";
-    if (type.str().empty())
-      type << (isa<ExitDataOp>(op) ? "release" : "alloc");
-
-    p << '(' << typeMod.str() << type.str() << " -> " << mapOp << " : "
-      << mapOp.getType() << ')';
-    if (i + 1 < e)
+  // handling of always, close, present placed at the beginning of the string
+  // to aid readability
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS))
+    mapTypeStrs.push_back("always");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE))
+    mapTypeStrs.push_back("close");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRESENT))
+    mapTypeStrs.push_back("present");
+
+  // special handling of to/from/tofrom/delete and release/alloc, release +
+  // alloc are the abscense of one of the other flags, whereas tofrom requires
+  // both the to and from flag to be set.
+  bool to = mapTypeToBitFlag(mapTypeBits,
+                             llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO);
+  bool from = mapTypeToBitFlag(
+      mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM);
+  if (to && from) {
+    emitAllocRelease = false;
+    mapTypeStrs.push_back("tofrom");
+  } else if (from) {
+    emitAllocRelease = false;
+    mapTypeStrs.push_back("from");
+  } else if (to) {
+    emitAllocRelease = false;
+    mapTypeStrs.push_back("to");
+  }
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE)) {
+    emitAllocRelease = false;
+    mapTypeStrs.push_back("delete");
+  }
+  if (emitAllocRelease)
+    mapTypeStrs.push_back("exit_release_or_enter_alloc");
+
+  // handling of non-specification runtime related flags
+  if (mapTypeToBitFlag(
+          mapTypeBits,
+          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ))
+    mapTypeStrs.push_back("ptr_and_obj");
+  if (mapTypeToBitFlag(
+          mapTypeBits,
+          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM))
+    mapTypeStrs.push_back("target_param");
+  if (mapTypeToBitFlag(
+          mapTypeBits,
+          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_RETURN_PARAM))
+    mapTypeStrs.push_back("return_param");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRIVATE))
+    mapTypeStrs.push_back("private");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL))
+    mapTypeStrs.push_back("literal");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT))
+    mapTypeStrs.push_back("implicit");
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_OMPX_HOLD))
+    mapTypeStrs.push_back("ompx_hold");
+  if (mapTypeToBitFlag(
+          mapTypeBits,
+          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NON_CONTIG))
+    mapTypeStrs.push_back("non_contig");
+
+  if (mapTypeToBitFlag(mapTypeBits,
+                       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF))
+    mapTypeStrs.push_back("member_of");
+
+  for (unsigned int i = 0; i < mapTypeStrs.size(); ++i) {
+    p << mapTypeStrs[i];
+    if (i + 1 < mapTypeStrs.size()) {
       p << ", ";
     }
   }
-
-static LogicalResult verifyMapClause(Operation *op, OperandRange map_operands,
-                                     std::optional<ArrayAttr> map_types) {
-  // Helper function to get bitwise AND of `value` and 'flag'
-  auto bitAnd = [](int64_t value,
-                   llvm::omp::OpenMPOffloadMappingFlags flag) -> bool {
-    return value &
-           static_cast<
-               std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-               flag);
-  };
-  if (!map_types) {
-    if (!map_operands.empty())
-      return emitError(op->getLoc(), "missing mapTypes");
-    else
-      return success();
-  }
 
-  if (map_operands.empty() && !map_types->empty())
-    return emitError(op->getLoc(), "missing mapOperands");
-
-  if (map_types->empty() && !map_operands.empty())
-    return emitError(op->getLoc(), "missing mapTypes");
-
-  if (map_operands.size() != map_types->size())
-    return emitError(op->getLoc(),
-                     "mismatch in number of mapOperands and mapTypes");
+static void printCaptureType(OpAsmPrinter &p, Operation *op,
+                             VariableCaptureKindAttr mapCaptureType) {
+  std::string typeCapStr;
+  llvm::raw_string_ostream typeCap(typeCapStr);
+  if (mapCaptureType.getValue() == mlir::omp::VariableCaptureKind::ByRef)
+    typeCap << "ByRef";
+  if (mapCaptureType.getValue() == mlir::omp::VariableCaptureKind::ByCopy)
+    typeCap << "ByCopy";
+  if (mapCaptureType.getValue() == mlir::omp::VariableCaptureKind::VLAType)
+    typeCap << "VLAType";
+  if (mapCaptureType.getValue() == mlir::omp::VariableCaptureKind::This)
+    typeCap << "This";
+  p << typeCap.str();
+}
+
+static ParseResult parseCaptureType(OpAsmParser &parser,
+                                    VariableCaptureKindAttr &mapCapture) {
+  StringRef mapCaptureKey;
+  if (parser.parseKeyword(&mapCaptureKey))
+    return failure();
+
+  if (mapCaptureKey == "This")
+    mapCapture = mlir::omp::VariableCaptureKindAttr::get(
+        parser.getContext(), mlir::omp::VariableCaptureKind::This);
+  if (mapCaptureKey == "ByRef")
+    mapCapture = mlir::omp::VariableCaptureKindAttr::get(
+        parser.getContext(), mlir::omp::VariableCaptureKind::ByRef);
+  if (mapCaptureKey == "ByCopy")
+    mapCapture = mlir::omp::VariableCaptureKindAttr::get(
+        parser.getContext(), mlir::omp::VariableCaptureKind::ByCopy);
+  if (mapCaptureKey == "VLAType")
+    mapCapture = mlir::omp::VariableCaptureKindAttr::get(
+        parser.getContext(), mlir::omp::VariableCaptureKind::VLAType);
 
-  for (const auto &mapTypeOp : *map_types) {
-    int64_t mapTypeBits = 0x00;
-
-    if (!llvm::isa<mlir::IntegerAttr>(mapTypeOp))
-      return failure();
-
-    mapTypeBits = llvm::cast<mlir::IntegerAttr>(mapTypeOp).getInt();
+  return success();
+}
 
-    bool to =
-        bitAnd(mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO);
-    bool from =
-        bitAnd(mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM);
-    bool del = bitAnd(mapTypeBits,
-                      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE);
+static LogicalResult verifyMapClause(Operation *op, OperandRange mapOperands) {
+
+  for (auto mapOp : mapOperands) {
+    if (!mapOp.getDefiningOp())
+      emitError(op->getLoc(), "missing map operation");
+
+    if (auto mapEntryOp =
+            mlir::dyn_cast<mlir::omp::MapEntryOp>(mapOp.getDefiningOp())) {
+
+      if (!mapEntryOp.getMapType().has_value())
+        emitError(op->getLoc(), "missing map type for map operand");
+
+      if (!mapEntryOp.getMapCaptureType().has_value())
+        emitError(op->getLoc(), "missing map capture type for map operand");
+
+      uint64_t mapTypeBits = mapEntryOp.getMapType().value();
+
+      bool to = mapTypeToBitFlag(
+          mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO);
+      bool from = mapTypeToBitFlag(
+          mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM);
+      bool del = mapTypeToBitFlag(
+          mapTypeBits, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE);
 
       if ((isa<DataOp>(op) || isa<TargetOp>(op)) && del)
         return emitError(op->getLoc(),
                          "to, from, tofrom and alloc map types are permitted");
+
       if (isa<EnterDataOp>(op) && (from || del))
         return emitError(op->getLoc(), "to and alloc map types are permitted");
+
       if (isa<ExitDataOp>(op) && to)
         return emitError(op->getLoc(),
                          "from, release and delete map types are permitted");
+    } else {
+      emitError(op->getLoc(), "map argument is not a map entry operation");
+    }
   }
 
   return success();
 }
 
 LogicalResult DataOp::verify() {
   if (getMapOperands().empty() && getUseDevicePtr().empty() &&
       getUseDeviceAddr().empty()) {
     return ::emitError(this->getLoc(), "At least one of map, useDevicePtr, or "
                                        "useDeviceAddr operand must be present");
   }
-  return verifyMapClause(*this, getMapOperands(), getMapTypes());
+  return verifyMapClause(*this, getMapOperands());
 }
 
 LogicalResult EnterDataOp::verify() {
-  return verifyMapClause(*this, getMapOperands(), getMapTypes());
+  return verifyMapClause(*this, getMapOperands());
 }
 
 LogicalResult ExitDataOp::verify() {
-  return verifyMapClause(*this, getMapOperands(), getMapTypes());
+  return verifyMapClause(*this, getMapOperands());
 }
 
 LogicalResult TargetOp::verify() {
-  return verifyMapClause(*this, getMapOperands(), getMapTypes());
+  return verifyMapClause(*this, getMapOperands());
 }
 
 //===----------------------------------------------------------------------===//
 // ParallelOp
 //===----------------------------------------------------------------------===//
 
 void ParallelOp::build(OpBuilder &builder, OperationState &state,
                        ArrayRef<NamedAttribute> attributes) {
if ((cct == ClauseCancellationConstructType::Sections) &&
       !(isa<SectionsOp>(parentOp) || isa<SectionOp>(parentOp))) {
     return emitOpError() << "cancellation point sections must appear "
                          << "inside a sections region";
   }
   // TODO : Add more when we support taskgroup.
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// DataBoundsOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult DataBoundsOp::verify() {
+  auto extent = getExtent();
+  auto upperbound = getUpperBound();
+  if (!extent && !upperbound)
+    return emitError("expected extent or upperbound.");
+  return success();
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"
+
+#define GET_TYPEDEF_CLASSES
+#include "mlir/Dialect/OpenMP/OpenMPOpsTypes.cpp.inc"
 No newline at end of file

mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

func.func @task_depend(%arg0: !llvm.ptr<i32>) {
   }
   return
 }
 
 // -----
 
 // CHECK-LABEL: @_QPomp_target_data
 // CHECK: (%[[ARG0:.*]]: !llvm.ptr<i32>, %[[ARG1:.*]]: !llvm.ptr<i32>, %[[ARG2:.*]]: !llvm.ptr<i32>, %[[ARG3:.*]]: !llvm.ptr<i32>)
-// CHECK:         omp.target_enter_data   map((to -> %[[ARG0]] : !llvm.ptr<i32>), (to -> %[[ARG1]] : !llvm.ptr<i32>), (always, alloc -> %[[ARG2]] : !llvm.ptr<i32>))
-// CHECK:         omp.target_exit_data   map((from -> %[[ARG0]] : !llvm.ptr<i32>), (from -> %[[ARG1]] : !llvm.ptr<i32>), (release -> %[[ARG2]] : !llvm.ptr<i32>), (always, delete -> %[[ARG3]] : !llvm.ptr<i32>))
-// CHECK:         llvm.return
+// CHECK: %[[MAP0:.*]] = omp.map_entry var_ptr(%[[ARG0]] : !llvm.ptr<i32>)   map_clauses(to, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: %[[MAP1:.*]] = omp.map_entry var_ptr(%[[ARG1]] : !llvm.ptr<i32>)   map_clauses(to, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: %[[MAP2:.*]] = omp.map_entry var_ptr(%[[ARG2]] : !llvm.ptr<i32>)   map_clauses(always, exit_release_or_enter_alloc, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: omp.target_enter_data map_entries(%[[MAP0]], %[[MAP1]], %[[MAP2]] : !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>)
+// CHECK: %[[MAP3:.*]] = omp.map_entry var_ptr(%[[ARG0]] : !llvm.ptr<i32>)   map_clauses(from, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: %[[MAP4:.*]] = omp.map_entry var_ptr(%[[ARG1]] : !llvm.ptr<i32>)   map_clauses(from, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: %[[MAP5:.*]] = omp.map_entry var_ptr(%[[ARG2]] : !llvm.ptr<i32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: %[[MAP6:.*]] = omp.map_entry var_ptr(%[[ARG3]] : !llvm.ptr<i32>)   map_clauses(always, delete, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+// CHECK: omp.target_exit_data map_entries(%[[MAP3]], %[[MAP4]], %[[MAP5]], %[[MAP6]] : !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>)
 
 llvm.func @_QPomp_target_data(%a : !llvm.ptr<i32>, %b : !llvm.ptr<i32>, %c : !llvm.ptr<i32>, %d : !llvm.ptr<i32>) {
-  omp.target_enter_data   map((to -> %a : !llvm.ptr<i32>), (to -> %b : !llvm.ptr<i32>), (always, alloc -> %c : !llvm.ptr<i32>))
-  omp.target_exit_data   map((from -> %a : !llvm.ptr<i32>), (from -> %b : !llvm.ptr<i32>), (release -> %c : !llvm.ptr<i32>), (always, delete -> %d : !llvm.ptr<i32>))
+  %0 = omp.map_entry var_ptr(%a : !llvm.ptr<i32>)   map_clauses(to, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  %1 = omp.map_entry var_ptr(%b : !llvm.ptr<i32>)   map_clauses(to, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  %2 = omp.map_entry var_ptr(%c : !llvm.ptr<i32>)   map_clauses(always, exit_release_or_enter_alloc, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  omp.target_enter_data map_entries(%0, %1, %2 : !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>) {}
+  %3 = omp.map_entry var_ptr(%a : !llvm.ptr<i32>)   map_clauses(from, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  %4 = omp.map_entry var_ptr(%b : !llvm.ptr<i32>)   map_clauses(from, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  %5 = omp.map_entry var_ptr(%c : !llvm.ptr<i32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  %6 = omp.map_entry var_ptr(%d : !llvm.ptr<i32>)   map_clauses(always, delete, target_param) capture(ByRef) -> !llvm.ptr<i32> {name = ""}
+  omp.target_exit_data map_entries(%3, %4, %5, %6 : !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>, !llvm.ptr<i32>) {}
   llvm.return
 }
 
 // -----
 
 // CHECK-LABEL: @_QPomp_target_data_region
 // CHECK: (%[[ARG0:.*]]: !llvm.ptr<array<1024 x i32>>, %[[ARG1:.*]]: !llvm.ptr<i32>) {
-// CHECK:   omp.target_data   map((tofrom -> %[[ARG0]] : !llvm.ptr<array<1024 x i32>>)) {
+// CHECK: %[[MAP_0:.*]] = omp.map_entry var_ptr(%[[ARG0]] : !llvm.ptr<array<1024 x i32>>)  map_clauses(tofrom, target_param) capture(ByRef) -> !llvm.ptr<array<1024 x i32>> {name = ""}
+// CHECK: omp.target_data map_entries(%[[MAP_0]] : !llvm.ptr<array<1024 x i32>>) {
 // CHECK:           %[[VAL_1:.*]] = llvm.mlir.constant(10 : i32) : i32
 // CHECK:           llvm.store %[[VAL_1]], %[[ARG1]] : !llvm.ptr<i32>
 // CHECK:           omp.terminator
 // CHECK:         }
 // CHECK:         llvm.return
 
 llvm.func @_QPomp_target_data_region(%a : !llvm.ptr<array<1024 x i32>>, %i : !llvm.ptr<i32>) {
-  omp.target_data   map((tofrom -> %a : !llvm.ptr<array<1024 x i32>>)) {
-    %1 = llvm.mlir.constant(10 : i32) : i32
-    llvm.store %1, %i : !llvm.ptr<i32>
+  %1 = omp.map_entry var_ptr(%a : !llvm.ptr<array<1024 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) -> !llvm.ptr<array<1024 x i32>> {name = ""}
+  omp.target_data map_entries(%1 : !llvm.ptr<array<1024 x i32>>) {
+    %2 = llvm.mlir.constant(10 : i32) : i32
+    llvm.store %2, %i : !llvm.ptr<i32>
     omp.terminator
   }
   llvm.return
 }
 
 // -----
 
 // CHECK-LABEL:   llvm.func @_QPomp_target(
 // CHECK:                             %[[ARG_0:.*]]: !llvm.ptr<array<1024 x i32>>,
 // CHECK:                             %[[ARG_1:.*]]: !llvm.ptr<i32>) {
 // CHECK:           %[[VAL_0:.*]] = llvm.mlir.constant(64 : i32) : i32
-// CHECK:           omp.target   thread_limit(%[[VAL_0]] : i32) map((tofrom -> %[[ARG_0]] : !llvm.ptr<array<1024 x i32>>)) {
+// CHECK:           %[[MAP:.*]] = omp.map_entry var_ptr(%[[ARG_0]] : !llvm.ptr<array<1024 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) -> !llvm.ptr<array<1024 x i32>> {name = ""}
+// CHECK:           omp.target   thread_limit(%[[VAL_0]] : i32) map_entries(%[[MAP]] : !llvm.ptr<array<1024 x i32>>) {  
 // CHECK:             %[[VAL_1:.*]] = llvm.mlir.constant(10 : i32) : i32
 // CHECK:             llvm.store %[[VAL_1]], %[[ARG_1]] : !llvm.ptr<i32>
 // CHECK:             omp.terminator
 // CHECK:           }
 // CHECK:           llvm.return
 // CHECK:         }
 
 llvm.func @_QPomp_target(%a : !llvm.ptr<array<1024 x i32>>, %i : !llvm.ptr<i32>) {
   %0 = llvm.mlir.constant(64 : i32) : i32
-  omp.target   thread_limit(%0 : i32) map((tofrom -> %a : !llvm.ptr<array<1024 x i32>>)) {
-    %1 = llvm.mlir.constant(10 : i32) : i32
-    llvm.store %1, %i : !llvm.ptr<i32>
+  %1 = omp.map_entry var_ptr(%a : !llvm.ptr<array<1024 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) -> !llvm.ptr<array<1024 x i32>> {name = ""}
+  omp.target   thread_limit(%0 : i32) map_entries(%1 : !llvm.ptr<array<1024 x i32>>) {
+    %2 = llvm.mlir.constant(10 : i32) : i32
+    llvm.store %2, %i : !llvm.ptr<i32>
     omp.terminator
   }
   llvm.return
 }
 
 // -----
 
 // CHECK-LABEL: @_QPsb
// CHECK: omp.terminator
     omp.terminator
   }
   llvm.return
 }
 // CHECK: @[[CALL_FUNC]]
 llvm.func @_QFPdo_work(%arg0: !llvm.ptr<i32> {fir.bindc_name = "i"}) {
   llvm.return
 }
+
+// -----
+
+// CHECK-LABEL:   llvm.func @_QPtarget_map_with_bounds(
+// CHECK:                             %[[ARG_0:.*]]: !llvm.ptr<i32>,
+// CHECK:                             %[[ARG_1:.*]]: !llvm.ptr<array<10 x i32>>,
+// CHECK:                             %[[ARG_2:.*]]: !llvm.ptr<array<10 x i32>>) {
+// CHECK: %[[C_01:.*]] = llvm.mlir.constant(4 : index) : i64
+// CHECK: %[[C_02:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[C_03:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[C_04:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[BOUNDS0:.*]] = omp.bounds   lower_bound(%[[C_02]] : i64) upper_bound(%[[C_01]] : i64) stride(%[[C_04]] : i64) start_idx(%[[C_04]] : i64)
+// CHECK: %[[MAP0:.*]] = omp.map_entry var_ptr(%[[ARG_1]] : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) bounds(%[[BOUNDS0]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+// CHECK: %[[C_11:.*]] = llvm.mlir.constant(4 : index) : i64
+// CHECK: %[[C_12:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[C_13:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[C_14:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[BOUNDS1:.*]] = omp.bounds   lower_bound(%[[C_12]] : i64) upper_bound(%[[C_11]] : i64) stride(%[[C_14]] : i64) start_idx(%[[C_14]] : i64)
+// CHECK: %[[MAP1:.*]] = omp.map_entry var_ptr(%[[ARG_2]] : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) bounds(%[[BOUNDS1]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+// CHECK: omp.target   map_entries(%[[MAP0]], %[[MAP1]] : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>) {
+// CHECK:   omp.terminator
+// CHECK: }
+// CHECK: llvm.return
+// CHECK:}
+
+llvm.func @_QPtarget_map_with_bounds(%arg0: !llvm.ptr<i32>, %arg1: !llvm.ptr<array<10 x i32>>, %arg2: !llvm.ptr<array<10 x i32>>) {
+  %0 = llvm.mlir.constant(4 : index) : i64
+  %1 = llvm.mlir.constant(1 : index) : i64
+  %2 = llvm.mlir.constant(1 : index) : i64
+  %3 = llvm.mlir.constant(1 : index) : i64
+  %4 = omp.bounds   lower_bound(%1 : i64) upper_bound(%0 : i64) stride(%3 : i64) start_idx(%3 : i64)
+  %5 = omp.map_entry var_ptr(%arg1 : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) bounds(%4) -> !llvm.ptr<array<10 x i32>> {name = ""}
+  %6 = llvm.mlir.constant(4 : index) : i64
+  %7 = llvm.mlir.constant(1 : index) : i64
+  %8 = llvm.mlir.constant(1 : index) : i64
+  %9 = llvm.mlir.constant(1 : index) : i64
+  %10 = omp.bounds   lower_bound(%7 : i64) upper_bound(%6 : i64) stride(%9 : i64) start_idx(%9 : i64)
+  %11 = omp.map_entry var_ptr(%arg2 : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param) capture(ByRef) bounds(%10) -> !llvm.ptr<array<10 x i32>> {name = ""}
+  omp.target   map_entries(%5, %11 : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>) {
+    omp.terminator
+  }
+  llvm.return
+}

mlir/test/Dialect/OpenMP/invalid.mlir

func.func @omp_threadprivate() {
   // expected-error @below {{op failed to verify that all of {sym_addr, tls_addr} have same type}}
   %2 = omp.threadprivate %1 : !llvm.ptr<i32> -> memref<i32>
   return
 }
 
 // -----
 
 func.func @omp_target(%map1: memref<?xi32>) {
+  %mapv = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(delete, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
   // expected-error @below {{to, from, tofrom and alloc map types are permitted}}
-  omp.target map((delete -> %map1 : memref<?xi32>)){}
+  omp.target map_entries(%mapv : memref<?xi32>){}
   return
 }
 
 // -----
 
 func.func @omp_target_data(%map1: memref<?xi32>) {
+  %mapv = omp.map_entry var_ptr(%map1 : memref<?xi32>)  map_clauses(delete, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
   // expected-error @below {{to, from, tofrom and alloc map types are permitted}}
-  omp.target_data map((delete -> %map1 : memref<?xi32>)){}
+  omp.target_data map_entries(%mapv : memref<?xi32>){}
   return
 }
 
 // -----
 
 func.func @omp_target_data() {
   // expected-error @below {{At least one of map, useDevicePtr, or useDeviceAddr operand must be present}}
   omp.target_data {}
   return
 }
 
 // -----
 
 func.func @omp_target_enter_data(%map1: memref<?xi32>) {
+  %mapv = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(from, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
   // expected-error @below {{to and alloc map types are permitted}}
-  omp.target_enter_data map((from -> %map1 : memref<?xi32>)){}
+  omp.target_enter_data map_entries(%mapv : memref<?xi32>){}
   return
 }
 
 // -----
 
 func.func @omp_target_exit_data(%map1: memref<?xi32>) {
+  %mapv = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(to, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
   // expected-error @below {{from, release and delete map types are permitted}}
-  omp.target_exit_data map((to -> %map1 : memref<?xi32>)){}
+  omp.target_exit_data map_entries(%mapv : memref<?xi32>){}
   return
 }
 
 llvm.mlir.global internal @_QFsubEx() : i32

mlir/test/Dialect/OpenMP/ops.mlir

func.func @omp_target(%if_cond : i1, %device : si32,  %num_threads : i32, %map1: memref<?xi32>, %map2: memref<?xi32>) -> () {
     // Test with optional operands; if_expr, device, thread_limit, private, firstprivate and nowait.
     // CHECK: omp.target if({{.*}}) device({{.*}}) thread_limit({{.*}}) nowait
     "omp.target"(%if_cond, %device, %num_threads) ({
        // CHECK: omp.terminator
        omp.terminator
     }) {nowait, operandSegmentSizes = array<i32: 1,1,1,0>} : ( i1, si32, i32 ) -> ()
 
     // Test with optional map clause.
-    // CHECK: omp.target map((tofrom -> %{{.*}} : memref<?xi32>), (alloc -> %{{.*}} : memref<?xi32>)) {
-    omp.target map((tofrom -> %map1 : memref<?xi32>), (alloc -> %map2 : memref<?xi32>)){}
-
-    // CHECK: omp.target map((to -> %{{.*}} : memref<?xi32>), (always, from -> %{{.*}} : memref<?xi32>)) {
-    omp.target map((to -> %map1 : memref<?xi32>), (always, from -> %map2 : memref<?xi32>)){}
-
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_1:.*]] : memref<?xi32>)   map_clauses(tofrom, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: %[[MAP_B:.*]] = omp.map_entry var_ptr(%[[VAL_2:.*]] : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target map_entries(%[[MAP_A]], %[[MAP_B]] : memref<?xi32>, memref<?xi32>) {
+    %mapv1 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(tofrom, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    %mapv2 = omp.map_entry var_ptr(%map2 : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target map_entries(%mapv1, %mapv2 : memref<?xi32>, memref<?xi32>){}
+    // CHECK: %[[MAP_C:.*]] = omp.map_entry var_ptr(%[[VAL_1:.*]] : memref<?xi32>)   map_clauses(to, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: %[[MAP_D:.*]] = omp.map_entry var_ptr(%[[VAL_2:.*]] : memref<?xi32>)   map_clauses(always, from, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target map_entries(%[[MAP_C]], %[[MAP_D]] : memref<?xi32>, memref<?xi32>) {
+    %mapv3 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(to, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    %mapv4 = omp.map_entry var_ptr(%map2 : memref<?xi32>)   map_clauses(always, from, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target map_entries(%mapv3, %mapv4 : memref<?xi32>, memref<?xi32>) {}
     // CHECK: omp.barrier
     omp.barrier
 
     return
 }
 
 // CHECK-LABEL: omp_target_data
 func.func @omp_target_data (%if_cond : i1, %device : si32, %device_ptr: memref<i32>, %device_addr: memref<?xi32>, %map1: memref<?xi32>, %map2: memref<?xi32>) -> () {
-    // CHECK: omp.target_data if(%[[VAL_0:.*]] : i1) device(%[[VAL_1:.*]] : si32) map((always, from -> %[[VAL_2:.*]] : memref<?xi32>))
-    omp.target_data if(%if_cond : i1) device(%device : si32) map((always, from -> %map1 : memref<?xi32>)){}
-
-    // CHECK: omp.target_data map((close, present, to -> %[[VAL_2:.*]] : memref<?xi32>)) use_device_ptr(%[[VAL_3:.*]] : memref<i32>) use_device_addr(%[[VAL_4:.*]] : memref<?xi32>)
-    omp.target_data map((close, present, to -> %map1 : memref<?xi32>)) use_device_ptr(%device_ptr : memref<i32>) use_device_addr(%device_addr : memref<?xi32>) {}
-
-    // CHECK: omp.target_data map((tofrom -> %[[VAL_2]] : memref<?xi32>), (alloc -> %[[VAL_5:.*]] : memref<?xi32>))
-    omp.target_data map((tofrom -> %map1 : memref<?xi32>), (alloc -> %map2 : memref<?xi32>)){}
-
-    // CHECK: omp.target_enter_data if(%[[VAL_0]] : i1) device(%[[VAL_1]] : si32) nowait map((alloc -> %[[VAL_2]] : memref<?xi32>))
-    omp.target_enter_data if(%if_cond : i1) device(%device : si32) nowait map((alloc -> %map1 : memref<?xi32>))
-
-    // CHECK: omp.target_exit_data if(%[[VAL_0]] : i1) device(%[[VAL_1]] : si32) nowait map((release -> %[[VAL_5]] : memref<?xi32>))
-    omp.target_exit_data if(%if_cond : i1) device(%device : si32) nowait map((release -> %map2 : memref<?xi32>))
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_2:.*]] : memref<?xi32>)   map_clauses(always, from, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target_data if(%[[VAL_0:.*]] : i1) device(%[[VAL_1:.*]] : si32) map_entries(%[[MAP_A]] : memref<?xi32>)
+    %mapv1 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(always, from, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target_data if(%if_cond : i1) device(%device : si32) map_entries(%mapv1 : memref<?xi32>){}
+
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_2:.*]] : memref<?xi32>)   map_clauses(close, present, to, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target_data map_entries(%[[MAP_A]] : memref<?xi32>) use_device_ptr(%[[VAL_3:.*]] : memref<i32>) use_device_addr(%[[VAL_4:.*]] : memref<?xi32>)
+    %mapv2 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(close, present, to, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target_data map_entries(%mapv2 : memref<?xi32>) use_device_ptr(%device_ptr : memref<i32>) use_device_addr(%device_addr : memref<?xi32>) {}
+
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_1:.*]] : memref<?xi32>)   map_clauses(tofrom, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: %[[MAP_B:.*]] = omp.map_entry var_ptr(%[[VAL_2:.*]] : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target_data map_entries(%[[MAP_A]], %[[MAP_B]] : memref<?xi32>, memref<?xi32>)
+    %mapv3 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(tofrom, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    %mapv4 = omp.map_entry var_ptr(%map2 : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target_data map_entries(%mapv3, %mapv4 : memref<?xi32>, memref<?xi32>) {}
+
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_3:.*]] : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target_enter_data if(%[[VAL_0:.*]] : i1) device(%[[VAL_1:.*]] : si32) nowait map_entries(%[[MAP_A]] : memref<?xi32>) 
+    %mapv5 = omp.map_entry var_ptr(%map1 : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target_enter_data if(%if_cond : i1) device(%device : si32) nowait map_entries(%mapv5 : memref<?xi32>) 
+
+    // CHECK: %[[MAP_A:.*]] = omp.map_entry var_ptr(%[[VAL_3:.*]] : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    // CHECK: omp.target_exit_data if(%[[VAL_0:.*]] : i1) device(%[[VAL_1:.*]] : si32) nowait map_entries(%[[MAP_A]] : memref<?xi32>)
+    %mapv6 = omp.map_entry var_ptr(%map2 : memref<?xi32>)   map_clauses(exit_release_or_enter_alloc, target_param) capture(ByRef) -> memref<?xi32> {name = ""}
+    omp.target_exit_data if(%if_cond : i1) device(%device : si32) nowait map_entries(%mapv6 : memref<?xi32>) 
     
     return
 }
 
 // CHECK-LABEL: omp_target_pretty
 func.func @omp_target_pretty(%if_cond : i1, %device : si32,  %num_threads : i32) -> () {
     // CHECK: omp.target if({{.*}}) device({{.*}})
     omp.target if(%if_cond) device(%device : si32) {
       omp.terminator
^bb1(%arg0: f32, %arg1: f32):
   omp.yield (%1 : f32)
 }
 atomic {
 ^bb2(%arg2: !llvm.ptr, %arg3: !llvm.ptr):
   %2 = llvm.load %arg3 : !llvm.ptr -> f32
   llvm.atomicrmw fadd %arg2, %2 monotonic : !llvm.ptr, f32
   omp.yield
 }
+
+// CHECK-LABEL: omp_targets_with_map_bounds
+// CHECK-SAME: (%[[ARG0:.*]]: !llvm.ptr<array<10 x i32>>, %[[ARG1:.*]]: !llvm.ptr<array<10 x i32>>)
+func.func @omp_targets_with_map_bounds(%arg0: !llvm.ptr<array<10 x i32>>, %arg1: !llvm.ptr<array<10 x i32>>) -> () {
+  // CHECK: %[[C_00:.*]] = llvm.mlir.constant(4 : index) : i64
+  // CHECK: %[[C_01:.*]] = llvm.mlir.constant(1 : index) : i64
+  // CHECK: %[[C_02:.*]] = llvm.mlir.constant(1 : index) : i64
+  // CHECK: %[[C_03:.*]] = llvm.mlir.constant(1 : index) : i64
+  // CHECK: %[[BOUNDS0:.*]] = omp.bounds   lower_bound(%[[C_01]] : i64) upper_bound(%[[C_00]] : i64) stride(%[[C_02]] : i64) start_idx(%[[C_03]] : i64)
+  // CHECK: %[[MAP0:.*]] = omp.map_entry var_ptr(%[[ARG0]] : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param, non_contig, member_of) capture(ByRef) bounds(%[[BOUNDS0]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    %0 = llvm.mlir.constant(4 : index) : i64
+    %1 = llvm.mlir.constant(1 : index) : i64
+    %2 = llvm.mlir.constant(1 : index) : i64
+    %3 = llvm.mlir.constant(1 : index) : i64
+    %4 = omp.bounds   lower_bound(%1 : i64) upper_bound(%0 : i64) stride(%2 : i64) start_idx(%3 : i64)
+    
+    %mapv1 = omp.map_entry var_ptr(%arg0 : !llvm.ptr<array<10 x i32>>)   map_clauses(tofrom, target_param, non_contig, member_of) capture(ByRef) bounds(%4) -> !llvm.ptr<array<10 x i32>> {name = ""}
+  // CHECK: %[[C_10:.*]] = llvm.mlir.constant(9 : index) : i64
+  // CHECK: %[[C_11:.*]] = llvm.mlir.constant(1 : index) : i64
+  // CHECK: %[[C_12:.*]] = llvm.mlir.constant(2 : index) : i64
+  // CHECK: %[[C_13:.*]] = llvm.mlir.constant(2 : index) : i64
+  // CHECK: %[[BOUNDS1:.*]] = omp.bounds   lower_bound(%[[C_11]] : i64) upper_bound(%[[C_10]] : i64) stride(%[[C_12]] : i64) start_idx(%[[C_13]] : i64)
+  // CHECK: %[[MAP1:.*]] = omp.map_entry var_ptr(%[[ARG1]] : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, target_param, implicit, ompx_hold) capture(ByCopy) bounds(%[[BOUNDS1]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    %6 = llvm.mlir.constant(9 : index) : i64
+    %7 = llvm.mlir.constant(1 : index) : i64
+    %8 = llvm.mlir.constant(2 : index) : i64
+    %9 = llvm.mlir.constant(2 : index) : i64
+    %10 = omp.bounds   lower_bound(%7 : i64) upper_bound(%6 : i64) stride(%8 : i64) start_idx(%9 : i64)
+    %mapv2 = omp.map_entry var_ptr(%arg1 : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, target_param, implicit, ompx_hold) capture(ByCopy) bounds(%10) -> !llvm.ptr<array<10 x i32>> {name = ""}
+
+    // CHECK: omp.target map_entries(%[[MAP0]], %[[MAP1]] : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>)
+    omp.target map_entries(%mapv1, %mapv2 : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>){}
+
+    // CHECK: omp.target_data map_entries(%[[MAP0]], %[[MAP1]] : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>)
+    omp.target_data map_entries(%mapv1, %mapv2 : !llvm.ptr<array<10 x i32>>, !llvm.ptr<array<10 x i32>>){}
+
+    // CHECK: %[[MAP2:.*]] = omp.map_entry var_ptr(%[[ARG0]] : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, target_param, private, literal) capture(VLAType) bounds(%[[BOUNDS0]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    // CHECK: omp.target_enter_data map_entries(%[[MAP2]] : !llvm.ptr<array<10 x i32>>)
+    %mapv3 = omp.map_entry var_ptr(%arg0 : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, target_param, private, literal) capture(VLAType) bounds(%4) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    omp.target_enter_data map_entries(%mapv3 : !llvm.ptr<array<10 x i32>>){}
+   
+    // CHECK: %[[MAP3:.*]] = omp.map_entry var_ptr(%[[ARG1]] : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, ptr_and_obj, target_param, return_param) capture(This) bounds(%[[BOUNDS1]]) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    // CHECK: omp.target_exit_data map_entries(%[[MAP3]] : !llvm.ptr<array<10 x i32>>)
+    %mapv4 = omp.map_entry var_ptr(%arg1 : !llvm.ptr<array<10 x i32>>)   map_clauses(exit_release_or_enter_alloc, ptr_and_obj, target_param, return_param) capture(This) bounds(%10) -> !llvm.ptr<array<10 x i32>> {name = ""}
+    omp.target_exit_data map_entries(%mapv4 : !llvm.ptr<array<10 x i32>>){}
+
+    return
+}