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: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:449
ParentOneOf<["WsLoopOp", "ReductionDeclareOp",
- "AtomicUpdateOp", "SimdLoopOp"]>]> {
+ "AtomicUpdateOp", "SimdLoopOp","DistributeOp"]>]> {
let summary = "loop yield and termination operation";
----------------
clementval wrote:
> abidmalikwaterloo wrote:
> > clementval wrote:
> > >
> > What's the problem here? It seems fine to me.
> syntax doesn't match the existing code. Change is highlighted.
Nit: Please fix this (adding a space before `"DistributeOp"`).
================
Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:211
+//===---------------------------------------------------------------------===//
+// Verifier for Dristribute Op
+//===---------------------------------------------------------------------===//
----------------
kiranchandramohan wrote:
>
Nit: Please make this change.
================
Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:220
+ return emitOpError() << "empty upperbound for distribute loop operation";
+
+ return success();
----------------
kiranchandramohan wrote:
> Nit: Check that their sizes are equal as well. And also if step is present
> then its size matches lowerbound/upperbound.
Nit: Please make this change as well.
================
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) -> () {
----------------
kiranchandramohan wrote:
> Add a pretty-print test as well.
Nit: please add a pretty-print test.
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
