[PATCH] D76571: [mlir] Start simple C++ backend

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar marked 2 inline comments as done.
jpienaar added a comment.

In D76571#1937625 <https://reviews.llvm.org/D76571#1937625>, @mehdi_amini wrote:

> Can you discuss this on Discourse?


Sure



================
Comment at: mlir/include/mlir/Target/Cpp.h:39
+/// Known limitations:
+/// - Doesn't understand C++ semantics and so may emit invalid code;
+/// - Doesn't attempt to ensure the function names are valid C++ names;
----------------
stephenneuendorffer wrote:
> What do you think about introducing an "EmitC" dialect, rather than doing string catenation here, isolate it somewhere else?
That would be very nice, but would be much larger effort, you'd need to be able to handle lambdas, templates and many different things that a dialect would want to emit.


================
Comment at: mlir/lib/Target/Cpp/TranslateToCpp.cpp:197-207
+llvm::StringMap<CppEmitter::EmitterRegistryFunction> &
+getMutableCppEmitterRegistry() {
+  static llvm::StringMap<CppEmitter::EmitterRegistryFunction> registry;
+  return registry;
+}
+
+void mlir::registerCppEmitter(
----------------
stephenneuendorffer wrote:
> Maybe an Interface would work well here, instead of having a separate registry?  I've used this to build a simulator for some dialects and it seems to scale reasonably well.
That is discussed in the rev description, making it an interface is a bit more invasive and requires one lowering for a given op, while one could have multiple completely different emissions for the same op, which is why I went for a separate registry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76571