shraiysh added a comment.
Thanks for the review @peixin.
================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:513
+
+def AtomicReadOp : OpenMP_Op<"atomic.read"> {
+ let arguments = (ins OpenMP_PointerLikeType:$address,
----------------
peixin wrote:
> How do you plan to handle synchronization between the threads if there is no
> region in atomic read op? Will there be one implicit `kmpc_flush` function
> call before `!$omp end atomic`? If yes, it is easier to find to location of
> emiting `kmpc_flush` if we have one region here. Please let me know if I am
> wrong.
Yes - there will be a kmpc_flush call, but OpenMP Dialect does not have to
worry about that. OpenMP IR Builder takes care of flushes in the function [[
https://llvm.org/doxygen/classllvm_1_1OpenMPIRBuilder.html#a388d5a62753f4e7ff4b72e54c1233fbc
| createAtomicRead ]].
================
Comment at: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp:1246
+ StringRef memOrder = op.memory_order().getValue();
+ if (memOrder.equals("acq_rel") || memOrder.equals("release"))
+ return op.emitError(
----------------
peixin wrote:
> Please also check OpenMP 5.1 Spec. It seems that `acq_rel` is allowed if read
> clause is specified.
Thanks for this observation, I will update it with the 5.1 standard.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111992/new/
https://reviews.llvm.org/D111992
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
