[TableGen] Support named arguments

[wangpc]

Summary

We provide a way to specify arguments in the form of name=value
so that we don't have to specify all optional arguments before the
one we'd like to change. Required arguments can alse be specified
in this way.

Note that the argument can only be specified once regardless of
the way (named or positional) to specify and positional arguments
should be put before named arguments.

[wangpc]

• Refactor.
• Add RISCV changes.

[wangpc]

llvm/lib/TableGen/TGParser.cpp

119–120

@nhaehnle
It seems that we can't define nested class in multiclass, so why do we need this?
Do you remember the context in D47431?

[wangpc]

Remove commented out code.

[michaelmaitland]

Thanks for this patch. It will make maintenance much easier!

llvm/lib/TableGen/TGParser.cpp

3138–3140

Does TemplateArgList production need to be updated to something along the lines of:

TemplateArgList ::= '<' [[NamedArg=]Value {',' [NamedArg=]Value}*] '>'

3214

nit: modifies element being iterated over, sets it to something in scope, but uses ArgName after we leave scope.

[craig.topper]

llvm/lib/TableGen/TGParser.h

293

Should the parameter be isDefm? Is see that's what one caller passes.

[Paul-C-Anagnostopoulos]

Nice. Don't forget to document it.

[wangpc]

• Rebase.
• Address comments.
• Update doc/ReleaseNotes.

[wangpc]

Fix test.

[wangpc]

• Refactor. ArgumentInits are added and we change the way to resolve arguments accordingly.
• Rebase.

[wangpc]

Update doc.

[wangpc]

Fix comments.

[michaelmaitland]

llvm/include/llvm/TableGen/Record.h

511

Should we call Value->getBit() instead? If the argument is a bit, we'd like it to behave like getBit was called on a bit, but if the argument was a IntInit we'd like it to behave like getBit was called on an int.

Another approach could be to put an llvm_unreachable.

513

Do we really want to return nullptr for getCastTo and convertInitializerTo? Maybe it would be better to llvm_unreachable("TODO: Implement").

llvm/lib/TableGen/TGParser.cpp

598

Add test for this error path?

613

Add test for error path?

641

Add test for this error path?

758–759

nit: should we rename isDefm to IsDefm?

[reames]

I think this is generally excellent idea. The overall structure seems to make sense here, but some of the details of TGParser.cpp lost me in a way where I can't give meaningful review feedback.

I'd like to suggest that you split this patch. A pre-patch which does nothing but add the ArgumentInit class *for positional arguments only* would go a long way to reducing the verbose, but uninteresting parts of this diff. It would also be entirely NFC, and thus easier to test and review on that basis. Once that landed - I am offering to review it - this could be rebased, and attention focused on the more semantically useful bits.

On a similar note, I think the extraction of resolveClassArguments and resolveMultiClassArguments could be done as a standalone NFC. Doing so would eliminate the mixture of code motion and modification in the current patch.

[jrtc27]

Unless I'm missing something I don't see a test for trying to name the same argument twice, only for both positional and named

[jrtc27]

Please also split out the RISC-V changes into their own review; they don't belong with the TableGen feature itself

[wangpc]

• Rebase.
• Add error test that specifies the same argument name twice.

[wangpc]

llvm/include/llvm/TableGen/Record.h

513

I decided to delegate all these method to Value.

llvm/lib/TableGen/TGParser.cpp

641

These should have already been catched by llvm/test/TableGen/template-args.td.

[wangpc]

clang-format.

[reames]

llvm/include/llvm/TableGen/Record.h

546–547

I'm not 100% on this, but I don't think sub-classing is the right model here. I think this code would be a lot easier to follow with a single ArgumentInit class with an optional index, an optional name, and a value. A single isPositionalArg() method then let's you dispatch in the few places you need to.

llvm/lib/TableGen/TGParser.cpp

740

Stray change, remove.

llvm/lib/TableGen/TGParser.h

280

Stray change, remove.

[MaskRay]

llvm/include/llvm/TableGen/Record.h

323

Add a trailing comma so that next time we add a new member, we can avoid touching this IK_NamedArgumentInit line

llvm/test/TableGen/named-arguments.td

15

Add spaces so that def align with }. This just looks nicer.

100

I wonder whether tablegen parsing doesn't have error recovery so that we need an invocation for every test...

In clang we test many errors in one invocation...

[wangpc]

• Rebase.
• No sub-typing for ArgumentInit.
• Add spaces before def in tests.

[wangpc]

llvm/include/llvm/TableGen/Record.h

546–547

The original patch was just like what you said when I started this patch. The complexity won't disappear, it just moves from sub-typing to branches I think.

llvm/test/TableGen/named-arguments.td

100

Yes, there is no error recovery for TableGen parser…
The parser is a really simple recursive descent parser for LL(1) grammar.

[reames]

llvm/include/llvm/TableGen/Record.h

496

Drop the union. The minimal space savings is not worth the awful and hard to debug problems.

You can use a signal value (i.e. -1 on Index) to track the type of the argument.

515–525

Add asserts on the kind for both this and the following accessor.

llvm/lib/TableGen/Record.cpp

382

Once you drop the union, this simplifies.

llvm/lib/TableGen/TGParser.cpp

119–120

The removal of this code needs to be resolved. I don't understand the context well enough to address this point.

3161

I'm missing something here. One the *first* named argument, aren't we constructing the value with the wrong type? Does that matter?

3170

I think this code might be easier to follow if you moved the error checking for name and unset value to the point the arguments are resolved. This would require that we support both a qualified and unqualified state in the argument name field which is not great. Unless, can we move resolution to the use of the named argument?

(Not at all sure of this proposal, take it as a weak suggestion only.)

