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

[Stephan Herhut via Phabricator via llvm-commits]

herhut added inline comments.


================
Comment at: mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp:2877
+static void
+populateStdToStdConversionPatterns(LLVMTypeConverter &converter,
+                                   OwningRewritePatternList &patterns) {
----------------
ftynse wrote:
> 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. 
Thinking of it, we do the same in the GPU dialect where we have the `AllReduceOpLowering` that also lives in `Transforms`. So +1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78753