[MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[abidmalikwaterloo]

Summary

This includes a basic implementation for the OpenMP target data, exit data, and enter data constructs
operation.

[clementval]

Is this ready for review?

[abidmalikwaterloo]

In D105255#2852959, @clementval wrote:

Is this ready for review?

Yes.

[clementval]

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

253

Indent looks off.

270

Since map is a required clause on the Fortran side it would be better to have it directly for the first version of the op.

275

Indent.

287

Not sure if there is a guidelines for that but I think op should use snake_case. At least in the OMP dialect snake_case is used until now so you should follow it. (ex: num_threads on the parallel op).

290

Indent looks off in the description.

304

Since map is a required clause on the Fortran side it would be better to have it directly for the first version of the op.

311

I don't think it makes sense to have a region attached for standalone directives.

337

Since map is a required clause on the Fortran side it would be better to have it directly for the first version of the op.

344

I don't think exit data should have a region attached.

[abidmalikwaterloo]

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

270

map([[map-type-modifier[,]]map-type:]list)

Based on Thread discussion, the following segmentation of the map clause operands should work.

Optional<StringAttr>:$mapTypeModifier
Optional<StringAttr>:$mapType
Variadic<AnyType>:$mapOperands

acc exit data and enter data also follow the same pattern. Any suggestions?

[clementval]

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

270

Not sure it would do the trick since you can have multiple map clauses with different map type and map type modifier. If you model it like that you can have only one clause represented by operation.

[abidmalikwaterloo]

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

270

I intend to make it more in line with the OpenACC data environment; copy, copying, copyout. etc.

Variadic<AnyType>:$mapOperands
Variadic<AnyType>:$mapToOperands
Variadic<AnyType>:$mapFromOperands
Variadic<AnyType>:$mapToFromOperands
Variadic<AnyType>:$mapAllocOperands
Variadic<AnyType>:$mapReleaseOperands
Variadic<AnyType>:$mapDeleteOperands
Variadic<AnyType>:$mapTypeModifier

[clementval]

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

270

Also clause can take several map-type-modifier (map([ [map-type-modifier[,] [map-type-modifier[,] ...] map-type : ] locator-list)) so a single Optional<StringAttr> will not work. You might need a special type that encapsulate all the information.

[abidmalikwaterloo]

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

270

How about Variadic<AnyType>:$mapTypeModifier? or we need a separate operand for each map<X> Operands? I feel that we need a separate modifier operand for each map<X>Operands.

[clementval]

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

270

I guess a enum would fit better here. The problem is that the map-type-modifier is associated with a specific map clause so if we have the different operands as you mentioned before, how do we know which one is associated with the map-type-modifier ?

[abidmalikwaterloo]

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

270

We need to know about each modifier for each map clause
OptionalAttribute<MapTypeModifier>=$mapTypeModifierTo
OptionalAttribute<MapTypeModifier>=$mapTypeModifierFrom
OptionalAttribute<MapTypeModifier>=$mapTypeModifierToFrom
and so on for each type.
enum MapTypeModifier = [ always, close, mapper].
Does this make sense?

[clementval]

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

270

I don't think that's enough since you can have something like in the following example:

!$omp target enter data map(always, to: a) map(to:b)

In this case the two operands should end up in mapToOperands but with a different modifier.

Maybe a vector holding the modifier for each operands would work (VectorOf<MapTypeModifier>). With this you would be able to store the operands in the mapToOperands with a vector of size 2 that holds the modifier values ([always, none]). The vector size is of course equal to the total number of operands in you 8 operands variadic arguments.

[abidmalikwaterloo]

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

270

MLIR does not have a VectorOf operation class. We need to define one ( I think, not an MLIR expert). The quick solution to this is to define an operand for each map-type for each type modifier.

FOR "TO"
Variadic <AnyType> = $mapToAlways
Variadic <AnyType> = $mapToClose
Variadic <AnyType> = $mapToMapper
Variadic <AnyType> = $mapToNone

Similarly, we can define for each map-type { from, tofrom , alloc ,release ,delete}. The only caveat is that we will have 24 operands for the operation.

[clementval]

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

270

It has vectorof class. I took it from the mlir file directly.

[clementval]

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

270

You might need to add it as an attribute instead of an operand. I didn’t check but for aure the operand segment attribute is an vector of int.

[abidmalikwaterloo]

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

270

let arguments = (ins Optional<I1>:$if_expr,

Optional<AnyInteger>:$device,
Variadic<AnyType>:$use_device_ptr,
Variadic<AnyType>:$use_device_addr,
Variadic<AnyType>:$mapOperands,
Variadic<AnyType>:$mapToOperands,
Variadic<AnyType>:$mapFromOperands,
Variadic<AnyType>:$mapToFromOperands,
Variadic<AnyType>:$mapAllocOperands,
Variadic<AnyType>:$mapReleaseOperands,
Variadic<AnyType>:$mapDeleteOperands,
VectorOfLengthAndType<[7],[AnyType]>:$map_type_modifier_always,
VectorOfLengthAndType<[7],[AnyType]>:$map_type_modifier_close,
VectorOfLengthAndType<[7],[AnyType]>:$map_type_modifier_mapper);

I feel this should work. The vectors will keep track of modifier for each map type.

[clementval]

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

270

I think a single vector would be easier and you can encode the values of always, close and mapper (maybe in a uint). It is done like this in the runtime so you will have less processing during the translation. It will also accept new modifiers if newer version of the standard add some.

The vector should not be fixed in size. Its size should be equal to the number of operands. Like if you have 2 operands in $mapOperands and in $mapToOperands` the vector size is 3.

[abidmalikwaterloo]

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

270

The vector should not be fixed in size. Its size should be equal to the number of operands. Like if you have 2 operands in $mapOperands and ( You mean 1 here right???) in $mapToOperands` the vector size is 3.

We are assuming the order in which the map operands appear will define the operands in the vector map_type_modifier and the corresponding element will define the content (modifier defined by a unit). For example, consider the following:

map (always, to : v1, v2) map(to: a, b) map ( from:p)

we have five operands. The vector size is five. The vector representation [always, always, none, none, none]

[clementval]

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

270

Yeah that's what I mean. If you find a better way represent the information it's also fine but I thought this is concise and will be perfectly fitted for the translation later.

[abidmalikwaterloo]

Added map clase support to target data, enter data and exit data
Revised according to the reviewer's comments
Added a test case
Used snake_case format

[clementval]

Indent are off in many places. Can you update that?

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

256

Indent is still off here.

276

Indent still off.

287

Arguments are not aligned.

313

Indent is off.

394

Indent

[abidmalikwaterloo]

Revised for indentation as pinpointed by the reviewer.

[clementval]

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

342

Indent.

mlir/test/Dialect/OpenMP/ops.mlir

296

Exit data is not tested here.

[abidmalikwaterloo]

Added test example for exit data operation

[clementval]

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

287

There is still an indent problem here.

401

Is AnyType really what we want here? Should be an enum or an integer encoding the value. As a side not, we could also have a single $map_operands argument and have a second vector to encode the map-type but that just a thought.

[abidmalikwaterloo]

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

276

Need to wor

287

Need to check my editor

[abidmalikwaterloo]

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

401

• Based on the previous patches, I have the impression that the community prefers to make operations flexible and adaptable as things are fluid at a higher level. I would prefer enum

• I tried to make the operation in line with the acc. data operations. Single vector to hold map_type and operand to hold all map operands should work as in the case of map_type_modifier

[clementval]

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

401

• Based on the previous patches, I have the impression that the community prefers to make operations flexible and adaptable as things are fluid at a higher level. I would prefer enum

I think we try to be permissive where we have actual data but here is smith that we have to encode so I would make it more strict otherwise the translation will be complicated.

• I tried to make the operation in line with the acc. data operations. Single vector to hold map_type and operand to hold all map operands should work as in the case of map_type_modifier

Sure it is just a suggestion. It is totally fine to keep it the way you have it now. OpenACC has multiple clauses to represent the different map-types so that's why it was done like that.

[abidmalikwaterloo]

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

401

• Based on the previous patches, I have the impression that the community prefers to make operations flexible and adaptable as things are fluid at a higher level. I would prefer enum

I think we try to be permissive where we have actual data but here is smith that we have to encode so I would make it more strict otherwise the translation will be complicated.

• I tried to make the operation in line with the acc. data operations. Single vector to hold map_type and operand to hold all map operands should work as in the case of map_type_modifier

Sure it is just a suggestion. It is totally fine to keep it the way you have it now. OpenACC has multiple clauses to represent the different map-types so that's why it was done like that.

401

VectorOf<[AnyInteger]>:$map_type_modifier should be fine. Whatever the coding we adapt, it will be translated to an integer number for storing.

[abidmalikwaterloo]

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

401

Does this make sence?

[clementval]

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

266

Longer than 80

268

Longer than 80

401

Yeah it should work. I would go ahead and even restrict the type of integer you use here.

[abidmalikwaterloo]

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

401

<IntOrIndex> should be Okay

[abidmalikwaterloo]

Made changes to alignment
Made AnyType to AnyInteger for map_type_modifier

[clementval]

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

115

Extra line doesn't seem to fit the file format.

230

Extra line doesn't seem to fit the file format.

244

Extra line doesn't seem to fit the file format.

353

What's the purpose of the region on enter data?

[abidmalikwaterloo]

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

353

If I am correct, it is defining the region within which the scope exit.

[abidmalikwaterloo]

Remove space as mentioned by the reviewer

[clementval]

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

353

I'm pretty sure enter data does not need an attached region. It's a standalone directive so no other operation will be nested inside its region. target exit data does not have one so these two operations are very close and it should be reflected in their MLIR design.

[abidmalikwaterloo]

made changes in the data enter operation

[clementval]

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

263

There still lines exceeding the 80 characters limit. Can you double check the whole patch so we can approve it.

[abidmalikwaterloo]

Correct indentation for comments

[clementval]

I have kind of lost track on this. Is this up to date and ready to go?

[abidmalikwaterloo]

Will try to finalize this week.

[abidmalikwaterloo]

Sorry for the delay. Will work on it

[shraiysh]

Looks okay to me, please wait for Kiran/Valentin's comments. Also, will you be adding assembly format/verifier for this later?

The summary says map clause is not supported but there are map operands in the patch. Can we get these two in sync - we should either update summary or remove map-related operands.

Also, please rebase before further reviews.

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

267

please put variables in documentation within backticks(`). This is because when the documentation is rendered on mlir.llvm.org, $ and _ interact to give unexpected text if backticks are not used.

[abidmalikwaterloo]

In D105255#3473574, @shraiysh wrote:

Looks okay to me, please wait for Kiran/Valentin's comments. Also, will you be adding an assembly format/verifier for this later?

Yes

The summary says the map clause is not supported but there are map operands in the patch. Can we get these two in sync - we should either update the summary or remove map-related operands.

Also, please rebase before further reviews.

Will do

[clementval]

In D105255#3474527, @abidmalikwaterloo wrote:

In D105255#3473574, @shraiysh wrote:

Looks okay to me, please wait for Kiran/Valentin's comments. Also, will you be adding an assembly format/verifier for this later?

Yes

The summary says the map clause is not supported but there are map operands in the patch. Can we get these two in sync - we should either update the summary or remove map-related operands.

Also, please rebase before further reviews.

Will do

Do you or did you work on the rest of the lowering to OpenMPIRBuilder/LLVM IR? Might be good to have the broader pictures sometime before designing the operation.

[abidmalikwaterloo]

In D105255#3474533, @clementval wrote:

In D105255#3474527, @abidmalikwaterloo wrote:

In D105255#3473574, @shraiysh wrote:

Looks okay to me, please wait for Kiran/Valentin's comments. Also, will you be adding an assembly format/verifier for this later?

Yes

The summary says the map clause is not supported but there are map operands in the patch. Can we get these two in sync - we should either update the summary or remove map-related operands.

Also, please rebase before further reviews.

Will do

Do you or did you work on the rest of the lowering to OpenMPIRBuilder/LLVM IR? Might be good to have the broader pictures sometime before designing the operation.

I put the line of action that we had in mind ( based on the OpenACC translation) in the FLANG meeting with other team members. I will work on the same line. My thinking is to work on translation and see what we need to change in the operation API at this level. But, I will clean and update the patch based on the feedback as it is hanging around too long.

[abidmalikwaterloo]

In D105255#3474527, @abidmalikwaterloo wrote:

In D105255#3473574, @shraiysh wrote:

Looks okay to me, please wait for Kiran/Valentin's comments. Also, will you be adding an assembly format/verifier for this later?

Yes

The summary says the map clause is not supported but there are map operands in the patch. Can we get these two in sync - we should either update the summary or remove map-related operands.

Also, please rebase before further reviews.

Will do

I have updated the summary.

[abidmalikwaterloo]

I have updated the summary.

[abidmalikwaterloo]

Do we have any comments on this? I can submit the "assembly format" later. Need the operation definition for the next step : lowering to LLVM IR

[shraiysh]

Please rebase, I cannot download this patch because it is too old :(

[abidmalikwaterloo]

Rebase the patch and clean some formatting as needed

[shraiysh]

Minor nits about documentation.

LGTM given that verifier and assembly format will be added soon for each of these as a separate patch soon. Please wait for atleast one more +1.

The OpenMP IRBuilder work does not have to wait for this patch to merge. You can start working on that while keeping this patch as a dependency.

clang/lib/Testing/CMakeLists.txt

6 ↗

(On Diff #429235)

nit: Merge conflict. Please remove.

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

267

nit: Ping on this comment. The variable names have not been put in backticks and this is going to cause issues with autogenerated documentation.

273

Data mapping is very similar to data privatization. Would it be a good idea to push this to the frontends while we have not concretely decided on openmp dialect privatization/mapping? If you have an idea on how to implement mapping in OpenMP Dialect to LLVM IR maybe we can go ahead with this.

CC @kiranchandramohan for suggestions regarding privatization.

340

It is not clear which element in this list corresponds to which map type. It would be nice if we could either document that here, or have separate operands for each map type. (same for other two constructs too)

[abidmalikwaterloo]

clang/lib/Testing/CMakeLists.txt

33 ↗

(On Diff #429235)

Are these okay? In D105584 these were not

[shraiysh]

clang/lib/Testing/CMakeLists.txt

33 ↗

(On Diff #429235)

No, this is not okay. There is some issue with these patches and I don't know what it is, but these unrelated changes should not be seen here. Thank you for pointing this out :)

[abidmalikwaterloo]

clang/lib/Testing/CMakeLists.txt

6 ↗

(On Diff #429235)

removed

33 ↗

(On Diff #429235)

removed them manually

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

273

@kiranchandramohan @raghavendhra I would like to keep it like this! Any comments

340

I feel keeping it in vector makes more sense. I will document it here. As per OpenMP Specifications:
The map-type can be to, from, tofrom, or alloc.

397

will document map type here as wekk

[abidmalikwaterloo]

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

340

Correction: map_type_modifier can be "always, close, and mapper"

[abidmalikwaterloo]

Revised the patcg according to the reviews of the reviewers.
clean the code and add some comments

[raghavendhra]

@abidmalikwaterloo ninja check-mlir is failing with your most upto date patch. Can you please double check?

Thanks!

[abidmalikwaterloo]

Update ops.mlir test cases. llvm-lit is working and all tests are passing

[clementval]

Why not adding the assemblyFormat directly for these operations like your patch D105584?

[abidmalikwaterloo]

In D105255#3643428, @clementval wrote:

Why not adding the assembly format directly for these operations like your patch D105584?

OK, I will do it!

[abidmalikwaterloo]

Added assembly format
changed the type of mape_type_modifier

[abidmalikwaterloo]

I just updated the patch. Something went wrong. I used arc diff --update <patch> for uploading. However, it seems it is not comparing the last submitted patch.

[raghavendhra]

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with usage of diff --update <patch>? Can you please check?

[abidmalikwaterloo]

In D105255#3706602, @raghavendhra wrote:

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with the usage of diff --update <patch>? Can you please check?

@raghavendhra I tried to correct it but could not figure out the solution. Do you have any? I am not experienced with the phabricator framework. One solution is to submit another patch with all changes till today and abandon this one.

[raghavendhra]

In D105255#3707088, @abidmalikwaterloo wrote:

In D105255#3706602, @raghavendhra wrote:

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with the usage of diff --update <patch>? Can you please check?

@raghavendhra I tried to correct it but could not figure out the solution. Do you have any? I am not experienced with the phabricator framework. One solution is to submit another patch with all changes till today and abandon this one.

Sorry, I am new to phabricator as well. I use "arc diff" to upload my revisions for review after addressing review comments. If no one responds probably I am fine with new review and abandoning this one.

[clementval]

In D105255#3707495, @raghavendhra wrote:

In D105255#3707088, @abidmalikwaterloo wrote:

In D105255#3706602, @raghavendhra wrote:

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with the usage of diff --update <patch>? Can you please check?

@raghavendhra I tried to correct it but could not figure out the solution. Do you have any? I am not experienced with the phabricator framework. One solution is to submit another patch with all changes till today and abandon this one.

Sorry, I am new to phabricator as well. I use "arc diff" to upload my revisions for review after addressing review comments. If no one responds probably I am fine with new review and abandoning this one.

Did you try arc diff --update D105255 from your branch?

[abidmalikwaterloo]

In D105255#3707533, @clementval wrote:

In D105255#3707495, @raghavendhra wrote:

In D105255#3707088, @abidmalikwaterloo wrote:

In D105255#3706602, @raghavendhra wrote:

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with the usage of diff --update <patch>? Can you please check?

@raghavendhra I tried to correct it but could not figure out the solution. Do you have any? I am not experienced with the phabricator framework. One solution is to submit another patch with all changes till today and abandon this one.

Sorry, I am new to phabricator as well. I use "arc diff" to upload my revisions for review after addressing review comments. If no one responds probably I am fine with new review and abandoning this one.

Did you try arc diff --update D105255 from your branch?

yes, I tried this. Actually, I updated the patch. through this

[clementval]

In D105255#3708258, @abidmalikwaterloo wrote:

In D105255#3707533, @clementval wrote:

In D105255#3707495, @raghavendhra wrote:

In D105255#3707088, @abidmalikwaterloo wrote:

In D105255#3706602, @raghavendhra wrote:

@abidmalikwaterloo Can you please check your last patch pushed for review? https://buildkite.com/llvm-project/diff-checks/builds/117796 states it can not apply your patch something to do with the usage of diff --update <patch>? Can you please check?

@raghavendhra I tried to correct it but could not figure out the solution. Do you have any? I am not experienced with the phabricator framework. One solution is to submit another patch with all changes till today and abandon this one.

Sorry, I am new to phabricator as well. I use "arc diff" to upload my revisions for review after addressing review comments. If no one responds probably I am fine with new review and abandoning this one.

Did you try arc diff --update D105255 from your branch?

yes, I tried this. Actually, I updated the patch. through this

The problem is likely that the commit before the commit of this patch is not in tree,

The easiest way is to create a new branch from main and cherry-pick the commit you need for this patch on the top of it. Then you just use arc diff --update and it should work.

[abidmalikwaterloo]

[MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR

[clementval]

In D105255#3723865, @abidmalikwaterloo wrote:

[MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR

Looks like it's not the complete revision. There are almost no diffs anymore.

[abidmalikwaterloo]

In D105255#3723940, @clementval wrote:

In D105255#3723865, @abidmalikwaterloo wrote:

[MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR

Looks like it's not the complete revision. There are almost no diffs anymore.

My OS system crashed last week. I am working on it.

Abid_ReadME.txt

-Start date : May 4th, 2021
----------------------------
-
-How to install LLVM on Mac:
-1) There is a problem in installing with libcxxabi and libcxx. So, on my Mac, I  installed it with out these two projects. However, I need to understand why it was not building on Mac with the LLVM. It works on Shilie' MacBook
-
-2) The configuration that works for me:
-
-cmake -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=OFF ../llvm    -DLLVM_ENABLE_PROJECTS="mlir;clang;clang-tools-extra;lld;openmp" -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/MLIR_target/llvm-project/myInstall -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-ninja all for builfing
-ninja install for installing
-
-I am not sure about the -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14" flag as I played with it to overcome the problem with libcxxabi and libcxx. I will try to build that same with this flag.
-
-The build works without the aformentioned flag as well.
-
-Python3 is required
-if there is a problem on Mac about the python instalation check the following website
-https://mattbanderson.com/so-you-hosed-your-mac-os-python-install/
-
-
-May 5th, 2021
---------------
--DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-Python 3.* is needed.
-On my Mac, the default was Python 2.7. You can alias python = 'Python3' to make the python3 as your default.
-** Still need to check why libcxxabi and libcxx could not build on my Mac. However, we donot need them for my MLIR OpenMP dialect work
-
-May 7th, 2021
---------------
-
-When I cmake: The following error was raised:
-
-
-No rule to make target /usr/lib/libz.dylib', needed by `lib/libzip.5.3.dylib'.  Stop.
-
-The solution was found on the following link
-
-https://stackoverflow.com/questions/62719000/can-not-find-libz-dylib-when-build-libzip-by-homebrew-in-mac-os-10-16-big-sur
-
-I reinstall the libbz with brew and also use the DZLIB_LIBRARY_RELEASE flag to the library. It works and error was not there.
-
-
-Also an interesting link for building LLVM
-
-https://lists.llvm.org/pipermail/llvm-dev/2019-September/135333.html
-
-May 8th, 2021
-==============
-
-For LLVM_9 , the following configuration work
-
-cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=OFF ../llvm    -DLLVM_ENABLE_PROJECTS="mlir;clang;clang-tools-extra;lld;openmp" -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/LLVM_9/llvm-project/INSATLL -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-
-I am not pointing anything. for libz.dylib which is needed for LLVM_11.
-
-Question: Any reason why? Is it related to the LLVM version? Need to check with the LLVM community.
-
-
-
-May 17th, 2021
-===============
-
-Build the MLIR with
-
-
-cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=ON ../llvm    -DLLVM_ENABLE_PROJECTS=mlir -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/MLIR_target/llvm-project/INSTALL/ -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-
-and then
-
-cmake --build . --target check-mlir
-
-Now all tests are passing
-
-
-May 18th, 2021
-===============
-
-FYI,
-
-TableGen syntax sometimes can be obscure; reading the generated content can be a very helpful way to understand and debug issues. 
-To build mlir-tblgen, run cmake --build . --target mlir-tblgen in your build directory and find the mlir-tblgen binary in the bin/ subdirectory. 
-All the supported generators can be found via mlir-tblgen --help. For example, --gen-op-decls and --gen-op-defs as explained in Generated C++ code .

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

let description = [{
     operation.  These regions are not expected to return any value so the
     terminator takes no operands. The terminator op returns control to the
     enclosing op.
   }];
 
   let assemblyFormat = "attr-dict";
 }
 
 //===----------------------------------------------------------------------===//
 // 2.9.2 Workshare Loop Construct
 //===----------------------------------------------------------------------===//
 
 def WsLoopOp : OpenMP_Op<"wsloop", [AttrSizedOperandSegments,
                          AllTypesMatch<["lowerBound", "upperBound", "step"]>]> {
   let summary = "workshare loop construct";
   let description = [{
     The workshare loop construct specifies that the iterations of the loop(s)
let description = [{
     operation's results.
   }];
 
   let arguments = (ins Variadic<AnyType>:$results);
 
   let assemblyFormat = [{ ( `(` $results^ `:` type($results) `)` )? attr-dict}];
 }
 
 //===----------------------------------------------------------------------===//
 // 2.10.4 taskyield Construct
 //===----------------------------------------------------------------------===//
 
 def TaskyieldOp : OpenMP_Op<"taskyield"> {
   let summary = "taskyield construct";
   let description = [{
     The taskyield construct specifies that the current task can be suspended
     in favor of execution of a different task.
   }];
 
   let assemblyFormat = "attr-dict";
 }
 
+//===--------------------------------------------------------------------===//
+// 2.12.2 target data Construct 
+//===--------------------------------------------------------------------===//
+
+def Target_Data: 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.
+
+    The omp target data directive maps variables to a device data environment, 
+    and defines the lexical scope of the data environment that is created. 
+    The omp target data directive can reduce data copies to and from the 
+    offloading device when multiple target regions are using the same data.
+
+    The optional $if_expr parameter specifies a boolean result of a
+    conditional check. If this value is 1 or is not provided then the target
+    region runs on a device, if it is 0 then the target region is executed on the
+    host device.
+
+    The optional $device parameter specifies the device number for the target region.
+
+    The optional $use_device_ptr specifies the device pointers to the corresponding 
+    list items in the device data environment
+  
+    The optional $use_device_addr specifies the adress of the objects in the device 
+    data enviornment
+
+    The $map_operands specifies operands with no map type
+
+    The $map_to_operands specifies operands with  map type "to"
+
+    The $map_from_operands specifies operands with map type "from"
+
+    The $map_alloc_operands specifies operands with map type "alloc"
+
+    The $map_release_operands specifies operands with map type "release"
+
+    The $map_delete_operands specifies operands with map type "delete"
+
+    The $map_type_modifier vector specifies the modifier for each map type operand
+  }];
+
+  let arguments = (ins Optional<I1>:$if_expr,
+             Optional<AnyInteger>:$device,
+             Variadic<AnyType>:$use_device_ptr,
+             Variadic<AnyType>:$use_device_addr,
+             Variadic<AnyType>:$map_operands,
+             Variadic<AnyType>:$map_to_operands,
+             Variadic<AnyType>:$map_from_operands,
+             Variadic<AnyType>:$map_to_from_operands,
+             Variadic<AnyType>:$map_alloc_operands,
+             Variadic<AnyType>:$map_release_operands,
+             Variadic<AnyType>:$map_delete_operands,
+             VectorOf<[AnyInteger]>:$map_type_modifier);
+
+  let regions = (region AnyRegion:$region);
+}
+
+//===---------------------------------------------------------------------===//
+// 2.12.3 target enter data Construct
+//===---------------------------------------------------------------------===//
+
+def Target_EnterDataOp: OpenMP_Op<"target_enter_data",  [AttrSizedOperandSegments]>{
+  let  summary = "target enter data construct";
+  let description = [{
+    The target enter data directive specifies that variables are mapped to a device 
+    data environment. The target enter data directive is a stand-alone directive.
+  
+    The optional $if_expr parameter specifies a boolean result of a
+    conditional check. If this value is 1 or is not provided then the target
+    region runs on a device, if it is 0 then the target region is executed on the
+    host device.
+
+    The optional $device parameter specifies the device number for the target region.
+
+    The optional $nowait elliminates the implicit barrier so the parent task can 
+    make progress even if the target task is not yet completed.
+
+    The $map_operands specifies operands with no map type
+
+    The $map_to_operands specifies operands with  map type "to"
+
+    The $map_from_operands specifies operands with map type "from"
+
+    The $map_alloc_operands specifies operands with map type "alloc"
+
+    The $map_release_operands specifies operands with map type "release"
+
+    The $map_delete_operands specifies operands with map type "delete"
+
+    The $map_type_modifier vector specifies the modifier for each map type operand
+
+    TODO:  depend
+  }];
+
+  let arguments = (ins Optional<I1>:$if_expr,
+             Optional<AnyInteger>:$device,
+             UnitAttr:$nowait,
+             Variadic<AnyType>:$map_operands,
+             Variadic<AnyType>:$map_to_operands,
+             Variadic<AnyType>:$map_from_operands,
+             Variadic<AnyType>:$map_to_from_operands,
+             Variadic<AnyType>:$map_alloc_operands,
+             Variadic<AnyType>:$map_release_operands,
+             Variadic<AnyType>:$map_delete_operands,
+             VectorOf<[AnyInteger]>:$map_type_modifier);
+
+  let regions = (region AnyRegion:$region);
+}
+
+//===---------------------------------------------------------------------===//
+// 2.12.4 target exit data Construct
+//===---------------------------------------------------------------------===//
+
+def Target_ExitDataOp: OpenMP_Op<"target_exit_data",  [AttrSizedOperandSegments]>{
+  let  summary = "target exit data construct";
+  let description = [{
+    The target exit data directive specifies that variables are mapped to a 
+    device data environment. The target exit data directive is a stand-alone directive.
+
+    The optional $if_expr parameter specifies a boolean result of a
+    conditional check. If this value is 1 or is not provided then the target
+    region runs on a device, if it is 0 then the target region is executed on the
+    host device.
+
+    The optional $device parameter specifies the device number for the target region.
+
+    The optional $nowait elliminates the implicit barrier so the parent task can make progress
+    even if the target task is not yet completed.
+  
+    The $map_operands specifies operands with no map type
+
+    The $map_to_operands specifies operands with  map type "to"
+
+    The $map_from_operands specifies operands with map type "from"
+
+    The $map_alloc_operands specifies operands with map type "alloc"
+
+    The $map_release_operands specifies operands with map type "release"
+
+    The $map_delete_operands specifies operands with map type "delete"
+
+    The $map_type_modifier vector specifies the modifier for each map type operand
+
+    TODO: depend
+  }];
+
+  let arguments = (ins Optional<I1>:$if_expr,
+             Optional<AnyInteger>:$device,
+             UnitAttr:$nowait,
+             Variadic<AnyType>:$map_operands,
+             Variadic<AnyType>:$map_to_operands,
+             Variadic<AnyType>:$map_from_operands,
+             Variadic<AnyType>:$map_to_from_operands,
+             Variadic<AnyType>:$map_alloc_operands,
+             Variadic<AnyType>:$map_release_operands,
+             Variadic<AnyType>:$map_delete_operands,
+             VectorOf<[AnyInteger]>:$map_type_modifier);
+                                     
+}
+
 //===----------------------------------------------------------------------===//
 // 2.13.7 flush Construct
 //===----------------------------------------------------------------------===//
 def FlushOp : OpenMP_Op<"flush"> {
   let summary = "flush construct";
   let description = [{
     The flush construct executes the OpenMP flush operation. This operation
     makes a thread’s temporary view of memory consistent with memory and

mlir/test/Dialect/OpenMP/ops.mlir

omp.wsloop (%iv1, %iv2) : i32 = (%lb1, %lb2) to (%ub1, %ub2) step (%step1, %step2) {
     %2 = "test.payload"(%iv2) : (i32) -> (index)
     memref.store %iv1, %data1[%1] : memref<?xi32>
     memref.store %iv2, %data1[%2] : memref<?xi32>
     omp.yield
   }
 
   return
 }
+// CHECK-LABEL: omp_target_data
+func @omp_target_data (%if_cond : i1, %device : si32,  %data1: memref<?xi32>, %data2: memref<?xi32>) -> () {
+
+  // CHECK: omp.target_data
+   "omp.target_data"(%if_cond, %device, %data1, %data2) ({
+       // CHECK: omp.terminator
+       omp.terminator
+    }) {operand_segment_sizes = dense<[1,1,1,1,0,0,0,0,0,0,0,0]>: vector<12xi32> } : ( i1, si32, memref<?xi32>, memref<?xi32> ) -> ()
+
+  // CHECK: omp.target_enter_data
+   "omp.target_enter_data"(%if_cond, %device) ({
+       // CHECK: omp.terminator
+       omp.terminator
+    }) {operand_segment_sizes = dense<[1,1,0,0,0,0,0,0,0,0]>: vector<10xi32>, nowait } : ( i1, si32 ) -> ()
+
+  // CHECK:omp.target_exit_data
+   "omp.target_exit_data"(%if_cond, %device)
+       {operand_segment_sizes = dense<[1,1,0,0,0,0,0,0,0,0]>: vector<10xi32>, nowait } : ( i1, si32 ) -> ()
+
+  // CHECK: omp.barrier
+    omp.barrier
+
+    return
+}
+