[PATCH] D78753: [MLIR] Add conversion from AtomicRMWOp -> GenericAtomicRMWOp.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added inline comments.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp:2877
+static void
+populateStdToStdConversionPatterns(LLVMTypeConverter &converter,
+                                   OwningRewritePatternList &patterns) {
----------------
herhut wrote:
> Expose this from a the `ConvertStandardToStandard` header file with a descriptive name.
Yes, I was thinking that this pattern is not at all concerned with LLVM and should not live in "StandardToLLVM". However, the decision when to apply this pattern is driven by what LLVM supports.

Also, ConvertStandardToStandard feels weird to me. lib/Conversion was originally intended to solve the problem of the A->B conversion depending both on A dialect and B dialect so making little sense in either Dialect/A or Dialect/B... I would rather have it in `lib/Dialect/StandardOps/Transforms/ExpandAtomic` and call it from the LLVM conversion if necessary. If we ever have non-LLVM users or want to use the expansion for other cases than we currently do, it will be easier to extend. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78753