[PATCH] D72802: [mlir] Introduce bare ptr calling convention for MemRefs in LLVM dialect

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 16 21:03:00 PST 2020


mehdi_amini added inline comments.


================
Comment at: mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h:76
+    LLVMTypeConverter &converter, OwningRewritePatternList &patterns);
+
 /// Creates a pass to convert the Standard dialect into the LLVMIR dialect.
----------------
I'm concerned about the scalability of this approach.

You're making the type converter extensible through inheritance and overloading, which is not gonna compose well: it is impossible to another orthogonal customization point.
Then extending it with a new subclass requires new header entry points: the multiple populate* functions does not seems very nice to me already, and this is making it just harder to figure out how to use this header.

If we are convinced that we don't need to scale or that we won't need more customization point, then I rather not use any virtual function and look into passing a config struct with flag on the lowering behavior we want.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72802





More information about the llvm-commits mailing list