[PATCH] D111992: [MLIR][OpenMP] Added omp.atomic.read and omp.atomic.write
Peixin Qiao via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 24 18:37:04 PDT 2021
peixin added inline comments.
================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5993
case OMPC_adjust_args:
+ case OMPC_memory_order:
llvm_unreachable("Clause is not allowed in 'omp atomic'.");
----------------
The memory_order clause in clang side is not handled in this patch. Maybe leaving the error is better since users will know it is not supported yet in clang.
================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:513
+
+def AtomicReadOp : OpenMP_Op<"atomic.read"> {
+ let arguments = (ins OpenMP_PointerLikeType:$address,
----------------
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.
================
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(
----------------
Please also check OpenMP 5.1 Spec. It seems that `acq_rel` is allowed if read clause is specified.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111992/new/
https://reviews.llvm.org/D111992
More information about the cfe-commits
mailing list