[TableGen] Improve handling for dag op names #149248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are currently no ways to add names to dag operators other than when defining them. Furthermore a !con operation as well as some others, drops the operator names. This patch propagates the name from the LHS dag for !con and adds a way to get and set the operator name for a dag (!getdagopname, !setdagopname).
Member
|
@llvm/pr-subscribers-tablegen Author: Nemanja Ivanovic (nemanjai) ChangesThere are currently no ways to add names to dag Full diff: https://github.com/llvm/llvm-project/pull/149248.diff 8 Files Affected:
diff --git a/llvm/docs/TableGen/ProgRef.rst b/llvm/docs/TableGen/ProgRef.rst
index 7b30698ce4fd6..1f60803e970fc 100644
--- a/llvm/docs/TableGen/ProgRef.rst
+++ b/llvm/docs/TableGen/ProgRef.rst
@@ -219,17 +219,17 @@ TableGen provides "bang operators" that have a wide variety of uses:
.. productionlist::
BangOperator: one of
- : !add !and !cast !con !dag
- : !div !empty !eq !exists !filter
- : !find !foldl !foreach !ge !getdagarg
- : !getdagname !getdagop !gt !head !if
- : !initialized !instances !interleave !isa !le
- : !listconcat !listflatten !listremove !listsplat !logtwo
- : !lt !match !mul !ne !not
- : !or !range !repr !setdagarg !setdagname
- : !setdagop !shl !size !sra !srl
- : !strconcat !sub !subst !substr !tail
- : !tolower !toupper !xor
+ : !add !and !cast !con !dag
+ : !div !empty !eq !exists !filter
+ : !find !foldl !foreach !ge !getdagarg
+ : !getdagname !getdagop !getdagopname !gt !head
+ : !if !initialized !instances !interleave !isa
+ : !le !listconcat !listflatten !listremove !listsplat
+ : !logtwo !lt !match !mul !ne
+ : !not !or !range !repr !setdagarg
+ : !setdagname !setdagop !setdagopname !shl !size
+ : !sra !srl !strconcat !sub !subst
+ : !substr !tail !tolower !toupper !xor
The ``!cond`` operator has a slightly different
syntax compared to other bang operators, so it is defined separately:
@@ -1443,7 +1443,8 @@ DAG.
The following bang operators are useful for working with DAGs:
``!con``, ``!dag``, ``!empty``, ``!foreach``, ``!getdagarg``, ``!getdagname``,
-``!getdagop``, ``!setdagarg``, ``!setdagname``, ``!setdagop``, ``!size``.
+``!getdagop``, ``!getdagopname``, ``!setdagarg``, ``!setdagname``, ``!setdagop``,
+``!setdagopname``, ``!size``.
Defvar in a record body
-----------------------
@@ -1819,6 +1820,10 @@ and non-0 as true.
dag d = !dag(!getdagop(someDag), args, names);
+``!getdagopname(``\ *dag*\ ``)``
+ This operator retrieves the name of the given *dag* operator. If the operator
+ has no name associated, ``?`` is returned.
+
``!gt(``\ *a*\ `,` *b*\ ``)``
This operator produces 1 if *a* is greater than *b*; 0 otherwise.
The arguments must be ``bit``, ``bits``, ``int``, or ``string`` values.
@@ -1949,6 +1954,10 @@ and non-0 as true.
Example: ``!setdagop((foo 1, 2), bar)`` results in ``(bar 1, 2)``.
+``!setdagopname(``\ *dag*\ ``,``\ *name*\ ``)``
+ This operator produces a DAG node with the same operator and arguments as
+ *dag*, but replacing the name of the operator with *name*.
+
``!shl(``\ *a*\ ``,`` *count*\ ``)``
This operator shifts *a* left logically by *count* bits and produces the resulting
value. The operation is performed on a 64-bit integer; the result
diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index a2b86eb8e7cad..9d67d8b3b9293 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -841,6 +841,7 @@ class UnOpInit final : public OpInit, public FoldingSetNode {
SIZE,
EMPTY,
GETDAGOP,
+ GETDAGOPNAME,
LOG2,
REPR,
LISTFLATTEN,
@@ -910,6 +911,7 @@ class BinOpInit final : public OpInit, public FoldingSetNode {
GETDAGARG,
GETDAGNAME,
SETDAGOP,
+ SETDAGOPNAME
};
private:
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index 1f3e5dc68f1d6..5a1fe87fef52e 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -985,6 +985,12 @@ const Init *UnOpInit::Fold(const Record *CurRec, bool IsFinal) const {
}
break;
+ case GETDAGOPNAME:
+ if (const auto *Dag = dyn_cast<DagInit>(LHS)) {
+ return Dag->getName();
+ }
+ break;
+
case LOG2:
if (const auto *LHSi = dyn_cast_or_null<IntInit>(
LHS->convertInitializerTo(IntRecTy::get(RK)))) {
@@ -1050,6 +1056,9 @@ std::string UnOpInit::getAsString() const {
case SIZE: Result = "!size"; break;
case EMPTY: Result = "!empty"; break;
case GETDAGOP: Result = "!getdagop"; break;
+ case GETDAGOPNAME:
+ Result = "!getdagopname";
+ break;
case LOG2 : Result = "!logtwo"; break;
case LISTFLATTEN:
Result = "!listflatten";
@@ -1310,7 +1319,7 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
SmallVector<std::pair<const Init *, const StringInit *>, 8> Args;
llvm::append_range(Args, LHSs->getArgAndNames());
llvm::append_range(Args, RHSs->getArgAndNames());
- return DagInit::get(Op, Args);
+ return DagInit::get(Op, LHSs->getName(), Args);
}
break;
}
@@ -1508,6 +1517,14 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
return DagInit::get(Op, Dag->getArgs(), Dag->getArgNames());
break;
}
+ case SETDAGOPNAME: {
+ const auto *Dag = dyn_cast<DagInit>(LHS);
+ const auto *Op = dyn_cast<StringInit>(RHS);
+ if (Dag && Op)
+ return DagInit::get(Dag->getOperator(), Op, Dag->getArgs(),
+ Dag->getArgNames());
+ break;
+ }
case ADD:
case SUB:
case MUL:
@@ -1620,6 +1637,9 @@ std::string BinOpInit::getAsString() const {
case STRCONCAT: Result = "!strconcat"; break;
case INTERLEAVE: Result = "!interleave"; break;
case SETDAGOP: Result = "!setdagop"; break;
+ case SETDAGOPNAME:
+ Result = "!setdagopname";
+ break;
case GETDAGARG:
Result = "!getdagarg<" + getType()->getAsString() + ">";
break;
diff --git a/llvm/lib/TableGen/TGLexer.cpp b/llvm/lib/TableGen/TGLexer.cpp
index aea1bb0c6d75e..c369916a48f0d 100644
--- a/llvm/lib/TableGen/TGLexer.cpp
+++ b/llvm/lib/TableGen/TGLexer.cpp
@@ -680,6 +680,8 @@ tgtok::TokKind TGLexer::LexExclaim() {
.Case("find", tgtok::XFind)
.Cases("setdagop", "setop", tgtok::XSetDagOp) // !setop is deprecated.
.Cases("getdagop", "getop", tgtok::XGetDagOp) // !getop is deprecated.
+ .Case("setdagopname", tgtok::XSetDagOpName)
+ .Case("getdagopname", tgtok::XGetDagOpName)
.Case("getdagarg", tgtok::XGetDagArg)
.Case("getdagname", tgtok::XGetDagName)
.Case("setdagarg", tgtok::XSetDagArg)
diff --git a/llvm/lib/TableGen/TGLexer.h b/llvm/lib/TableGen/TGLexer.h
index ed7d8f3baae59..5725e391d0c4d 100644
--- a/llvm/lib/TableGen/TGLexer.h
+++ b/llvm/lib/TableGen/TGLexer.h
@@ -150,6 +150,8 @@ enum TokKind {
XGt,
XSetDagOp,
XGetDagOp,
+ XSetDagOpName,
+ XGetDagOpName,
XExists,
XListRemove,
XToLower,
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 62c5355654149..e347162af64f2 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -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))
diff --git a/llvm/test/TableGen/getsetop.td b/llvm/test/TableGen/getsetop.td
index aac644fe34cb2..031606fc940d7 100644
--- a/llvm/test/TableGen/getsetop.td
+++ b/llvm/test/TableGen/getsetop.td
@@ -28,6 +28,7 @@ def bob : Super;
def test {
dag orig = (foo 1, 2:$a, $b);
dag another = (qux "hello", $world);
+ dag named = (foo:$root 1, 2:$a, $b);
// CHECK: dag replaceWithBar = (bar 1, 2:$a, ?:$b);
dag replaceWithBar = !setop(orig, bar);
@@ -41,6 +42,19 @@ def test {
// CHECK: dag getopToSetop = (foo "hello", ?:$world);
dag getopToSetop = !setdagop(another, !getdagop(orig));
+ // CHECK: dag setOpName = (foo:$baz 1, 2:$a, ?:$b);
+ dag setOpName = !setdagopname(orig, "baz");
+
+ // CHECK: dag getopNameToSetOpName = (foo:$root 1, 2:$a, ?:$b);
+ dag getopNameToSetOpName = !setdagopname(orig, !getdagopname(named));
+
+ // CHECK: dag setOpNameExpl = (foo:$baz 1, 2:$a, ?:$b);
+ dag setOpNameExpl = !setdagopname((foo 1, 2:$a, $b), "baz");
+
+ // CHECK: dag getopNameToSetOpNameExpl = (foo:$root 1, 2:$a, ?:$b);
+ dag getopNameToSetOpNameExpl =
+ !setdagopname(orig, !getdagopname((foo:$root 1, 2:$a, $b)));
+
// CHECK: dag getopToBangDag = (foo 1:$a, 2:$b, 3:$c);
dag getopToBangDag = !dag(!getdagop(orig), [1, 2, 3], ["a", "b", "c"]);
diff --git a/llvm/test/TableGen/unsetop.td b/llvm/test/TableGen/unsetop.td
index 7a4f98aea459c..c8556a6b12d1c 100644
--- a/llvm/test/TableGen/unsetop.td
+++ b/llvm/test/TableGen/unsetop.td
@@ -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)'
|
krzysz00
approved these changes
Jul 17, 2025
Contributor
krzysz00
left a comment
krzysz00
left a comment
There was a problem hiding this comment.
No objections, op seems reasonable, documentation updates good, lgtm pending the tablegen maintainers showing up to complain
Member
Author
|
Thank you. I'll give it a few days in case any objections come up and if not, I'll merge it. |
jurahul
reviewed
Jul 22, 2025
llvm/lib/TableGen/TGParser.cpp
Outdated
| case tgtok::XStrConcat: Code = BinOpInit::STRCONCAT; break; | ||
| case tgtok::XInterleave: Code = BinOpInit::INTERLEAVE; break; | ||
| case tgtok::XSetDagOp: Code = BinOpInit::SETDAGOP; break; | ||
| case tgtok::XStrConcat: |
Contributor
There was a problem hiding this comment.
This looks like unrelated formatting change. Can you undo it? Or did clang-format do it?
Member
Author
There was a problem hiding this comment.
This was done by git-clang-format because I added the tgtok::XSetDagOpName and attempted to format it the same as the rest. I can certainly undo it if you insist though.
Contributor
There was a problem hiding this comment.
Right, if we can undo that would be good. Additionally the new code now is formatted differently. May be we can run clang-format on this file wholesale after this PR goes in.
So for now, let's undo formatting changes to code not touched by this change. Thanks.
jurahul
reviewed
Jul 22, 2025
llvm/test/TableGen/unsetop.td
Outdated
| 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); |
Contributor
There was a problem hiding this comment.
For these 2, should we choose the rhs name if the lhs one is empty?
jurahul
reviewed
Jul 22, 2025
llvm/lib/TableGen/Record.cpp
Outdated
| llvm::append_range(Args, LHSs->getArgAndNames()); | ||
| llvm::append_range(Args, RHSs->getArgAndNames()); | ||
| return DagInit::get(Op, Args); | ||
| return DagInit::get(Op, LHSs->getName(), Args); |
Contributor
There was a problem hiding this comment.
See comment below, should we choose the non-empty name if LHS name is empty but LHS not?
Member
Author
There was a problem hiding this comment.
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.
Contributor
There was a problem hiding this comment.
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.
Contributor
|
Overall looks good, just some minor comments. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Member
Author
|
@jurahul Are you ok with the updates? Should I merge this? |
Contributor
|
Can you fix the clang-format failures shown in CI? I'll take a look at the updates soon |
Member
Author
But the clang-format failures are due to me rejecting clang-format changes as per your suggestion here: #149248 (comment) |
Contributor
|
I see, so it's formatting surrounding code untouched by the PR? In that case, let's go back to the whatever formatting clang-format decides. I'll approve. |
jurahul
approved these changes
Jul 29, 2025
Member
Author
|
Thank you for the review. I'll merge once all the pre-commit checks pass. |
jurahul
added a commit
to jurahul/llvm-project
that referenced
this pull request
Jul 31, 2025
In llvm#149248, clang-format applied some formatting to lines untouched by that PR, because the existing code is not clang-formatted well. Hence applying clang-format on the entire files here.
jurahul
added a commit
that referenced
this pull request
Aug 5, 2025
In #149248, clang-format applied some formatting to lines untouched by that PR, because the existing code is not clang-format compliant. Hence applying clang-format on the entire files here.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 6, 2025
In llvm/llvm-project#149248, clang-format applied some formatting to lines untouched by that PR, because the existing code is not clang-format compliant. Hence applying clang-format on the entire files here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are currently no ways to add names to dag
operators other than when defining them. Furthermore a !con operation as well as some others, drops the operator names.
This patch propagates the name from the LHS dag
for !con and adds a way to get and set the operator name for a dag (!getdagopname, !setdagopname).