3189

Please use early-return here.

[arsenm]

llvm/include/llvm/TableGen/Record.h

496

or use std::variant

[wangpc]

• Use std::variant instead of union.
• Address @reames 's comments.

[wangpc]

llvm/lib/TableGen/TGParser.cpp

119–120

The removed code assumed that we can define classes inside a multiclass, so the name of outer multiclass is concatenated to the qualified name. But for current TableGen grammar, we can't define classes in multiclass, so it is unnecessary.

The reason why we need to remove it in this patch is:

1. We can define records, invoke class-based subroutines inside a multiclass.
2. When parsing the name of named arguments in ParseTemplateArgValueList, we need to construct the qualified name to find if there is an argument with that name.
3. CurMultiClass may not be null, so the qualified name woule be like MultiClass::Class:argname, which doesn't exist.
4. Then we fail to find the argument which exists actually.

3161

For the first named argument, we are actually parsing the name value (which is a StringInit) here, so the type doesn't matter.
I set nullptr to ItemType explictly here for non-first named arguments.

3170

When resolving arguments, we can only see Inits and Records. We will lose exact error location information if we move the error checking to later parts.

[wangpc]

clang-format

[wangpc]

Ping.
@reames Are there any other issues I need to fix?

[reames]

LGTM w/one required comment

llvm/lib/TableGen/TGParser.cpp

119–120

Please separate this diff, and commit it separately. Your explanation is sufficient, and you don't need separate review. You should take the time to construct a commit comment which explains which this is fully NFC (without this patch).

[wangpc]

Rebase.

[DavidSpickett]

llvm/lib/TableGen/TGParser.cpp

598

I noticed this working on my Tablegen notebooks, the following example gives the error, but is not itself a multiclass at least not from the user's point of view:

class C <int a, int b> {
    int c = a;
    int d = b;
}
def X: C<0> {}

$ ./bin/llvm-tblgen /tmp/test.td
/tmp/test.td:5:8: error: value not specified for template argument (C:b) of multiclass 'C'
def X: C<0> {}
       ^

Do you have enough information in this context to be able to make the error message more accurate?

[wangpc]

llvm/lib/TableGen/TGParser.cpp

598

Oops, it is my mistake. I will fix it and print accurate location of unspecified argument.

[wangpc]

llvm/lib/TableGen/TGParser.cpp

598

Should be fixed in D156966.

llvm/docs/ReleaseNotes.rst

* A new FatLTO pipeline was added to support generating object files that have
   for more details.
 
 Changes to building LLVM
 ------------------------
 
 Changes to TableGen
 -------------------
 
+* Named arguments are supported. Arguments can be specified in the form of
+  ``name=value``.
+
 Changes to Interprocedural Optimizations
 ----------------------------------------
 
 Changes to the AArch64 Backend
 ------------------------------
 
 * Added Assembly Support for the 2022 A-profile extensions FEAT_GCS (Guarded
   Control Stacks), FEAT_CHK (Check Feature Status), and FEAT_ATS1A.

llvm/docs/TableGen/ProgRef.rst

 * A variable defined with the ``defvar`` or ``defset`` statements.
 
 * The iteration variable of a ``foreach``, such as the use of ``i`` in::
 
      foreach i = 0...5 in
        def Foo#i;
 
 .. productionlist::
-   SimpleValue8: `ClassID` "<" `ValueListNE` ">"
+   SimpleValue8: `ClassID` "<" `ArgValueList` ">"
 
 This form creates a new anonymous record definition (as would be created by an
 unnamed ``def`` inheriting from the given class with the given template
 arguments; see `def`_) and the value is that record. A field of the record can be
 obtained using a suffix; see `Suffixed Values`_.
 
 Invoking a class in this manner can provide a simple subroutine facility.
 See `Using Classes as Subroutines`_ for more information.

 parent classes of the class or record. The record body also
 includes the main body of the definition, which contains the specification
 of the fields of the class or record.
 
 .. productionlist::
    RecordBody: `ParentClassList` `Body`
    ParentClassList: [":" `ParentClassListNE`]
    ParentClassListNE: `ClassRef` ("," `ClassRef`)*
-   ClassRef: (`ClassID` | `MultiClassID`) ["<" [`ValueList`] ">"]
+   ClassRef: (`ClassID` | `MultiClassID`) ["<" [`ArgValueList`] ">"]
+   ArgValueList: `PostionalArgValueList` [","] `NamedArgValueList`
+   PostionalArgValueList: [`Value` {"," `Value`}*]
+   NamedArgValueList: [`NameValue` "=" `Value` {"," `NameValue` "=" `Value`}*]
 
 A :token:`ParentClassList` containing a :token:`MultiClassID` is valid only
 in the class list of a ``defm`` statement. In that case, the ID must be the
 name of a multiclass.
 
+The argument values can be specified in two forms:
+
+* Positional argument (``value``). The value is assigned to the argument in the
+  corresponding position. For ``Foo<a0, a1>``, ``a0`` will be assigned to first
+  argument and ``a1`` will be assigned to second argument.
+* Named argument (``name=value``). The value is assigned to the argument with
+  the specified name. For ``Foo<a=a0, b=a1>``, ``a0`` will be assigned to the
+  argument with name ``a`` and ``a1`` will be assigned to the argument with
+  name ``b``.
+
+Required arguments can alse be specified as named argument.
+
+Note that the argument can only be specified once regardless of the way (named
+or positional) to specify and positional arguments should be put before named
+arguments.
+
 .. productionlist::
    Body: ";" | "{" `BodyItem`* "}"
    BodyItem: (`Type` | "code") `TokIdentifier` ["=" `Value`] ";"
            :| "let" `TokIdentifier` ["{" `RangeList` "}"] "=" `Value` ";"
            :| "defvar" `TokIdentifier` "=" `Value` ";"
            :| `Assert`
 
 A field definition in the body specifies a field to be included in the class

