[PATCH] D43519: [tblgen] Allow !cast to records that don't yet exist during multiclass parsing.

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

In https://reviews.llvm.org/D43519#1014318, @nhaehnle wrote:

> I'm a bit suspicious of this change because it introduces the potential for recursion and I think it relies on some implementation detail of how the !cast is re-resolved once the record is finally instantiated.


I've failed to come up with a way to trigger recursion. We do need to instantiate the records in the end and at that point, if there's recursion, we'll eventually end up doing a !cast to a record that does not exist yet and we will fail then. I may be wrong. Can you think of an example?

One way to think of what this patch does is delaying evaluation of !cast inside multiclass as if the arguments of !cast are not foldable yet. E.g. is M2 would pass to https://reviews.llvm.org/M1 a template parameter instead of a constant string.  So, we already allow parsing multiclasses with unresolved casts. Whether it's unresolved because the record does not exist or because the name of the record is not known should not matter, IMO.

> I suspect the proper way to fix this is to change multiclass M2 slightly:
> 
>   multiclass M2 {
>     defm NAME: M1<NAME # "d0">;
>   }
> 
> 
> ... and augment TGParser::ParseIDValue to explicitly handle "NAME" in the if (CurMultiClass) statement.

The NAME resolution is a different issue. For this patch, the key problem is that in `M1` the record created by _r1, which _m1 eventually wants to refer to, does not exist until https://reviews.llvm.org/M1 is instantiated. 
M2 is just used to trigger the problem. It's easy to work around by just doing top-level `defm d1: M1<"d1">;`

> That said, I haven't tried this, and record name resolution is a complete inconsistent mess. FWIW, my take on it is that NAME should simply be an implicit class/multiclass template argument, but unfortunately a lot of .td files have grown dependent on subtle implementation details, so cleaning this up is tough.




https://reviews.llvm.org/D43519