[PATCH] D47431: TableGen: Allow foreach in multiclass to depend on template args

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham accepted this revision.
simon_tatham added a comment.
This revision is now accepted and ready to land.

In that case, LGTM – though I must admit that this is probably the part of tablegen I understand least well, so I can't guarantee not to have missed anything subtle!



================
Comment at: lib/TableGen/TGParser.cpp:2917
 
-    TemplateArgs.emplace_back(QualifiedNameOfImplicitName(MC), DefmName);
-
-    // Loop over all the def's in the multiclass, instantiating each one.
-    for (const std::unique_ptr<Record> &DefProto : MC->DefPrototypes) {
-      auto CurRec = make_unique<Record>(*DefProto);
-      CurRec->appendLoc(SubClassLoc);
+    Substs.emplace_back(QualifiedNameOfImplicitName(MC), DefmName);
 
----------------
nhaehnle wrote:
> simon_tatham wrote:
> > While you're looking at this code, should this be tracking whether the DefmName you're adding to Substs here was explicitly given by the user, or made up anonymously on line 2861?
> > 
> > I feel as if the end-product record that comes out of the whole chain of defms and foreaches ought to have its 'anonymous' flag set if //any// defm or def on the way to it was anonymous, because in that case some part of the final name will include something TableGen made up.
> I'd agree if we were recreating TableGen from scratch.
> 
> However, there are some existing TableGen files (e.g. something in X86 about condition codes) that rely on being able to substitute `NAME` reliably in a multiclass that is instantiated by a defm without a name. I did ponder changing those TableGen files accordingly while I did the earlier change (about streamlining NAME resolution), but ultimately decided against it to reduce the churn in existing .td files.
> 
> While I also think the behavior with the made-up "anonymous_NNNN" part in names is a bit weird, it's a comparatively minor wart that doesn't **really** hurt anything as far as I can tell.
Fair enough -- I suppose if the backend just needs a set of unique string identifiers and doesn't necessarily care exactly what they are, then that's enough.

In that case I should probably revert the tweak in that area I just made to my JSON backend (when I started exporting the anonymous flag). Fortunately you're about to cause a merge conflict that will remind me to do that :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D47431