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

Tue Nov 30 03:43:50 PST 2021

kiranchandramohan added a comment.

In D112982#3161031 <https://reviews.llvm.org/D112982#3161031>, @shraiysh wrote:

> Thanks for the review @kiranchandramohan.
>
> In D112982#3160888 <https://reviews.llvm.org/D112982#3160888>, @kiranchandramohan wrote:
>
>> What will prevent the builder from building operations which has the errors that you are catching during parsing?
>
> The BinOp error will be taken care of by the AtomicBinOpKindAttr. An integer value from the allowed ones will be forced because of that constraint.
>
> The other error - about X being in RHS - that is taken care of by design as the in memory structure has X, expr, binOp and isXBinOpExpr. It is not possible to run into that error while building the operation.

Thanks. I have one more question regarding binary ops.

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:594
 }

+def ATOMIC_BINOP_KIND_ADD : I64EnumAttrCase<"ADD", 0>;
----------------
Nit: Please add a TODO here to autogenerate from OMP.td in future if possible.

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:624
+                       AnyType:$expr,
+                       UnitAttr:$isXBinopExpr,
+                       AtomicBinOpKindAttr:$binop,
----------------
Are any non-binary operations supported currently? If not, can this be removed? If yes, can a test be added?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112982