-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[TableGen] Improve handling for dag op names #149248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "TGParser.h" | ||
| #include "TGLexer.h" | ||
| #include "llvm/ADT/SmallVector.h" | ||
| #include "llvm/ADT/StringExtras.h" | ||
| #include "llvm/ADT/Twine.h" | ||
|
|
@@ -1199,6 +1200,7 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| case tgtok::XCast: | ||
| case tgtok::XRepr: | ||
| case tgtok::XGetDagOp: | ||
| case tgtok::XGetDagOpName: | ||
| case tgtok::XInitialized: { // Value ::= !unop '(' Value ')' | ||
| UnOpInit::UnaryOp Code; | ||
| const RecTy *Type = nullptr; | ||
|
|
@@ -1287,6 +1289,11 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| } | ||
| Code = UnOpInit::GETDAGOP; | ||
| break; | ||
| case tgtok::XGetDagOpName: | ||
| Lex.Lex(); // eat the operation | ||
| Type = StringRecTy::get(Records); | ||
| Code = UnOpInit::GETDAGOPNAME; | ||
| break; | ||
| case tgtok::XInitialized: | ||
| Lex.Lex(); // eat the operation | ||
| Code = UnOpInit::INITIALIZED; | ||
|
|
@@ -1514,7 +1521,8 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| case tgtok::XInterleave: | ||
| case tgtok::XGetDagArg: | ||
| case tgtok::XGetDagName: | ||
| case tgtok::XSetDagOp: { // Value ::= !binop '(' Value ',' Value ')' | ||
| case tgtok::XSetDagOp: | ||
| case tgtok::XSetDagOpName: { // Value ::= !binop '(' Value ',' Value ')' | ||
| tgtok::TokKind OpTok = Lex.getCode(); | ||
| SMLoc OpLoc = Lex.getLoc(); | ||
| Lex.Lex(); // eat the operation | ||
|
|
@@ -1547,9 +1555,18 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| case tgtok::XListRemove: | ||
| Code = BinOpInit::LISTREMOVE; | ||
| break; | ||
| case tgtok::XStrConcat: Code = BinOpInit::STRCONCAT; break; | ||
| case tgtok::XInterleave: Code = BinOpInit::INTERLEAVE; break; | ||
| case tgtok::XSetDagOp: Code = BinOpInit::SETDAGOP; break; | ||
| case tgtok::XStrConcat: | ||
|
||
| Code = BinOpInit::STRCONCAT; | ||
| break; | ||
| case tgtok::XInterleave: | ||
| Code = BinOpInit::INTERLEAVE; | ||
| break; | ||
| case tgtok::XSetDagOp: | ||
| Code = BinOpInit::SETDAGOP; | ||
| break; | ||
| case tgtok::XSetDagOpName: | ||
| Code = BinOpInit::SETDAGOPNAME; | ||
| break; | ||
| case tgtok::XGetDagArg: | ||
| Code = BinOpInit::GETDAGARG; | ||
| break; | ||
|
|
@@ -1580,6 +1597,10 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| } | ||
| ArgType = DagRecTy::get(Records); | ||
| break; | ||
| case tgtok::XSetDagOpName: | ||
| Type = DagRecTy::get(Records); | ||
| ArgType = DagRecTy::get(Records); | ||
| break; | ||
| case tgtok::XGetDagName: | ||
| Type = StringRecTy::get(Records); | ||
| ArgType = DagRecTy::get(Records); | ||
|
|
@@ -1773,22 +1794,26 @@ const Init *TGParser::ParseOperation(Record *CurRec, const RecTy *ItemType) { | |
| // Deal with BinOps whose arguments have different types, by | ||
| // rewriting ArgType in between them. | ||
| switch (Code) { | ||
| case BinOpInit::SETDAGOP: | ||
| // After parsing the first dag argument, switch to expecting | ||
| // a record, with no restriction on its superclasses. | ||
| ArgType = RecordRecTy::get(Records, {}); | ||
| break; | ||
| case BinOpInit::GETDAGARG: | ||
| // After parsing the first dag argument, expect an index integer or a | ||
| // name string. | ||
| ArgType = nullptr; | ||
| break; | ||
| case BinOpInit::GETDAGNAME: | ||
| // After parsing the first dag argument, expect an index integer. | ||
| ArgType = IntRecTy::get(Records); | ||
| break; | ||
| default: | ||
| break; | ||
| case BinOpInit::SETDAGOPNAME: | ||
| // After parsing the first dag argument, expect a string. | ||
| ArgType = StringRecTy::get(Records); | ||
| break; | ||
| case BinOpInit::SETDAGOP: | ||
| // After parsing the first dag argument, switch to expecting | ||
| // a record, with no restriction on its superclasses. | ||
| ArgType = RecordRecTy::get(Records, {}); | ||
| break; | ||
| case BinOpInit::GETDAGARG: | ||
| // After parsing the first dag argument, expect an index integer or a | ||
| // name string. | ||
| ArgType = nullptr; | ||
| break; | ||
| case BinOpInit::GETDAGNAME: | ||
| // After parsing the first dag argument, expect an index integer. | ||
| ArgType = IntRecTy::get(Records); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| if (!consume(tgtok::comma)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,12 @@ def test { | |
| dag undefSecond = !con((op 1), (? 2)); | ||
| // CHECK: dag undefBoth = (? 1, 2); | ||
| dag undefBoth = !con((? 1), (? 2)); | ||
| // CHECK: dag namedLHS = (op:$lhs 1, 2); | ||
| dag namedLHS = !con((op:$lhs 1), (op 2)); | ||
| // CHECK: dag namedRHS = (op 1, 2); | ||
|
||
| dag namedRHS = !con((op 1), (op:$rhs 2)); | ||
| // CHECK: dag namedBoth = (op:$lhs 1, 2); | ||
| dag namedBoth = !con((op:$lhs 1), (op:$rhs 2)); | ||
|
|
||
| #ifdef ERROR | ||
| // ERROR: Concatenated Dag operators do not match: '(op 1)' vs. '(otherop 2)' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment below, should we choose the non-empty name if LHS name is empty but LHS not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am certainly not opposed to doing so. I didn't do it initially in order to ensure consistency of behaviour (i.e. always using the name of the LHS regardless of whether it exists). However, perhaps I should favour the LHS name if set and use the RHS name if the LHS name isn't.
I should also perhaps add a note about that behaviour in the documentation. Please let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might be better. Use lhs name if it's not empty else use rhs name. When both names exist, we have a choice of just using lhs, or using lhs if it matches rhs else concat the names with some delimiter. But just something simple like lhs or rhs name seems reasonable.