[PATCH] D112982: [MLIR][OpenMP] Added omp.atomic.update

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

Thanks for this patch. Had a quick look. Looks OK. I have a couple of points.



================
Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:1403
+    return parser.emitError(parser.getNameLoc())
+           << "invalid atomic bin op in atomic update\n";
+  auto attr =
----------------
Move to verifier (also)?


================
Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:1415-1418
+    return parser.emitError(parser.getNameLoc())
+           << "atomic update variable " << x.name
+           << " not found in the RHS of the assignment statement in an"
+              " atomic.update operation";
----------------
Should this be moved to the verifier (also)?


================
Comment at: mlir/test/Dialect/OpenMP/ops.mlir:538-544
+  // CHECK: omp.atomic.update %[[XBOOL]] = %[[XBOOL]] and %[[EXPRBOOL]] : memref<i1>, i1
+  omp.atomic.update %xBool = %xBool and %exprBool : memref<i1>, i1
+  // CHECK: omp.atomic.update %[[XBOOL]] = %[[XBOOL]] or %[[EXPRBOOL]] : memref<i1>, i1
+  omp.atomic.update %xBool = %xBool or %exprBool : memref<i1>, i1
+  // CHECK: omp.atomic.update %[[XBOOL]] = %[[XBOOL]] xor %[[EXPRBOOL]] : memref<i1>, i1
+  omp.atomic.update %xBool = %xBool xor %exprBool : memref<i1>, i1
+  // CHECK: omp.atomic.update %[[XBOOL]] = %[[XBOOL]] shiftr %[[EXPRBOOL]] : memref<i1>, i1
----------------
These could be for integers as well right? Can you add a test for regular integer types also?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112982