This commit adds support for processing tablegen include files, and importing
various information from ODS. This includes operations, attribute+type constraints,
attribute/operation/type interfaces, etc. This will allow for much more robust tooling,
and also allows for referencing ODS constructs directly within PDLL (imported interfaces
can be used as constraints, operation result names can be used for member access, etc).
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Just half a scan (sorry have to context switch again but asking some in interim)
llvm/include/llvm/Support/SourceMgr.h | ||
---|---|---|
103 | Could we split these out? | |
mlir/include/mlir/Tools/PDLL/ODS/Constraint.h | ||
42 | Are these std::string due to life time concerns? | |
mlir/include/mlir/Tools/PDLL/ODS/Context.h | ||
65 | Contrary to MLIR IR side is there any expectation that this is useful beyond debugging? | |
mlir/include/mlir/Tools/PDLL/ODS/Operation.h | ||
115 | What does opaque here signify? |
mlir/include/mlir/Tools/PDLL/ODS/Constraint.h | ||
---|---|---|
1 | PDLL instead of PD:L | |
7 | (PDLL ODS constraints is such a mouthful that it may make sense to add file header comment) | |
mlir/include/mlir/Tools/PDLL/ODS/Context.h | ||
42 | What happens if a constraint by that name already exists? | |
48 | The above have add being const reference return, while here add is for mutable and lookup for non-mutable without insertion. What mutation is expected here/why isn't that expected above? | |
57 | insertOperation? (This is why your comment even says :-)). Up to you. | |
mlir/include/mlir/Tools/PDLL/ODS/Dialect.h | ||
36 | Ditto | |
43 | Out dated comment? (The name makes more sense to me with this comment) | |
mlir/include/mlir/Tools/PDLL/ODS/Operation.h | ||
55 | Same question wrt string vs stringref here. | |
71 | "represents a ... representation" feels off | |
124 | And here adds don't return anything (these feel inconsistent to have same names/prefixes enable different things) | |
mlir/lib/Tools/PDLL/ODS/Context.cpp | ||
102 | Could we use the scoped & indented ostream emitter? (Seems would make it more less likely to forget a space) | |
mlir/lib/Tools/PDLL/Parser/Parser.cpp | ||
716 | Sneaky :-) And include paths are not an issue? |
@rriddle could you land your stack?
Also, what do you expect the support to look like for something like class RankedFloatElementsAttr<int width, list<int> dims> : ?
Is the idea that pdll will understand int as a first class citizen or that we'll go through e.g. I64?
What about list<int> ?
Thanks!
Yep, sorry for the delay. Now that Jacques reviewed, PDL(L) is back at the top of my stack, so I'll be going through things tomorrow (PST).
Also, what do you expect the support to look like for something like class RankedFloatElementsAttr<int width, list<int> dims> : ?
Is the idea that pdll will understand int as a first class citizen or that we'll go through e.g. I64?
What about list<int> ?
For tablegen import, we can only really import defs at this point; i.e., we won't be able to import templated classes. So for now, if we want to
use them we'll generally need to explicitly define the templates we care about in .td and then import the fully specialized def.
mlir/include/mlir/Tools/PDLL/ODS/Constraint.h | ||
---|---|---|
42 | Yeah, in a follow up I intend to make the summary/description stuff all optional. | |
mlir/include/mlir/Tools/PDLL/ODS/Context.h | ||
42 | Just returns the previous one. Added in asserts that the info is the same on duplicate insertion. | |
48 | Constraints are self-contained, and don't really need any mutation after insertion. Dialects can have additional constructs inserted (operations/etc). We could make the constraints non-const on return, but there isn't a reason to right now. | |
65 | Not really. Printing here is really only for debugging/testing. | |
mlir/include/mlir/Tools/PDLL/ODS/Operation.h | ||
55 | Yep, this is for lifetime issues. | |
115 | Just dropped all of the opaque. | |
124 | Keeping these as add now that the functions returning things are called "insert". |
Could we split these out?