[PATCH] D74401: [MLIR] Add std.atomic_rmw op

[River Riddle via Phabricator via llvm-commits]

rriddle accepted this revision.
rriddle added a comment.
This revision is now accepted and ready to land.

LGTM after comments are resolved.



================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:2502
 
+struct SimpleAtomicMatch {
+  LLVM::AtomicBinOp binOp;
----------------
Can you document this struct please.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:2509
+
+// Try to match a simple atomic operation that is supported by the atomicrmw
+// intrinsic. This is looking for one of two cases:
----------------
Use /// for all top-level comments. Here and below.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:2728
+    // Conditionally branch to the end or back to the loop depending on %ok.
+    std::array<Value, 1> condBrProperOperands{ok};
+    std::array<Block *, 2> condBrDestinations{endBlock, loopBlock};
----------------
For single element things you should be able to pass the values directly. Is there a problem you are running into doing that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74401