[PATCH] D86904: [mlir] Support for defining Types in tblgen

Mon Sep 28 12:56:27 PDT 2020

jdd added inline comments.

================
Comment at: mlir/include/mlir/TableGen/TypeDefGenHelpers.h:2
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
----------------
rriddle wrote:
> jdd wrote:
> > rriddle wrote:
> > > Do we need this file for this commit? This looks like it is only used for the testing and I'd rather leave parsing/printer utilities separately.
> > It's not just for testing. The auto-generated parsers/printers use the templates in this file so whenever one is using auto-generated printers/parsers this file has to be #included. The alternative is to emit this file in the generated code, but I prefer this approach so as to not bloat the emit'ed code.
> > 
> > I get that this template-based design decision is likely to be controversial. This is one piece where I'm looking for high-level feedback.
> Yeah, I'd prefer removing all of the parsing stuff from this revision. It is just going to hold back all of the other stuff, and is something that can be discussed separately.
If the parsing stuff what's blocking this, then I agree. I'd like to keep the custom parser code in tblgen and the parsing dispatch. I think this is value and and is not controversial, right?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86904