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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added inline comments.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:2656
+///
+struct AtomicCmpXchgOpLowering : public LoadStoreOpLowering<AtomicRMWOp> {
+  using Base::Base;
----------------
This seems like it'd be better a `loop.while` + progressive lowering? 


================
Comment at: mlir/test/Conversion/StandardToLLVM/convert-to-llvmir.mlir:869
+  atomic_rmw %loaded = %I[%i] : memref<10xi32> {
+    %0 = addi %loaded, %ival : i32
+    atomic_rmw_yield %0 : i32
----------------
So this is interesting to me.
Weren't you and/or @jbruestle advocating that we should have the reduction op encoded as an attribute in the case or affine.parallel_for with reduction semantics?
It seems a very similar scenario to me, I'd be interested of where you draw the distinction between "encoded as an attribute" and just use a region?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74401