[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Use `omp.canonical_loop` once it is in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. In D105584#4081228 , @kiranchandramohan wrote: > Since the plan is to switch to the Canonical Style operation, maybe this can > wait till the `omp.canonical_loop` changes are in? Ok..make sense Repository: rG

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan added a comment. Herald added a subscriber: thopre. Since the plan is to switch to the Canonical Style operation, may be this can wait till the `omp.canonical_loop` changes are in? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 486732. abidmalikwaterloo added a comment. Added a pretty test for omp.distrubute in ops.mlir Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 Files:

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan added inline comments. Comment at: mlir/test/Dialect/OpenMP/ops.mlir:124 +// CHECK-LABEL: omp_DistributeOp +func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, %data_var : memref, %chunk_var : i32) -> () { abidmalikwaterloo

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked 5 inline comments as done. abidmalikwaterloo added inline comments. Comment at: mlir/test/Dialect/OpenMP/ops.mlir:124 +// CHECK-LABEL: omp_DistributeOp +func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, %data_var : memref,

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. @abidmalikwaterloo It seems you missed a few of the previous comments. Please fix these so that we can approve. Comment at:

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 480159. abidmalikwaterloo marked 3 inline comments as done. abidmalikwaterloo added a comment. Update the patch based on the feedback and comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan added a comment. In D105584#3954144 , @abidmalikwaterloo wrote: > In D105584#3917238 , > @kiranchandramohan wrote: > >> Patch probably needs a rebase. A few more minor things to fix. Looks

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. In D105584#3917238 , @kiranchandramohan wrote: > Patch probably needs a rebase. A few more minor things to fix. Looks mostly > ready. With which branch should I rebase? I have taken care rest of the comments.

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

kiranchandramohan requested changes to this revision. kiranchandramohan added a comment. This revision now requires changes to proceed. Patch probably needs a rebase. A few more minor things to fix. Looks mostly ready. Comment at:

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 473706. abidmalikwaterloo marked 2 inline comments as done. abidmalikwaterloo added a comment. corrected the syntex of the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449 ParentOneOf<["WsLoopOp", "ReductionDeclareOp", - "AtomicUpdateOp", "SimdLoopOp"]>]> { + "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> { let summary = "loop yield

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked 2 inline comments as done. abidmalikwaterloo added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449 ParentOneOf<["WsLoopOp", "ReductionDeclareOp", - "AtomicUpdateOp", "SimdLoopOp"]>]> { + "AtomicUpdateOp",

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. I still see some review comments not done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___ cfe-commits mailing list

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. Ping! Does this need any further change? Seems okay to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___ cfe-commits

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. Seems that everything has been taken care of. It was accidentally out of my radar (do not know why). I will go through it once again and update it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. Herald added a reviewer: nicolasvasilache. Herald added subscribers: Moerafaat, zero9178. Are you still working on this? Otherwise please close it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449 ParentOneOf<["WsLoopOp", "ReductionDeclareOp", - "AtomicUpdateOp", "SimdLoopOp"]>]> { + "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> { let summary = "loop yield

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 442343. abidmalikwaterloo added a comment. Removed unrelated changes in CMakefile.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 Files:

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:30 + clangBasic + clangFrontend ) You still have these changes that are unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked an inline comment as done. abidmalikwaterloo added a comment. update the changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105584/new/ https://reviews.llvm.org/D105584 ___

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 442321. abidmalikwaterloo marked 11 inline comments as done. abidmalikwaterloo added a comment. Herald added a subscriber: anlunx. Updated DistributeOp::verify as per reviewer's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added inline comments. Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:215 +LogicalResult DistributeOp::verify(){ + if (this->lowerBound().empty()) { +return emitOpError() << "empty lowerbound for distribute loop operation"; No brace -

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 441105. abidmalikwaterloo added a comment. Update the patch for assembly format. Change the WSLoopControl to LoopControl. Added DistributeOp to parent of Yield OP. Added one test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. In D105584#3618040 , @abidmalikwaterloo wrote: > In D105584#3617666 , @clementval > wrote: > >> In D105584#3617456 , >>

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. In D105584#3617666 , @clementval wrote: > In D105584#3617456 , > @abidmalikwaterloo wrote: > >> // CHECK-LABEL: omp_DistributeOp >> func.func @omp_DistributeOp(%lb :

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

clementval added a comment. In D105584#3617456 , @abidmalikwaterloo wrote: > // CHECK-LABEL: omp_DistributeOp > func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, > %data_var : memref, %chunk_var : i32) -> () { > // CHECK:

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added a comment. // CHECK-LABEL: omp_DistributeOp func.func @omp_DistributeOp(%lb : index, %ub : index, %step : index, %data_var : memref, %chunk_var : i32) -> () { // CHECK: omp.wsloop collapse(2) "omp.DistributeOp" (%lb, %ub, %step) ({ ^bb0(%iv: index):

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked an inline comment as done. abidmalikwaterloo added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend shraiysh wrote: > abidmalikwaterloo wrote: > > shraiysh wrote: > > >

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

shraiysh added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend abidmalikwaterloo wrote: > shraiysh wrote: > > abidmalikwaterloo wrote: > > > shraiysh wrote: > > > > unrelated change? > > > When I

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked an inline comment as done. abidmalikwaterloo added inline comments. Herald added a subscriber: bzcheeseman. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend shraiysh wrote: > abidmalikwaterloo

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

shraiysh added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend abidmalikwaterloo wrote: > shraiysh wrote: > > unrelated change? > When I rebase, these changes were highlighted in the main branch which

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo marked 2 inline comments as done. abidmalikwaterloo added inline comments. Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:488-490 +`private_var`, `firstprivate_var`, and `lastprivate_var` arguments are +variadic list of operands that specify

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo added inline comments. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend shraiysh wrote: > unrelated change? When I rebase, these changes were highlighted in the main branch which was missing in the

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

shraiysh added a comment. Thanks for working on this. A couple comments. There are no testcases. Please add testcases. Comment at: clang/lib/Testing/CMakeLists.txt:29 llvm_gtest + clangBasic + clangFrontend unrelated change? Comment

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

abidmalikwaterloo updated this revision to Diff 428955. abidmalikwaterloo added a comment. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Added the assembly format, dist_schedule parser support Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION