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

This revision was automatically updated to reflect the committed changes. Closed by commit rGa4699a43e426: [MLIR][OpenMP] Added target data, exit data, and enter data operation… (authored by TIFitis). Changed prior to commit: https://reviews.llvm.org/D131915?vs=489881&id=491299#toc Repository:

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

clementval added a comment. Just small nit Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:628-629 +int64_t mapTypeBits = 0x00; +auto mapOp = map_operands[i]; +auto mapTypeOp = map_types[i]; + auto should be spelled out here. Repository:

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

TIFitis marked 2 inline comments as done. TIFitis added a comment. I'm planning on merging the patch early next week. If you'd like to see any changes or need more time to review, please let me know :) Akash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

TIFitis marked 5 inline comments as done. TIFitis added a comment. In D131915#4056730 , @kiranchandramohan wrote: > LGTM. Thanks for making the changes and for your patience. Please wait a > couple of days to give other reviewers a chance to have a look

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

TIFitis updated this revision to Diff 489881. TIFitis added a comment. Addressed reviewer comments. Updated printer and parser to remove the IntegerAttr from appearing. Removed none from map type modifiers, absence of other modifiers implicitly means none. This helps keep it in closer to the spe

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

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Thanks for making the changes and for your patience. Please wait a couple of days to give other reviewers a chance to have a look before submitting. Could you updat

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

TIFitis updated this revision to Diff 489515. TIFitis added a comment. Minor indentation changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131915/new/ https://reviews.llvm.org/D131915 Files: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td m

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

TIFitis updated this revision to Diff 489510. TIFitis added a comment. Removed use of VariadicOfVariadic. Added new I64ArrayAttr for map_types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131915/new/ https://reviews.llvm.org/D131915 Files: mli

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

kiranchandramohan added inline comments. Comment at: mlir/lib/Dialect/OpenMP/CMakeLists.txt:15 MLIRLLVMDialect + MLIRArithDialect ) TIFitis wrote: > kiranchandramohan wrote: > > TIFitis wrote: > > > kiranchandramohan wrote: > > > > Why is this needed here?

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

TIFitis marked 4 inline comments as done. TIFitis added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:829 + VariadicOfVariadic:$map_operands, + DenseI32ArrayAttr:$map_operand_segments); + kiranchandramohan wr

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

TIFitis updated this revision to Diff 489345. TIFitis marked 7 inline comments as done. TIFitis added a comment. Added verifiers, removed use of magic constants and addressed other reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Can you add the other authors as Co-authors in the patch Summary? In D131915#4001620 , @TIFitis wrote: >> Can you add

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

clementval added inline comments. Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:584 +std::stringstream typeMod, type; +if (mapTypeBits & 0x04) + typeMod << "always "; jdoerfert wrote: > TIFitis wrote: > > kiranchandramohan wrote: > > > The

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

jdoerfert added inline comments. Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:584 +std::stringstream typeMod, type; +if (mapTypeBits & 0x04) + typeMod << "always "; TIFitis wrote: > kiranchandramohan wrote: > > There is a `bitn` function

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

TIFitis marked 3 inline comments as done. TIFitis added a comment. > Can you add this as a test? AFAIS, the tests attached to this patch do not > seem to be exercising the `VariadicofVariadic` requirement. An explanation > with an example would be great. `VariadicOfVariadic` gives us a `SmallVe

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

TIFitis updated this revision to Diff 483545. TIFitis marked 2 inline comments as done. TIFitis added a comment. Addresed reviewer comments. Added second map clause to test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131915/new/ https://reviews.

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

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Thanks for the changes and your work here. In D131915#3994160 , @TIFitis wrote: > Added custom printer & parser for ma

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

TIFitis marked 10 inline comments as done. TIFitis added inline comments. Comment at: mlir/test/Dialect/OpenMP/ops.mlir:389 +// CHECK: omp.target_data +"omp.target_data"(%if_cond, %device, %data1, %data2) ({ + clementval wrote: > Can you switch to

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

TIFitis updated this revision to Diff 482732. TIFitis added a comment. Added custom printer & parser for map clause. Updated ops.mlir test to address reviewer comments. Custom Printer example: OMP: !$omp target exit data map(from: a,b) map(release: c) map(delete: d) map(always, from: e) Custom

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

TIFitis marked an inline comment as done. TIFitis added a comment. If people are happy with this structure then I'll work on adding a pretty printer for the map clause. Please suggest any other changes that are required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

TIFitis updated this revision to Diff 480029. TIFitis added a comment. Herald added a subscriber: Moerafaat. Changed map_operands to VariadicOfVariadic type. Variadic Enums are not supported, hence I've changed the map type to VariadicOfVariadic which allows to store the entire map clause includ

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

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:828 +(`use_device_addr` `(` $use_device_addr^ `:` type($use_device_addr) `)` )? +(`map` `(` $map_type_modifier_val `,` $map_type_val `:` $map_operands^ `:` type($map_operands) `)`

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

TIFitis requested changes to this revision. TIFitis added inline comments. This revision now requires changes to proceed. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:817-818 + Variadic:$use_device_addr, + MapTypeModifierAttr:$map_type_modifie

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

raghavendhra added a comment. Ping for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131915/new/ https://reviews.llvm.org/D131915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

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

raghavendhra updated this revision to Diff 468448. raghavendhra added a comment. Herald added projects: clang, LLVM, Flang. Herald added subscribers: llvm-commits, cfe-commits. Seperated map_operands with map-type and map-type-modifier as enum attributes. Repository: rG LLVM Github Monorepo C