llvm/include/llvm/TableGen/Record.h

 #include <cassert>
 #include <cstddef>
 #include <cstdint>
 #include <map>
 #include <memory>
 #include <optional>
 #include <string>
 #include <utility>
+#include <variant>
 #include <vector>
 
 namespace llvm {
 namespace detail {
 struct RecordKeeperImpl;
 } // namespace detail
 
 class ListRecTy;
enum InitKind : uint8_t {
     IK_AnonymousNameInit,
     IK_StringInit,
     IK_VarInit,
     IK_VarBitInit,
     IK_VarDefInit,
     IK_LastTypedInit,
     IK_UnsetInit,
     IK_ArgumentInit,
   };
 
 private:
   const InitKind Kind;
 
 protected:
   uint8_t Opc; // Used by UnOpInit, BinOpInit, and TernOpInit
 
 private:
public:
 
   bool isConcrete() const override { return true; }
 
   /// Get the string representation of the Init.
   std::string getAsString() const override { return "?"; }
 };
 
 // Represent an argument.
+using ArgAuxType = std::variant<unsigned, Init *>;
 class ArgumentInit : public Init, public FoldingSetNode {
+public:
+  enum Kind {
+    Positional,
+    Named,
+  };
+
+private:
   Init *Value;
+  ArgAuxType Aux;
 
 protected:
-  explicit ArgumentInit(Init *Value) : Init(IK_ArgumentInit), Value(Value) {}
+  explicit ArgumentInit(Init *Value, ArgAuxType Aux)
+      : Init(IK_ArgumentInit), Value(Value), Aux(Aux) {}
 
 public:
   ArgumentInit(const ArgumentInit &) = delete;
   ArgumentInit &operator=(const ArgumentInit &) = delete;
 
   static bool classof(const Init *I) { return I->getKind() == IK_ArgumentInit; }
 
   RecordKeeper &getRecordKeeper() const { return Value->getRecordKeeper(); }
 
-  static ArgumentInit *get(Init *Value);
+  static ArgumentInit *get(Init *Value, ArgAuxType Aux);
+
+  bool isPositional() const { return Aux.index() == Positional; }
+  bool isNamed() const { return Aux.index() == Named; }
 
   Init *getValue() const { return Value; }
+  unsigned getIndex() const {
+    assert(isPositional() && "Should be positional!");
+    return std::get<Positional>(Aux);
+  }
+  Init *getName() const {
+    assert(isNamed() && "Should be named!");
+    return std::get<Named>(Aux);
+  }
+  ArgumentInit *cloneWithValue(Init *Value) const { return get(Value, Aux); }
 
   void Profile(FoldingSetNodeID &ID) const;
 
   Init *resolveReferences(Resolver &R) const override;
-  std::string getAsString() const override { return Value->getAsString(); }
+  std::string getAsString() const override {
+    if (isPositional())
+      return utostr(getIndex()) + ": " + Value->getAsString();
+    if (isNamed())
+      return getName()->getAsString() + ": " + Value->getAsString();
+    llvm_unreachable("Unsupported argument type!");
+    return "";
+  }
 
   bool isComplete() const override { return false; }
   bool isConcrete() const override { return false; }
   Init *getBit(unsigned Bit) const override { return Value->getBit(Bit); }
   Init *getCastTo(RecTy *Ty) const override { return Value->getCastTo(Ty); }
   Init *convertInitializerTo(RecTy *Ty) const override {
     return Value->convertInitializerTo(Ty);
   }
 };
 
 /// 'true'/'false' - Represent a concrete initializer for a bit.
 class BitInit final : public TypedInit {
   friend detail::RecordKeeperImpl;
 
   bool Value;
 
   explicit BitInit(bool V, RecTy *T) : TypedInit(IK_BitInit, T), Value(V) {}
 
 public:

llvm/lib/TableGen/Record.cpp

 Init *UnsetInit::getCastTo(RecTy *Ty) const {
   return const_cast<UnsetInit *>(this);
 }
 
 Init *UnsetInit::convertInitializerTo(RecTy *Ty) const {
   return const_cast<UnsetInit *>(this);
 }
 
-static void ProfileArgumentInit(FoldingSetNodeID &ID, Init *Value) {
+static void ProfileArgumentInit(FoldingSetNodeID &ID, Init *Value,
+                                ArgAuxType Aux) {
+  auto I = Aux.index();
+  ID.AddInteger(I);
+  if (I == ArgumentInit::Positional)
+    ID.AddInteger(std::get<ArgumentInit::Positional>(Aux));
+  if (I == ArgumentInit::Named)
+    ID.AddPointer(std::get<ArgumentInit::Named>(Aux));
   ID.AddPointer(Value);
 }
 
 void ArgumentInit::Profile(FoldingSetNodeID &ID) const {
-  ProfileArgumentInit(ID, Value);
+  ProfileArgumentInit(ID, Value, Aux);
 }
 
-ArgumentInit *ArgumentInit::get(Init *Value) {
+ArgumentInit *ArgumentInit::get(Init *Value, ArgAuxType Aux) {
   FoldingSetNodeID ID;
-  ProfileArgumentInit(ID, Value);
+  ProfileArgumentInit(ID, Value, Aux);
 
   RecordKeeper &RK = Value->getRecordKeeper();
   detail::RecordKeeperImpl &RKImpl = RK.getImpl();
   void *IP = nullptr;
   if (ArgumentInit *I = RKImpl.TheArgumentInitPool.FindNodeOrInsertPos(ID, IP))
     return I;
 
-  ArgumentInit *I = new (RKImpl.Allocator) ArgumentInit(Value);
+  ArgumentInit *I = new (RKImpl.Allocator) ArgumentInit(Value, Aux);
   RKImpl.TheArgumentInitPool.InsertNode(I, IP);
   return I;
 }
 
 Init *ArgumentInit::resolveReferences(Resolver &R) const {
   Init *NewValue = Value->resolveReferences(R);
   if (NewValue != Value)
-    return ArgumentInit::get(NewValue);
+    return cloneWithValue(NewValue);
 
   return const_cast<ArgumentInit *>(this);
 }
 
 BitInit *BitInit::get(RecordKeeper &RK, bool V) {
   return V ? &RK.getImpl().TrueBitInit : &RK.getImpl().FalseBitInit;
 }
 
if (!Def) {
 
     // Copy assertions from class to instance.
     NewRec->appendAssertions(Class);
 
     // Substitute and resolve template arguments
     ArrayRef<Init *> TArgs = Class->getTemplateArgs();
     MapResolver R(NewRec);
 
-    for (unsigned i = 0, e = TArgs.size(); i != e; ++i) {
-      if (i < args_size())
-        R.set(TArgs[i], getArg(i)->getValue());
-      else
-        R.set(TArgs[i], NewRec->getValue(TArgs[i])->getValue());
+    for (unsigned I = 0, E = TArgs.size(); I != E; ++I) {
+      R.set(TArgs[I], NewRec->getValue(TArgs[I])->getValue());
+      NewRec->removeValue(TArgs[I]);
+    }
 
-      NewRec->removeValue(TArgs[i]);
+    for (auto *Arg : args()) {
+      if (Arg->isPositional())
+        R.set(TArgs[Arg->getIndex()], Arg->getValue());
+      if (Arg->isNamed())
+        R.set(Arg->getName(), Arg->getValue());
     }
 
     NewRec->resolveReferences(R);
 
     // Add superclasses.
     ArrayRef<std::pair<Record *, SMRange>> SCs = Class->getSuperClasses();
     for (const auto &SCPair : SCs)
       NewRec->addSuperClass(SCPair.first, SCPair.second);

llvm/lib/TableGen/TGParser.h

private:  // Parser methods.
   bool ParseObjectBody(Record *CurRec);
   bool ParseBody(Record *CurRec);
   bool ParseBodyItem(Record *CurRec);
 
   bool ParseTemplateArgList(Record *CurRec);
   Init *ParseDeclaration(Record *CurRec, bool ParsingTemplateArgs);
   VarInit *ParseForeachDeclaration(Init *&ForeachListValue);
 
   SubClassReference ParseSubClassReference(Record *CurRec, bool isDefm);
   SubMultiClassReference ParseSubMultiClassReference(MultiClass *CurMC);
 
   Init *ParseIDValue(Record *CurRec, StringInit *Name, SMRange NameLoc,
                      IDParseMode Mode = ParseValueMode);
   Init *ParseSimpleValue(Record *CurRec, RecTy *ItemType = nullptr,
                          IDParseMode Mode = ParseValueMode);
   Init *ParseValue(Record *CurRec, RecTy *ItemType = nullptr,
                    IDParseMode Mode = ParseValueMode);
   void ParseValueList(SmallVectorImpl<llvm::Init*> &Result,
                       Record *CurRec, RecTy *ItemType = nullptr);
   bool ParseTemplateArgValueList(SmallVectorImpl<llvm::ArgumentInit *> &Result,
-                                 Record *CurRec, Record *ArgsRec);
+                                 Record *CurRec, Record *ArgsRec,
+                                 bool IsDefm = false);
   void ParseDagArgList(
       SmallVectorImpl<std::pair<llvm::Init*, StringInit*>> &Result,
       Record *CurRec);
   bool ParseOptionalRangeList(SmallVectorImpl<unsigned> &Ranges);
   bool ParseOptionalBitList(SmallVectorImpl<unsigned> &Ranges);
   TypedInit *ParseSliceElement(Record *CurRec);
   TypedInit *ParseSliceElements(Record *CurRec, bool Single = false);
   void ParseRangeList(SmallVectorImpl<unsigned> &Result);

llvm/lib/TableGen/TGParser.cpp

 
 /// Return an Init with a qualifier prefix referring
 /// to CurRec's name.
 static Init *QualifyName(Record &CurRec, MultiClass *CurMultiClass, Init *Name,
                          StringRef Scoper) {
   RecordKeeper &RK = CurRec.getRecords();
   Init *NewName = BinOpInit::getStrConcat(CurRec.getNameInit(),
                                           StringInit::get(RK, Scoper));
   NewName = BinOpInit::getStrConcat(NewName, Name);
 
   if (BinOpInit *BinOp = dyn_cast<BinOpInit>(NewName))
     NewName = BinOp->Fold(&CurRec);
   return NewName;
 }
 
 /// Return the qualified version of the implicit 'NAME' template argument.
 static Init *QualifiedNameOfImplicitName(Record &Rec,
                                          MultiClass *MC = nullptr) {

 }
 
 bool TGParser::resolveArguments(Record *Rec, ArrayRef<ArgumentInit *> ArgValues,
                                 SMLoc Loc, ArgValueHandler ArgValueHandler) {
   ArrayRef<Init *> ArgNames = Rec->getTemplateArgs();
   assert(ArgValues.size() <= ArgNames.size() &&
          "Too many template arguments allowed");
 
-  // Loop over the template argument names. If a value was specified,
-  // handle the (name, value) pair. If not and there was no default, complain.
-  for (unsigned I = 0, E = ArgNames.size(); I != E; ++I) {
-    if (I < ArgValues.size())
-      ArgValueHandler(ArgNames[I], ArgValues[I]->getValue());
-    else {
-      Init *Default = Rec->getValue(ArgNames[I])->getValue();
-      if (!Default->isComplete())
-        return Error(Loc, "Value not specified for template argument '" +
-                              ArgNames[I]->getAsUnquotedString() + "' (#" +
-                              Twine(I) + ") of parent class '" +
-                              Rec->getNameInitAsString() + "'");
-      ArgValueHandler(ArgNames[I], Default);
+  // Loop over the template arguments and handle the (name, value) pair.
+  SmallVector<Init *, 2> UnsolvedArgNames(ArgNames);
+  for (auto *Arg : ArgValues) {
+    Init *ArgName = nullptr;
+    Init *ArgValue = Arg->getValue();
+    if (Arg->isPositional())
+      ArgName = ArgNames[Arg->getIndex()];
+    if (Arg->isNamed())
+      ArgName = Arg->getName();
+
+    // We can only specify the template argument once.
+    if (!is_contained(UnsolvedArgNames, ArgName))
+      return Error(Loc, "We can only specify the template argument '" +
+                            ArgName->getAsUnquotedString() + "' once");
+
+    ArgValueHandler(ArgName, ArgValue);
+    llvm::erase_value(UnsolvedArgNames, ArgName);
+  }
+
+  // For unsolved arguments, if there is no default value, complain.
+  for (auto *UnsolvedArgName : UnsolvedArgNames) {
+    Init *Default = Rec->getValue(UnsolvedArgName)->getValue();
+    if (!Default->isComplete()) {
+      return Error(Loc, "value not specified for template argument (" +
+                            UnsolvedArgName->getAsUnquotedString() +
+                            ") of multiclass '" + Rec->getNameInitAsString() +
+                            "'");
     }
+    ArgValueHandler(UnsolvedArgName, Default);
   }
 
   return false;
 }
 
 /// Resolve the arguments of class and set them to MapResolver.
 /// Returns true if failed.
 bool TGParser::resolveArgumentsOfClass(MapResolver &R, Record *Rec,
                                        ArrayRef<ArgumentInit *> ArgValues,
                                        SMLoc Loc) {
   return resolveArguments(Rec, ArgValues, Loc,
                           [&](Init *Name, Init *Value) { R.set(Name, Value); });
 }
 
 /// Resolve the arguments of multiclass and store them into SubstStack.
 /// Returns true if failed.
 bool TGParser::resolveArgumentsOfMultiClass(SubstStack &Substs, MultiClass *MC,
                                             ArrayRef<ArgumentInit *> ArgValues,
                                             Init *DefmName, SMLoc Loc) {
   // Add an implicit argument NAME.

 static bool isObjectStart(tgtok::TokKind K) {
   return K == tgtok::Assert || K == tgtok::Class || K == tgtok::Def ||
          K == tgtok::Defm || K == tgtok::Defset || K == tgtok::Defvar ||
          K == tgtok::Foreach || K == tgtok::If || K == tgtok::Let ||
          K == tgtok::MultiClass;
 }
 
 bool TGParser::consume(tgtok::TokKind K) {
   if (Lex.getCode() == K) {
     Lex.Lex();
     return true;
   }
   return false;
 }
 
 /// ParseObjectName - If a valid object name is specified, return it. If no
 /// name is specified, return the unset initializer. Return nullptr on parse
MultiClass *TGParser::ParseMultiClassID() {
   Lex.Lex();
   return Result;
 }
 
 /// ParseSubClassReference - Parse a reference to a subclass or a
 /// multiclass. This returns a SubClassRefTy with a null Record* on error.
 ///
 ///  SubClassRef ::= ClassID
-///  SubClassRef ::= ClassID '<' ValueList '>'
+///  SubClassRef ::= ClassID '<' ArgValueList '>'
 ///
 SubClassReference TGParser::
 ParseSubClassReference(Record *CurRec, bool isDefm) {
   SubClassReference Result;
   Result.RefRange.Start = Lex.getLoc();
 
   if (isDefm) {
     if (MultiClass *MC = ParseMultiClassID())
       Result.Rec = &MC->Rec;
   } else {
     Result.Rec = ParseClassID();
   }
   if (!Result.Rec) return Result;
 
   // If there is no template arg list, we're done.
   if (!consume(tgtok::less)) {
     Result.RefRange.End = Lex.getLoc();
     return Result;
   }
 
-  if (ParseTemplateArgValueList(Result.TemplateArgs, CurRec, Result.Rec)) {
+  if (ParseTemplateArgValueList(Result.TemplateArgs, CurRec, Result.Rec,
+                                isDefm)) {
     Result.Rec = nullptr; // Error parsing value list.
     return Result;
   }
 
   if (CheckTemplateArgValues(Result.TemplateArgs, Result.RefRange.Start,
                              Result.Rec)) {
     Result.Rec = nullptr; // Error checking value list.
     return Result;
   }
 
   Result.RefRange.End = Lex.getLoc();
   return Result;
 }
 
 /// ParseSubMultiClassReference - Parse a reference to a subclass or to a
 /// templated submulticlass.  This returns a SubMultiClassRefTy with a null
 /// Record* on error.
 ///
 ///  SubMultiClassRef ::= MultiClassID
-///  SubMultiClassRef ::= MultiClassID '<' ValueList '>'
+///  SubMultiClassRef ::= MultiClassID '<' ArgValueList '>'
 ///
 SubMultiClassReference TGParser::
 ParseSubMultiClassReference(MultiClass *CurMC) {
   SubMultiClassReference Result;
   Result.RefRange.Start = Lex.getLoc();
 
   Result.MC = ParseMultiClassID();
   if (!Result.MC) return Result;
 
   // If there is no template arg list, we're done.
   if (!consume(tgtok::less)) {
     Result.RefRange.End = Lex.getLoc();
     return Result;
   }
 
   if (ParseTemplateArgValueList(Result.TemplateArgs, &CurMC->Rec,
-                                &Result.MC->Rec)) {
+                                &Result.MC->Rec, true)) {
     Result.MC = nullptr; // Error parsing value list.
     return Result;
   }
 
   Result.RefRange.End = Lex.getLoc();
 
   return Result;
 }
case tgtok::CodeFragment:
     break;
   case tgtok::question:
     R = UnsetInit::get(Records);
     Lex.Lex();
     break;
   case tgtok::Id: {
     SMRange NameLoc = Lex.getLocRange();
     StringInit *Name = StringInit::get(Records, Lex.getCurStrVal());
-    if (Lex.Lex() != tgtok::less)  // consume the Id.
-      return ParseIDValue(CurRec, Name, NameLoc, Mode);    // Value ::= IDValue
+    tgtok::TokKind Next = Lex.Lex();
+    if (Next == tgtok::equal) // Named argument.
+      return Name;
+    if (Next != tgtok::less)                            // consume the Id.
+      return ParseIDValue(CurRec, Name, NameLoc, Mode); // Value ::= IDValue
 
-    // Value ::= CLASSID '<' ValueListNE '>' (CLASSID has been consumed)
+    // Value ::= CLASSID '<' ArgValueList '>' (CLASSID has been consumed)
     // This is supposed to synthesize a new anonymous definition, deriving
     // from the class with the template arguments, but no body.
     Record *Class = Records.getClass(Name->getValue());
     if (!Class) {
       Error(NameLoc.Start,
             "Expected a class name, got '" + Name->getValue() + "'");
       return nullptr;
     }
if (!Result.back()) {
       Result.clear();
       return;
     }
   }
 }
 
 // ParseTemplateArgValueList - Parse a template argument list with the syntax
 // shown, filling in the Result vector. The open angle has been consumed.
-// An empty argument list is allowed. Return false if okay, true if an 
+// An empty argument list is allowed. Return false if okay, true if an
 // error was detected.
 //
-//   TemplateArgList ::= '<' [Value {',' Value}*] '>'
+//   ArgValueList ::= '<' PostionalArgValueList [','] NamedArgValueList '>'
+//   PostionalArgValueList ::= [Value {',' Value}*]
+//   NamedArgValueList ::= [NameValue '=' Value {',' NameValue '=' Value}*]
 bool TGParser::ParseTemplateArgValueList(
-    SmallVectorImpl<ArgumentInit *> &Result, Record *CurRec, Record *ArgsRec) {
-
+    SmallVectorImpl<ArgumentInit *> &Result, Record *CurRec, Record *ArgsRec,
+    bool IsDefm) {
   assert(Result.empty() && "Result vector is not empty");
   ArrayRef<Init *> TArgs = ArgsRec->getTemplateArgs();
-  unsigned ArgIndex = 0;
-  RecTy *ItemType;
 
   if (consume(tgtok::greater)) // empty value list
     return false;
 
+  bool HasNamedArg = false;
+  unsigned ArgIndex = 0;
   while (true) {
     if (ArgIndex >= TArgs.size()) {
       TokError("Too many template arguments: " + utostr(ArgIndex + 1));
       return true;
     }
-    const RecordVal *Arg = ArgsRec->getValue(TArgs[ArgIndex]);
-    assert(Arg && "Template argument record not found");
 
-    ItemType = Arg->getType();
-    Init *Value = ParseValue(CurRec, ItemType);
+    SMLoc ValueLoc = Lex.getLoc();
+    // If we are parsing named argument, we don't need to know the argument name
+    // and argument type will be resolved after we know the name.
+    Init *Value = ParseValue(
+        CurRec,
+        HasNamedArg ? nullptr : ArgsRec->getValue(TArgs[ArgIndex])->getType());
     if (!Value)
       return true;
-    Result.push_back(ArgumentInit::get(Value));
+
+    // If we meet '=', then we are parsing named arguments.
+    if (Lex.getCode() == tgtok::equal) {
+      if (!isa<StringInit>(Value))
+        return Error(ValueLoc,
+                     "The name of named argument should be a valid identifier");
+
+      auto *Name = cast<StringInit>(Value);
+      Init *QualifiedName =
+          QualifyName(*ArgsRec, CurMultiClass, Name, IsDefm ? "::" : ":");
+      auto *NamedArg = ArgsRec->getValue(QualifiedName);
+      if (!NamedArg)
+        return Error(ValueLoc,
+                     "Argument " + Name->getAsString() + " doesn't exist");
+
+      Lex.Lex(); // eat the '='.
+      ValueLoc = Lex.getLoc();
+      Value = ParseValue(CurRec, NamedArg->getType());
+      // Named value can't be uninitialized.
+      if (isa<UnsetInit>(Value))
+        return Error(ValueLoc,
+                     "The value of named argument should be initialized, "
+                     "but we got '" +
+                         Value->getAsString() + "'");
+
+      Result.push_back(ArgumentInit::get(Value, QualifiedName));
+      HasNamedArg = true;
+    } else {
+      // Positional arguments should be put before named arguments.
+      if (HasNamedArg)
+        return Error(ValueLoc,
+                     "Positional argument should be put before named argument");
+
+      Result.push_back(ArgumentInit::get(Value, ArgIndex));
+    }
 
     if (consume(tgtok::greater)) // end of argument list?
       return false;
     if (!consume(tgtok::comma))
       return TokError("Expected comma before next argument");
     ++ArgIndex;
   }
 }
 
 /// ParseDeclaration - Read a declaration, returning the name of field ID, or an
 /// empty string on error.  This can happen in a number of different contexts,
 /// including within a def or in the template args for a class (in which case
 /// CurRec will be non-null) and within the template args for a multiclass (in
 /// which case CurRec will be null, but CurMultiClass will be set).  This can
 /// also happen within a def that is within a multiclass, which will set both
 /// CurRec and CurMultiClass.
 ///
 ///  Declaration ::= FIELD? Type ID ('=' Value)?
 ///
 Init *TGParser::ParseDeclaration(Record *CurRec,
                                        bool ParsingTemplateArgs) {
   // Read the field prefix if present.

 // inheritance, multiclass invocation, or anonymous class invocation.
 // If necessary, replace an argument with a cast to the required type.
 // The argument count has already been checked.
 bool TGParser::CheckTemplateArgValues(
     SmallVectorImpl<llvm::ArgumentInit *> &Values, SMLoc Loc, Record *ArgsRec) {
   ArrayRef<Init *> TArgs = ArgsRec->getTemplateArgs();
 
   for (unsigned I = 0, E = Values.size(); I < E; ++I) {
-    RecordVal *Arg = ArgsRec->getValue(TArgs[I]);
-    RecTy *ArgType = Arg->getType();
     auto *Value = Values[I];
+    Init *ArgName = nullptr;
+    if (Value->isPositional())
+      ArgName = TArgs[Value->getIndex()];
+    if (Value->isNamed())
+      ArgName = Value->getName();
+
+    RecordVal *Arg = ArgsRec->getValue(ArgName);
+    RecTy *ArgType = Arg->getType();
 
     if (TypedInit *ArgValue = dyn_cast<TypedInit>(Value->getValue())) {
       auto *CastValue = ArgValue->getCastTo(ArgType);
       if (CastValue) {
         assert((!isa<TypedInit>(CastValue) ||
                 cast<TypedInit>(CastValue)->getType()->typeIsA(ArgType)) &&
                "result of template arg value cast has wrong type");
-        Values[I] = ArgumentInit::get(CastValue);
+        Values[I] = Value->cloneWithValue(CastValue);
       } else {
-        PrintFatalError(Loc,
-                        "Value specified for template argument '" +
-                            Arg->getNameInitAsString() + "' (#" + Twine(I) +
-                            ") is of type " + ArgValue->getType()->getAsString() +
-                            "; expected type " + ArgType->getAsString() + ": " +
-                            ArgValue->getAsString());
+        PrintFatalError(Loc, "Value specified for template argument '" +
+                                 Arg->getNameInitAsString() + "' is of type " +
+                                 ArgValue->getType()->getAsString() +
+                                 "; expected type " + ArgType->getAsString() +
+                                 ": " + ArgValue->getAsString());
       }
     }
   }
 
   return false;
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

llvm/test/TableGen/named-arguments.td

+// RUN: llvm-tblgen %s | FileCheck %s
+// RUN: not llvm-tblgen -DERROR1 %s 2>&1 | FileCheck --check-prefix=ERROR1 %s
+// RUN: not llvm-tblgen -DERROR2 %s 2>&1 | FileCheck --check-prefix=ERROR2 %s
+// RUN: not llvm-tblgen -DERROR3 %s 2>&1 | FileCheck --check-prefix=ERROR3 %s
+// RUN: not llvm-tblgen -DERROR4 %s 2>&1 | FileCheck --check-prefix=ERROR4 %s
+// RUN: not llvm-tblgen -DERROR5 %s 2>&1 | FileCheck --check-prefix=ERROR5 %s
+// RUN: not llvm-tblgen -DERROR6 %s 2>&1 | FileCheck --check-prefix=ERROR6 %s
+// RUN: not llvm-tblgen -DERROR7 %s 2>&1 | FileCheck --check-prefix=ERROR7 %s
+// RUN: not llvm-tblgen -DERROR8 %s 2>&1 | FileCheck --check-prefix=ERROR8 %s
+
+class TestClass<int a, int b = 2, int c = 3> {
+  int value =  !add(a, b, c);
+}
+// CHECK:      def testClass1 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass2 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass3 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass4 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass5 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass6 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass7 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testClass8 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+def testClass1: TestClass<1>;
+def testClass2: TestClass<1, 2>;
+def testClass3: TestClass<1, 2, 3>;
+def testClass4: TestClass<1, b=2>;
+def testClass5: TestClass<1, c=3>;
+def testClass6: TestClass<1, b=2, c=3>;
+def testClass7: TestClass<1, c=3, b=2>;
+def testClass8: TestClass<a=1, c=3, b=2>;
+
+multiclass TestMultiClass<int a, int b = 2, int c = 3> {
+  def "": TestClass<a, b=b, c=c>;
+}
+
+// CHECK:      def testMultiClass1 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass2 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass3 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass4 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass5 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass6 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass7 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+// CHECK:      def testMultiClass8 {
+// CHECK-NEXT:   int value = 6;
+// CHECK-NEXT: }
+defm testMultiClass1: TestMultiClass<1>;
+defm testMultiClass2: TestMultiClass<1, 2>;
+defm testMultiClass3: TestMultiClass<1, 2, 3>;
+defm testMultiClass4: TestMultiClass<1, b=2>;
+defm testMultiClass5: TestMultiClass<1, c=3>;
+defm testMultiClass6: TestMultiClass<1, b=2, c=3>;
+defm testMultiClass7: TestMultiClass<1, c=3, b=2>;
+defm testMultiClass8: TestMultiClass<a=1, b=2, c=3>;
+
+class TestSubroutine<int a, int b=a>{
+  int value=!add(a, b);
+}
+
+// CHECK:      def testSubroutine {
+// CHECK-NEXT:   int value1 = 2;
+// CHECK-NEXT:   int value2 = 2;
+// CHECK-NEXT:   int value3 = 2;
+// CHECK-NEXT: }
+def testSubroutine {
+  int value1=TestSubroutine<1>.value;
+  int value2=TestSubroutine<1, b=1>.value;
+  int value3=TestSubroutine<b=1, a=1>.value;
+}
+
+#ifdef ERROR1
+// ERROR1: Argument "d" doesn't exist
+def testError1: TestClass<1, d=3>;
+#endif
+
+#ifdef ERROR2
+// ERROR2: The name of named argument should be a valid identifier
+def testError2: TestClass<1, 3=0>;
+#endif
+
+#ifdef ERROR3
+// ERROR3: Positional argument should be put before named argument
+def testError3: TestClass<1, b=1, 2>;
+#endif
+
+#ifdef ERROR4
+// ERROR4: The value of named argument should be initialized, but we got '?'
+def testError4: TestClass<1, b=?>;
+#endif
+
+#ifdef ERROR5
+// ERROR5: We can only specify the template argument 'TestClass:a' once
+def testError5: TestClass<1, a=1>;
+#endif
+
+#ifdef ERROR6
+// ERROR6: We can only specify the template argument 'TestMultiClass::a' once
+defm testError6: TestMultiClass<1, a=1>;
+#endif
+
+#ifdef ERROR7
+// ERROR7: We can only specify the template argument 'TestSubroutine:a' once
+def testError7 {
+  int value=TestSubroutine<1, a=1>.value;
+}
+#endif
+
+#ifdef ERROR8
+// ERROR8: We can only specify the template argument 'TestClass:b' once
+def testError8: TestClass<a=1, b=1, b=1>;
+#endif

llvm/test/TableGen/template-args.td

 // CHECK:   string Name = "Alice"
 // CHECK:   string NameName = "AliceAlice"
 
 def Rec1 : Class1<"Alice"> {
   string NameName = Name # Name;
 }
 
 #ifdef ERROR1
-// ERROR1: Value specified for template argument 'Class1:nm' (#0) is of type int
+// ERROR1: Value specified for template argument 'Class1:nm' is of type int
 
 def Rec2 : Class1<42> {
 }
 #endif
 
 class Class2<bits<8> cd> {
   int Code = cd;
 }

 // CHECK:   int Code = 42
 // CHECK:   list<int> CodeList = [42]
 
 def Rec4 : Class2<42> {
   list<int> CodeList = [Code];
 }
 
 #ifdef ERROR2
-// ERROR2: Value specified for template argument 'Class2:cd' (#0) is of type string
+// ERROR2: Value specified for template argument 'Class2:cd' is of type string
 
 def Rec5 : Class2<"oops"> {
   list<int> CodeList = [Code];
 }
 #endif
 
 // Anonymous class instantiation template arguments.
 
 // CHECK: def Rec6
 // CHECK:   string Name = "Ted"
 
 def Rec6 {
   string Name = Class1<"Ted">.Name;
 }
 
 #ifdef ERROR3
-// ERROR3: Value specified for template argument 'Class1:nm' (#0) is of type int
+// ERROR3: Value specified for template argument 'Class1:nm' is of type int
 
 def Rec7 {
   string Name = Class1<42>.Name;
 }
 #endif
 
 // CHECK: def Rec8
 // CHECK:   list<int> CodeList = [42]
 
 def Rec8 {
   list<int> CodeList = [Class2<42>.Code];
 }
 
 #ifdef ERROR4
-// ERROR4: Value specified for template argument 'Class2:cd' (#0) is of type string
+// ERROR4: Value specified for template argument 'Class2:cd' is of type string
 
 def Rec9 {
   list<int> CodeList = [Class2<"huh?">.Code];
 }
 #endif
 
 // Multiclass template arguments.
 

 // CHECK: def RecMC1_1
 // CHECK:   string Name = "Carol"
 // CHECK: def RecMC1_2
 // CHECK:   string NameNmae = "CarolCarol"
 
 defm RecMC1 : MC1<"Carol">;
 
 #ifdef ERROR5
-// ERROR5: Value specified for template argument 'MC1::nm' (#0) is of type int
+// ERROR5: Value specified for template argument 'MC1::nm' is of type int
 
 defm RecMC2 : MC1<42>;
 #endif
 
 multiclass MC2<bits<8> cd> {
   def _1 {
     bits<8> Code = cd;
   }

 // CHECK: def RecMC3_2
 // CHECK:   int Code = 42
 // CHECK: def RecMC3_3
 // CHECK:   list<int> CodeList = [42]
 
 defm RecMC3 : MC2<42>;
 
 #ifdef ERROR6
-// ERROR6: Value specified for template argument 'MC2::cd' (#0) is of type string
+// ERROR6: Value specified for template argument 'MC2::cd' is of type string
 
 defm RecMC4 : MC2<"Bob">;
 #endif
 
 #ifdef ERROR7
 multiclass TwoArgs<bits<8> a, string b> {
   def _1 { bits<8> A = a; }
   def _2 { string B = b; }
 }
 defm Good : TwoArgs<1, "one">;
 defm MissingComma : TwoArgs<2 "two">;
 // ERROR7: [[#@LINE-1]]:31: error: Expected comma before next argument
 #endif