[PATCH] D79410: [MLIR] [OpenMP] Add basic OpenMP parallel operation

[David Truby via Phabricator via llvm-commits]

DavidTruby marked an inline comment as done.
DavidTruby added inline comments.


================
Comment at: mlir/test/Dialect/OpenMP/ops.mlir:28
+  // CHECK: omp_parallel
+  "omp.parallel" (%if_cond, %num_threads, %data_var, %data_var, %data_var, %data_var) ({
+
----------------
ftynse wrote:
> These tests only use the generic syntax for `omp.parallel` so they don't exercise the custom parser you wrote. Generally, we trust the core infra tests to exercise the generic syntax and don't write tests for every op in the generic syntax. We do use generic syntax to test custom verifiers in cases when they cannot be triggered using the custom parser.
> 
> We do however write tests for the custom non-generated syntax, which you did not provide here. From reading the code in this diff, I suspect the parser and printer for `omp.parallel` dont match and there are no tests to convince me otherwise.
I'm working on a parser and pretty printer separately, so I think I should just remove the ones here. I wasn't sure if I still needed them or not in the mean time but from your comments it seems not. I'll remove them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79410/new/

https://reviews.llvm.org/D79410