[TableGen] Eliminate the dependency on SDNode definition order #168745
Conversation
Fix CodeGenDAGPatterns::ParseDefaultOperands() not to depend on the
first SDNode defined being usable as an output pattern operator.
Presently, each `OperandWithDefaultOps` record is processed by
constructing an instance of TreePattern from its `DefaultOps` argument
that has the form `(ops ...)`. Nevertheless the result of processing
the root operator of that DAG is not inspected by `ParseDefaultOperands()`
function itself, that operator has to be parseable by the underlying
`TreePattern::ParseTreePattern()` function. For that reason, a temporary
DAG is created by replacing the root operator of `DefaultOps` argument
from `ops` to the first SDNode defined, which is usually `def imm : ...`
defined in `TargetSelectionDAG.td` file.
This results in misleading errors being reported when defining new
`SDNode` types, if the new definition happens to be added before the
`def imm : ...` line. The error is reported by several test cases
executed by `check-llvm` target, as well as if one of the targets
being built inherits its operand type from `OperandWithDefaultOps`:
OptionalIntOperand: ../llvm/test/TableGen/DAGDefaultOps.td:28:5: error: In OptionalIntOperand: Cannot use 'unexpected_node' in an output pattern!
def OptionalIntOperand: OperandWithDefaultOps<i32, (ops (i32 0))>;
This commit implements a dedicated constructor of `TreePattern` to be
used if the caller does not care about the particular root operator of
the pattern being processed.
|
The example error output was produced by defining diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td
index a9750a5ab03f..1143b749f6e1 100644
--- a/llvm/include/llvm/Target/TargetSelectionDAG.td
+++ b/llvm/include/llvm/Target/TargetSelectionDAG.td
@@ -371,6 +371,8 @@ def set;
def node;
def srcvalue;
+def unexpected_node : SDNode<"ISD::Unexpected", SDTIntShiftOp, []>;
+
def imm : SDNode<"ISD::Constant" , SDTIntLeaf , [], "ConstantSDNode">;
def timm : SDNode<"ISD::TargetConstant",SDTIntLeaf, [], "ConstantSDNode">;
def fpimm : SDNode<"ISD::ConstantFP", SDTFPLeaf , [], "ConstantFPSDNode">;Unfortunately, it is non-trivial to add a test case, as it is impossible to put |
🐧 Linux x64 Test Results
|
|
@llvm/pr-subscribers-tablegen Author: Anatoly Trosinenko (atrosinenko) ChangesFix the dependency of Presently, each This results in misleading errors being reported when implementing new This commit implements a dedicated constructor of Full diff: https://github.com/llvm/llvm-project/pull/168745.diff 2 Files Affected:
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index 34355d5d6b743..6829a4dd1d155 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -2923,6 +2923,14 @@ TreePattern::TreePattern(const Record *TheRec, const DagInit *Pat, bool isInput,
Trees.push_back(ParseTreePattern(Pat, ""));
}
+TreePattern::TreePattern(const Record *TheRec, ArrayRef<const Init *> Args,
+ ArrayRef<const StringInit *> ArgNames, bool isInput,
+ CodeGenDAGPatterns &cdp)
+ : TheRecord(TheRec), CDP(cdp), isInputPattern(isInput), HasError(false),
+ Infer(*this) {
+ Trees.push_back(ParseRootlessTreePattern(Args, ArgNames));
+}
+
TreePattern::TreePattern(const Record *TheRec, TreePatternNodePtr Pat,
bool isInput, CodeGenDAGPatterns &cdp)
: TheRecord(TheRec), CDP(cdp), isInputPattern(isInput), HasError(false),
@@ -2951,6 +2959,19 @@ void TreePattern::ComputeNamedNodes(TreePatternNode &N) {
ComputeNamedNodes(Child);
}
+TreePatternNodePtr
+TreePattern::ParseRootlessTreePattern(ArrayRef<const Init *> Args,
+ ArrayRef<const StringInit *> ArgNames) {
+ std::vector<TreePatternNodePtr> Children;
+
+ for (auto [Arg, ArgName] : llvm::zip_equal(Args, ArgNames)) {
+ StringRef NameStr = ArgName ? ArgName->getValue() : "";
+ Children.push_back(ParseTreePattern(Arg, NameStr));
+ }
+
+ return makeIntrusiveRefCnt<TreePatternNode>(nullptr, std::move(Children), 1);
+}
+
TreePatternNodePtr TreePattern::ParseTreePattern(const Init *TheInit,
StringRef OpName) {
RecordKeeper &RK = TheInit->getRecordKeeper();
@@ -3488,20 +3509,12 @@ void CodeGenDAGPatterns::ParseDefaultOperands() {
ArrayRef<const Record *> DefaultOps =
Records.getAllDerivedDefinitions("OperandWithDefaultOps");
- // Find some SDNode.
- assert(!SDNodes.empty() && "No SDNodes parsed?");
- const Init *SomeSDNode = SDNodes.begin()->first->getDefInit();
-
for (unsigned i = 0, e = DefaultOps.size(); i != e; ++i) {
const DagInit *DefaultInfo = DefaultOps[i]->getValueAsDag("DefaultOps");
- // Clone the DefaultInfo dag node, changing the operator from 'ops' to
- // SomeSDnode so that we can parse this.
- const DagInit *DI = DagInit::get(SomeSDNode, DefaultInfo->getArgs(),
- DefaultInfo->getArgNames());
-
// Create a TreePattern to parse this.
- TreePattern P(DefaultOps[i], DI, false, *this);
+ TreePattern P(DefaultOps[i], DefaultInfo->getArgs(),
+ DefaultInfo->getArgNames(), false, *this);
assert(P.getNumTrees() == 1 && "This ctor can only produce one tree!");
// Copy the operands over into a DAGDefaultOperand.
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
index aa9a0a442424d..f69a606a84062 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.h
@@ -925,6 +925,9 @@ class TreePattern {
CodeGenDAGPatterns &ise);
TreePattern(const Record *TheRec, const DagInit *Pat, bool isInput,
CodeGenDAGPatterns &ise);
+ TreePattern(const Record *TheRec, ArrayRef<const Init *> Args,
+ ArrayRef<const StringInit *> ArgNames, bool isInput,
+ CodeGenDAGPatterns &ise);
TreePattern(const Record *TheRec, TreePatternNodePtr Pat, bool isInput,
CodeGenDAGPatterns &ise);
@@ -989,6 +992,9 @@ class TreePattern {
private:
TreePatternNodePtr ParseTreePattern(const Init *DI, StringRef OpName);
+ TreePatternNodePtr
+ ParseRootlessTreePattern(ArrayRef<const Init *> Args,
+ ArrayRef<const StringInit *> ArgNames);
void ComputeNamedNodes();
void ComputeNamedNodes(TreePatternNode &N);
};
|
Fix the dependency of
CodeGenDAGPatterns::ParseDefaultOperands()on the particular order of SDNode definitions. Implicit usage of the first definition as a placeholder makesllvm-tblgen -gen-dag-iselfail if that SDNode is not usable as an output pattern operator and an instance ofOperandWithDefaultOpsis used in a pattern.Presently, each
OperandWithDefaultOpsrecord is processed by constructing an instance of TreePattern from itsDefaultOpsargument that has the form(ops ...). Even though the result of processing the root operator of that DAG is not inspected byParseDefaultOperands()function itself, that operator has to be supported by the underlyingTreePattern::ParseTreePattern()function. For that reason, a temporary DAG is created by replacing the root operator ofDefaultOpsargument with the first SDNode defined, which is usuallydef imm : ...defined inTargetSelectionDAG.tdfile.This results in misleading errors being reported when implementing new
SDNodetypes, if the new definition happens to be added before thedef imm : ...line. The error is reported by several test cases executed bycheck-llvmtarget, as well as by the regular build, if one of the enabled targets inherit one of its operand types fromOperandWithDefaultOps:This commit implements a dedicated constructor of
TreePatternto be used if the caller does not care about the particular root operator of the pattern being processed.