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 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 the data sharing attributes of the + list of values ---------------- If they are not added, please remove them from documentation too. It would be misleading otherwise. ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:514 + oilist(`schedule` `(` + custom<DistributeScheduleClause>( + $dist_schedule_var, ---------------- dist_schedule is a dummy clause, where kind is only allowed to be `static` according to the standard. I don't think that requires a custom function, we can instead have something like the following - ``` arguments = UnitAttr:$static_dist_schedule, Optional<IntLikeType>:$schedule_chunk assemblyFormat = `dist_schedule` `(` (`static` $static_dist_schedule^)? (`:` $schedule_chunk^)? `)` ``` Would that work? Let me know if there are any suggestions. ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:528 + + let regions = (region AnyRegion:$region); +} ---------------- I think we need a verifier for this too. There are a couple semantic checks which we can do in verifier. ================ Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:226 + return parser.emitError(parser.getNameLoc()) << + " expected scheudle kind"; + ---------------- nit: schedule spelling. 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
