This is an archive of the discontinued LLVM Phabricator instance.

TableGen: Delay instantiating inline anonymous records
ClosedPublic

Authored by nhaehnle on Feb 26 2018, 12:32 AM.

Details

Summary

Only instantiate anonymous records once all variable references in template
arguments have been resolved. This allows patterns like the new test case,
which in practice can appear in expressions like:

class IntrinsicTypeProfile<list<LLVMType> ty, int shift> {
  list<LLVMType> types =
    !listconcat(ty, [llvm_any_ty, LLVMMatchType<shift>]);
}

class FooIntrinsic<IntrinsicTypeProfile P, ...>
  : Intrinsic<..., P.types, ...>;

Without this change, the anonymous LLVMMatchType instantiation would
never get resolved.

Another consequence of this change is that anonymous inline
instantiations are uniqued via the folding set of the newly introduced
VarDefInit.

Change-Id: I7a7041a20e297cf98c9109b28d85e64e176c932a

Diff Detail

Event Timeline

nhaehnle created this revision.Feb 26 2018, 12:32 AM
tra added a comment.Feb 26 2018, 5:00 PM

Yay! Another of my favorite complaints about tablegen will be gone! Thank you!
Looks good overall, except for that concern about static pool and using tablegen as a library.

include/llvm/TableGen/Record.h
1065

Nit. I'd move it down to args_begin/args_end so it's easy to see the details of const_iterator next to the functions that return it. It's fine either way.

lib/TableGen/Record.cpp
1369

Same concern as I've raised in D43680 regarding use of the pool values across multiple invocations of tablegen's main function.

lib/TableGen/TGParser.cpp
1360–1363

TArgs could use a better name. Something along the lines of ExpectedArgNum. Calling it TArgs makes the error message somewhat confusing. My first thought was "Hold on a sec. We've just checked that template args (TArgs) is *less* than the number of [expected] arguments (Args)... Oh! TArgs is not the template args, it is the expected args".

1370

Nit. Class->getValue(TArgs[i]) could be extracted into a variable at the beginning of the for above.

nhaehnle updated this revision to Diff 136995.Mar 5 2018, 7:14 AM
nhaehnle marked 3 inline comments as done.

Address style issues.

include/llvm/TableGen/Record.h
1065

Makes sense. I believe I was just following some existing pattern elsewhere in the code.

lib/TableGen/Record.cpp
1369

Hmm. I'm less inclined to change this here, since all the other *Init classes work the same way already. I don't see anything that makes VarDefInit qualitatively different from e.g. OpInit.

lib/TableGen/TGParser.cpp
1360–1363

Good point.

1370

Done.

tra accepted this revision.Mar 5 2018, 4:27 PM
tra added inline comments.
lib/TableGen/Record.cpp
1369

OK.

One of these days we'll need to clean these static pools up on exit from TableGenMain.

This revision is now accepted and ready to land.Mar 5 2018, 4:27 PM
This revision was automatically updated to reflect the committed changes.