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: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:490 + + The optional `dist_schedule_var` attribute specifies the schedule for this + loop, determining how the loop is distributed across the parallel threads. ---------------- Do you mean `static_dist_schedule` here? ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:495-496 + + The optional `collapse` attribute specifies the number of loops which + are collapsed to form the distribute loop. + }]; ---------------- We can remove `collapse` for now. ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:513 + ( $schedule_chunk^ `:` type($schedule_chunk))? `)` + |`collapse` `(` $collapse_val `)` + | `allocate` `(` ---------------- In https://reviews.llvm.org/D128338 we removed collapse from all constructs. Currently collapse is modelled by having multiple entries in the lowerBound, upperBound and step. ================ Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:211 +//===---------------------------------------------------------------------===// +// Verifier for Dristribute Op +//===---------------------------------------------------------------------===// ---------------- ================ Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:220 + return emitOpError() << "empty upperbound for distribute loop operation"; + + return success(); ---------------- Nit: Check that their sizes are equal as well. And also if step is present then its size matches lowerbound/upperbound. ================ 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<i32>, %chunk_var : i32) -> () { ---------------- Add a pretty-print test as well. 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 https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
