[PATCH] D44109: TableGen: Add a defset statement

[Artem Belevich via Phabricator via llvm-commits]

tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: docs/TableGen/LangRef.rst:360-368
+``defset``
+----------
+.. productionlist::
+   Defset: "defset" `Type` `TokIdentifier` "=" "{" `Object`* "}"
+
+All records inside the braces are defined normally, and are additionally
+collected in a globally accessible list of the given name. The given type
----------------
nhaehnle wrote:
> tra wrote:
> > Does it mean that instantiating any record that is not of type A will be an error? That sounds rather limiting.
> > That would prevent use of anonymous records or non-anonymous records or different types we may want to instantiate within the same multiclass hierarchy without having to duplicate all of it.
> > 
> > Could we change the semantics to `the list collects all instantiated records of type A` and, maybe, allow more than one pair of type:list , so we can collect multiple record types?
> > 
> > Another approach would be to say -- give me all records of type A, created by this defm instance. I.e. instead of `defset A foo = { records }` you do something like `defm FOO : M<...>; list<A> foo = !defset(FOO)`. 
> > 
> Anonymous (or rather "inline"?) records are not an issue: they are created on a different path and never added to a defset, and I think they shouldn't be. I've clarified that.
> 
> I did make incompatible (non-anonymous) record types an error because one could accidentally make the type of `list<A>` too strict and then end up with records silently not being collected in surprising ways.
> 
> I see your point about allowing different types explicitly, but I'm not entirely convinced yet. In part the whole point of `defset` is that it helps avoid reducing duplication by using the resulting list in a `foreach` later. I'm also a bit hesitant about proliferation of `defset`, and I'd personally err on the side of starting with stricter semantics. It's always easier to go to looser semantics later if good use cases for it show up than the other way around.
Fair enough. I don't have any specific use case for multiple types of records inside defset and if anonymous records are allowed, this leaves some flexibility for what could be done inside the defset. 

I'm OK with this variant. It should indeed be reasonably easy to extend if/when we have specific use cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D44109