Hi Artem,
Thank you for your encouraging reply
I have now cleaned up the first batch of changes and put them on Phabricator here: ⚙ D43552 TableGen: Add some more helpful error messages
I've tried to keep individual changes small, and I've verified with `git rebase -x` that the build is good after each change. This first batch does not cause any changes in backend's generated files.
[snip]> - cleanup type checking> - cleanup variable resolving> - cleanup record instantiation> > That can mean whole lot of different things. Do you have an > overarching plan how tablegen is supposed to do all of the above in a > consistent manner? Just making it different may not improve things much.
Right, and we'll have to see in reviews. But just to give you an example, for the last point (cleaning up record instantiation), I ended up removing more than 150 lines of code while simultaneously enabling simple cases of foreach inside of multiclasses. I think that speaks for itself.
When it comes to variable resolving, I propose a simple resolver interface, see ⚙ D43564 TableGen: Introduce an abstract variable resolver interface.
This allows multiple template arguments to be resolved simultaneously, which should be a small performance improvements aside from the more subjective cleanup of the IMO odd resolveReferences interface. At the same time, this allows proper variable hiding when resolving inside nested !foreachs.
The type system stuff is more vague, and is mostly about making sure that we actually get the right types everywhere. For example, list types end up being incorrect all over the place at the moment.
- late generation of anonymous records that appear in expressions
That would help, somewhat, though the biggest problem (IMO) with the anonymous records is that when they are instantiated using multiclass template parameters, the inner records are not folded because the values are not known yet and they may never be known because instantiated anonymous record has no access to template arguments that may be used by its fields. If you moved instantiation of anonymous records to the point where the instances of the class/multiclass that uses them are generated, that would help a lot.
That's precisely what I've done, if I understand you correctly.
I've added a VarDefInit class which acts as an uninstantiated anonymous record and will "fold" to a DefInit once the template arguments are fully resolved to contain no variable references.
Additional benefits of this are that we don't keep partially resolved anonymous records in the RecordKeeper, and anonymous records that are instantiated with the same template arguments are instantiated only once, so there's less computation and memory use.
- cleanup !foreach
+1 for having scoped temporary variable. ATM tablegen can use a class field for that, but using it in the global scope still needs a record for it. Things also go wrong when one accidentally uses a record/field with a known value.
... and when nesting !foreach with the same iteration variable
- add !foldl
- add !isa<type>(...) and make !cast more useful
Nice.
- add !dag builtin to generate DAG nodes from lists
Yes, please! Constructing dags using !foreach and !con() is a pain.
- some other minor new built-ins like !size, !le, !lt, !ge, !gt
- add a defset mechanism for collecting records which can then later be
looped over in a foreach statement
- make foreach statements in multiclass work
- probably more...
I'll list few things that I have on my list of tablegen annoyances to fix:
- !subst() needs some minor tweaks when it's applied to dag from !foreach -- it wants specific types, but I sometimes want to replace a dag argument (a record) with another dag.
I haven't touched !subst, but I have a similar issue with !dag.
One of the things I've been thinking about (but haven't actually done) is that we should perhaps make the type lattice complete, by making UnsetInit typed with a new UnsetType, and adding an AnyType.
I ran into this mostly because it would make sense for !dag(op, childnodes, childnodenames) to de facto have the type !dag(any, list<any> or unset, list<string> or unset), or perhaps !dag(anyrecord, list<any> or unset, list<string> or unset).
- I want some sort of !macro() operator that will never be folded by itself, but will instead literally substitute its value when it's used, so it will have access to the multiclass template arguments, class fields, etc. We're currently using anonymous records for similar purposes, but those are instantiated at the wrong time, so it's often impossible to use them as sort of 'subroutines'. Your late generation change you've mentioned above may help, but I'm not sure if that's the use pattern you had in mind.
Actually yes, that's one of the things I rely on quite heavily in my AMDGPU changes. The AMDGPU backend already uses a pattern to implement subroutines that looks like this:
class getWhatever<parameters> {
type ret = ...;
}
This tends to work out in many cases because getWhatever<...>.ret is substituted early, but there are corner cases when you push it further, and I believe those are all fixed with the VarDefInit change I've described above. In any case, I'm using "subroutines" very heavily [0]
The other use pattern I had in mind is that I want to concatenate intrinsic type lists that include LLVMMatchType, and so I need to generate LLVMMatchType with calculated reference numbers, and that just doesn't fly today.
An earlier version of the patches is here if you already want to take a
look: ~nh/llvm - Misc LLVM things, mostly radeonsi (AMDGPU)
My plan is to clean those up over the next days and weeks and submit
them to Phabricator.
I'd be happy to help reviewing them.
Much appreciated!
With the exception of !foreach, which doesn't actually seem to be used,
I do have few outstanding changes for NVPTX where I rely on !foreach, but I think it should not be too hard to adapt them.
That's good to hear
Thanks,
Nicolai
[0] Tagebuch eines Interplanetaren Botschafters: TableGen #2: Functional Programming