[TableGen] Support for optional chain in Selection DAG nodes #163079
+246
−56
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
This change adds a new property for Selection DAG nodes used in pattern
descriptions: SDNPMayHaveChain. A node with this property may have or
may not have a chain operand. For example, both of the following
variants become valid:
t3: f32,ch = fnearbyint t0, t2
t3: f32 = fnearbyint t2
The specific variant is determined during pattern matching, based on
whether the first operand is a chain (i.e. has the type MVT::Other).
This feature is intended to be used for floating point operations. They
have side effects in a strictfp environment and are pure functions in
the default FP environment. Currently each such operation requires two
opcodes - one for each kind of FP environment. These opcodes represent
the same operation and are processed similarly, which increase amount of
code. With this feature the support of strictfp environment should be
easier, as it can use the same opcode as the default environment.
|
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-tablegen Author: Serge Pavlov (spavloff) ChangesThis change adds a new property for Selection DAG nodes used in pattern descriptions: SDNPMayHaveChain. A node with this property may have or may not have a chain operand. For example, both of the following variants become valid: The specific variant is determined during pattern matching, based on whether the first operand is a chain (i.e. has the type MVT::Other). This feature is intended to be used for floating point operations. They have side effects in a strictfp environment and are pure functions in the default FP environment. Currently each such operation requires two opcodes - one for each kind of FP environment. These opcodes represent the same operation and are processed similarly, which increase amount of code. With this feature the support of strictfp environment should be easier, as it can use the same opcode as the default environment. Patch is 36.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163079.diff 19 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/SDNodeInfo.h b/llvm/include/llvm/CodeGen/SDNodeInfo.h
index ba6c343ee1838..41154b1d50445 100644
--- a/llvm/include/llvm/CodeGen/SDNodeInfo.h
+++ b/llvm/include/llvm/CodeGen/SDNodeInfo.h
@@ -26,6 +26,7 @@ enum SDNP {
SDNPOptInGlue,
SDNPMemOperand,
SDNPVariadic,
+ SDNPMayHaveChain
};
enum SDTC : uint8_t {
diff --git a/llvm/include/llvm/CodeGen/SDNodeProperties.td b/llvm/include/llvm/CodeGen/SDNodeProperties.td
index d32904283a11a..640cdd08b9b7d 100644
--- a/llvm/include/llvm/CodeGen/SDNodeProperties.td
+++ b/llvm/include/llvm/CodeGen/SDNodeProperties.td
@@ -29,3 +29,4 @@ def SDNPMayLoad : SDNodeProperty; // May read memory, sets 'mayLoad'.
def SDNPSideEffect : SDNodeProperty; // Sets 'HasUnmodelledSideEffects'.
def SDNPMemOperand : SDNodeProperty; // Touches memory, has assoc MemOperand
def SDNPVariadic : SDNodeProperty; // Node has variable arguments.
+def SDNPMayHaveChain: SDNodeProperty; // Optionally has chain operand/result.
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index 5241a51dd8cd8..c57f18368b2ee 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -151,6 +151,7 @@ class SelectionDAGISel {
OPC_RecordChild6,
OPC_RecordChild7,
OPC_RecordMemRef,
+ OPC_RecordOptionalChain,
OPC_CaptureGlueInput,
OPC_MoveChild,
OPC_MoveChild0,
@@ -493,7 +494,8 @@ class SelectionDAGISel {
private:
void DoInstructionSelection();
SDNode *MorphNode(SDNode *Node, unsigned TargetOpc, SDVTList VTList,
- ArrayRef<SDValue> Ops, unsigned EmitNodeInfo);
+ ArrayRef<SDValue> Ops, unsigned EmitNodeInfo,
+ bool OptionalChain);
/// Prepares the landing pad to take incoming values or do other EH
/// personality specific tasks. Returns true if the block should be
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 116911699ab9f..019747136df5b 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -717,6 +717,7 @@ END_TWO_BYTE_PACK()
case ISD::STRICT_FP_TO_FP16:
case ISD::STRICT_BF16_TO_FP:
case ISD::STRICT_FP_TO_BF16:
+#define FP_OPERATION(NAME, NARG, ROUND_MODE, INTRINSIC, DAGN)
#define DAG_INSTRUCTION(NAME, NARG, ROUND_MODE, INTRINSIC, DAGN) \
case ISD::STRICT_##DAGN:
#include "llvm/IR/ConstrainedOps.def"
@@ -724,6 +725,23 @@ END_TWO_BYTE_PACK()
}
}
+ /// Test if this node is a floating-point operation which can exist in two
+ /// forms, - with chain or without it.
+ bool isFPOperation() const {
+ switch (NodeType) {
+ default:
+ return false;
+#define FP_OPERATION(NAME, NARG, ROUND_MODE, INTRINSIC, DAGN) case ISD::DAGN:
+#include "llvm/IR/ConstrainedOps.def"
+ return true;
+ }
+ }
+
+ /// Test if this node has an input chain.
+ bool hasChain() const {
+ return NumOperands > 0 && OperandList[0].getValueType() == MVT::Other;
+ }
+
/// Test if this node is an assert operation.
bool isAssert() const {
switch (NodeType) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SDNodeInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/SDNodeInfo.cpp
index e3f6c98a9a90a..25f147b3b1ba9 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SDNodeInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SDNodeInfo.cpp
@@ -47,6 +47,16 @@ void SDNodeInfo::verifyNode(const SelectionDAG &DAG, const SDNode *N) const {
bool HasInGlue = Desc.hasProperty(SDNPInGlue);
bool HasOptInGlue = Desc.hasProperty(SDNPOptInGlue);
bool IsVariadic = Desc.hasProperty(SDNPVariadic);
+ bool MayHaveChain = Desc.hasProperty(SDNPMayHaveChain);
+
+ if (HasChain && MayHaveChain)
+ reportNodeError(
+ DAG, N, "Flags 'HasChain' and 'MayHaveChain' cannot be both specified");
+
+ if (MayHaveChain && N->getNumOperands() > 0 &&
+ N->getOperand(0).getValueType() == MVT::Other) {
+ HasChain = true;
+ }
unsigned ActualNumResults = N->getNumValues();
unsigned ExpectedNumResults = Desc.NumResults + HasChain + HasOutGlue;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 6c11c5b815b6b..7db90800792d7 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -2836,9 +2836,9 @@ HandleMergeInputChains(SmallVectorImpl<SDNode*> &ChainNodesMatched,
}
/// MorphNode - Handle morphing a node in place for the selector.
-SDNode *SelectionDAGISel::
-MorphNode(SDNode *Node, unsigned TargetOpc, SDVTList VTList,
- ArrayRef<SDValue> Ops, unsigned EmitNodeInfo) {
+SDNode *SelectionDAGISel::MorphNode(SDNode *Node, unsigned TargetOpc,
+ SDVTList VTList, ArrayRef<SDValue> Ops,
+ unsigned EmitNodeInfo, bool OptionalChain) {
// It is possible we're using MorphNodeTo to replace a node with no
// normal results with one that has a normal result (or we could be
// adding a chain) and the input could have glue and chains as well.
@@ -2880,7 +2880,7 @@ MorphNode(SDNode *Node, unsigned TargetOpc, SDVTList VTList,
--ResNumResults;
// Move the chain reference if needed.
- if ((EmitNodeInfo & OPFL_Chain) && OldChainResultNo != -1 &&
+ if ((EmitNodeInfo & OPFL_Chain || OptionalChain) && OldChainResultNo != -1 &&
static_cast<unsigned>(OldChainResultNo) != ResNumResults - 1)
ReplaceUses(SDValue(Node, OldChainResultNo),
SDValue(Res, ResNumResults - 1));
@@ -3385,6 +3385,12 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
// update the chain results when the pattern is complete.
SmallVector<SDNode*, 3> ChainNodesMatched;
+ bool HasNodesWithOptionalChain = false;
+
+ // List of pattern matches nodes that may have input/output chains and
+ // actually have them.
+ SmallVector<SDNode *, 8> OptionalChainNodes;
+
LLVM_DEBUG(dbgs() << "ISEL: Starting pattern match\n");
// Determine where to start the interpreter. Normally we start at opcode #0,
@@ -3505,7 +3511,8 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
case OPC_RecordChild2: case OPC_RecordChild3:
case OPC_RecordChild4: case OPC_RecordChild5:
case OPC_RecordChild6: case OPC_RecordChild7: {
- unsigned ChildNo = Opcode-OPC_RecordChild0;
+ unsigned ChildNo =
+ Opcode - OPC_RecordChild0 + !OptionalChainNodes.empty();
if (ChildNo >= N.getNumOperands())
break; // Match fails if out of range child #.
@@ -3523,6 +3530,17 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
continue;
+ case OPC_RecordOptionalChain:
+ HasNodesWithOptionalChain = true;
+ // If the current node has input chain, record it.
+ if (N->getNumOperands() != 0) {
+ SDValue FirstOperand = N->getOperand(0);
+ if (FirstOperand.getValueType() == MVT::Other) {
+ OptionalChainNodes.push_back(N.getNode());
+ }
+ }
+ continue;
+
case OPC_CaptureGlueInput:
// If the current node has an input glue, capture it in InputGlue.
if (N->getNumOperands() != 0 &&
@@ -3531,7 +3549,8 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
continue;
case OPC_MoveChild: {
- unsigned ChildNo = MatcherTable[MatcherIndex++];
+ unsigned ChildNo =
+ MatcherTable[MatcherIndex++] + !OptionalChainNodes.empty();
if (ChildNo >= N.getNumOperands())
break; // Match fails if out of range child #.
N = N.getOperand(ChildNo);
@@ -3543,7 +3562,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
case OPC_MoveChild2: case OPC_MoveChild3:
case OPC_MoveChild4: case OPC_MoveChild5:
case OPC_MoveChild6: case OPC_MoveChild7: {
- unsigned ChildNo = Opcode-OPC_MoveChild0;
+ unsigned ChildNo = Opcode - OPC_MoveChild0 + !OptionalChainNodes.empty();
if (ChildNo >= N.getNumOperands())
break; // Match fails if out of range child #.
N = N.getOperand(ChildNo);
@@ -3568,6 +3587,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
unsigned SiblingNo = Opcode == OPC_MoveSibling
? MatcherTable[MatcherIndex++]
: Opcode - OPC_MoveSibling0;
+ SiblingNo += !OptionalChainNodes.empty();
if (SiblingNo >= N.getNumOperands())
break; // Match fails if out of range sibling #.
N = N.getOperand(SiblingNo);
@@ -3998,11 +4018,16 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
// Ignore these because the newly token factored chain should not refer to
// the old nodes.
unsigned NumChains = MatcherTable[MatcherIndex++];
- assert(NumChains != 0 && "Can't TF zero chains");
+ assert((NumChains != 0 || HasNodesWithOptionalChain) &&
+ "Can't TF zero chains");
assert(ChainNodesMatched.empty() &&
"Should only have one EmitMergeInputChains per match");
+ if (NumChains == 0 && HasNodesWithOptionalChain &&
+ OptionalChainNodes.empty())
+ continue;
+
// Read all of the chained nodes.
for (unsigned i = 0; i != NumChains; ++i) {
unsigned RecNo = MatcherTable[MatcherIndex++];
@@ -4020,6 +4045,8 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
}
}
+ ChainNodesMatched.append(OptionalChainNodes);
+
// If the inner loop broke out, the match fails.
if (ChainNodesMatched.empty())
break;
@@ -4164,7 +4191,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
VTs.push_back(VT);
}
- if (EmitNodeInfo & OPFL_Chain)
+ if (EmitNodeInfo & OPFL_Chain || !OptionalChainNodes.empty())
VTs.push_back(MVT::Other);
if (EmitNodeInfo & OPFL_GlueOutput)
VTs.push_back(MVT::Glue);
@@ -4186,6 +4213,10 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
unsigned RecNo = MatcherTable[MatcherIndex++];
if (RecNo & 128)
RecNo = GetVBR(RecNo, MatcherTable, MatcherIndex);
+ if (HasNodesWithOptionalChain) {
+ assert(RecNo > 0);
+ RecNo--;
+ }
assert(RecNo < RecordedNodes.size() && "Invalid EmitNode");
Ops.push_back(RecordedNodes[RecNo].first);
@@ -4209,7 +4240,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
}
// If this has chain/glue inputs, add them.
- if (EmitNodeInfo & OPFL_Chain)
+ if (EmitNodeInfo & OPFL_Chain || !OptionalChainNodes.empty())
Ops.push_back(InputChain);
if ((EmitNodeInfo & OPFL_GlueInput) && InputGlue.getNode() != nullptr)
Ops.push_back(InputGlue);
@@ -4219,7 +4250,12 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
// We need to perform this check before potentially modifying one of the
// nodes via MorphNode.
bool MayRaiseFPException =
- llvm::any_of(ChainNodesMatched, [this](SDNode *N) {
+ llvm::any_of(ChainNodesMatched,
+ [this](SDNode *N) {
+ return mayRaiseFPException(N) &&
+ !N->getFlags().hasNoFPExcept();
+ }) ||
+ llvm::any_of(OptionalChainNodes, [this](SDNode *N) {
return mayRaiseFPException(N) && !N->getFlags().hasNoFPExcept();
});
@@ -4251,8 +4287,9 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
"Chain node replaced during MorphNode");
llvm::erase(Chain, N);
});
- Res = cast<MachineSDNode>(MorphNode(NodeToMatch, TargetOpc, VTList,
- Ops, EmitNodeInfo));
+ Res = cast<MachineSDNode>(MorphNode(NodeToMatch, TargetOpc, VTList, Ops,
+ EmitNodeInfo,
+ !OptionalChainNodes.empty()));
}
// Set the NoFPExcept flag when no original matched node could
@@ -4264,9 +4301,9 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
// chain and glue.
if (EmitNodeInfo & OPFL_GlueOutput) {
InputGlue = SDValue(Res, VTs.size()-1);
- if (EmitNodeInfo & OPFL_Chain)
+ if (EmitNodeInfo & OPFL_Chain || !OptionalChainNodes.empty())
InputChain = SDValue(Res, VTs.size()-2);
- } else if (EmitNodeInfo & OPFL_Chain)
+ } else if (EmitNodeInfo & OPFL_Chain || !OptionalChainNodes.empty())
InputChain = SDValue(Res, VTs.size()-1);
// If the OPFL_MemRefs glue is set on this node, slap all of the
@@ -4430,7 +4467,7 @@ bool SelectionDAGISel::mayRaiseFPException(SDNode *N) const {
const SelectionDAGTargetInfo &TSI = CurDAG->getSelectionDAGInfo();
return TSI.mayRaiseFPException(N->getOpcode());
}
- return N->isStrictFPOpcode();
+ return N->isStrictFPOpcode() || N->isFPOperation();
}
bool SelectionDAGISel::isOrEquivalentToAdd(const SDNode *N) const {
diff --git a/llvm/test/TableGen/SDNodeInfoEmitter/advanced.td b/llvm/test/TableGen/SDNodeInfoEmitter/advanced.td
index d7eeaba9d8552..b76f98f38613d 100644
--- a/llvm/test/TableGen/SDNodeInfoEmitter/advanced.td
+++ b/llvm/test/TableGen/SDNodeInfoEmitter/advanced.td
@@ -46,15 +46,22 @@ def my_node_3 : SDNode<
SDNPOutGlue, SDNPInGlue, SDNPOptInGlue]
>;
+def my_node_4 : SDNode<
+ "MyTargetISD::NODE_4",
+ SDTypeProfile<1, 1, [SDTCisVT<0, f32>, SDTCisVT<1, f32>]>,
+ [SDNPMayHaveChain]
+>;
+
// CHECK: namespace llvm::MyTargetISD {
// CHECK-EMPTY:
// CHECK-NEXT: enum GenNodeType : unsigned {
// CHECK-NEXT: NODE_1 = ISD::BUILTIN_OP_END,
// CHECK-NEXT: NODE_2,
// CHECK-NEXT: NODE_3,
+// CHECK-NEXT: NODE_4
// CHECK-NEXT: };
// CHECK-EMPTY:
-// CHECK-NEXT: static constexpr unsigned GENERATED_OPCODE_END = NODE_3 + 1;
+// CHECK-NEXT: static constexpr unsigned GENERATED_OPCODE_END = NODE_4 + 1;
// CHECK-EMPTY:
// CHECK-NEXT: } // namespace llvm::MyTargetISD
@@ -63,6 +70,7 @@ def my_node_3 : SDNode<
// CHECK-NEXT: "MyTargetISD::NODE_1\0"
// CHECK-NEXT: "MyTargetISD::NODE_2\0"
// CHECK-NEXT: "MyTargetISD::NODE_3\0"
+// CHECK-NEXT: "MyTargetISD::NODE_4\0"
// CHECK-NEXT: ;
// CHECK: static const SDTypeConstraint MyTargetSDTypeConstraints[] = {
@@ -81,14 +89,16 @@ def my_node_3 : SDNode<
// CHECK-SAME: {SDTCisInt, 2, 0, MVT::INVALID_SIMPLE_VALUE_TYPE},
// CHECK-SAME: {SDTCisPtrTy, 1, 0, MVT::INVALID_SIMPLE_VALUE_TYPE},
// CHECK-SAME: {SDTCisVT, 0, 0, MVT::i1},
+// CHECK-NEXT: /* 15 */ {SDTCisVT, 1, 0, MVT::f32}, {SDTCisVT, 0, 0, MVT::f32},
// CHECK-NEXT: };
// CHECK-EMPTY:
// CHECK-NEXT: static const SDNodeDesc MyTargetSDNodeDescs[] = {
// CHECK-NEXT: {1, 1, 0|1<<SDNPHasChain, 0, 0, 1, 0, 2}, // NODE_1
// CHECK-NEXT: {3, 1, 0|1<<SDNPVariadic|1<<SDNPMemOperand, 0, 42, 21, 11, 4}, // NODE_2
// CHECK-NEXT: {2, -1, 0|1<<SDNPHasChain|1<<SDNPOutGlue|1<<SDNPInGlue|1<<SDNPOptInGlue, 0|1<<SDNFIsStrictFP, 24, 41, 2, 13}, // NODE_3
+// CHECK-NEXT: {1, 1, 0|1<<SDNPMayHaveChain, 0, 0, 61, 15, 2}, // NODE_4
// CHECK-NEXT: };
// CHECK-EMPTY:
// CHECK-NEXT: static const SDNodeInfo MyTargetGenSDNodeInfo(
-// CHECK-NEXT: /*NumOpcodes=*/3, MyTargetSDNodeDescs,
+// CHECK-NEXT: /*NumOpcodes=*/4, MyTargetSDNodeDescs,
// CHECK-NEXT: MyTargetSDNodeNames, MyTargetSDTypeConstraints);
diff --git a/llvm/test/TableGen/optional-chain.td b/llvm/test/TableGen/optional-chain.td
new file mode 100644
index 0000000000000..9905a8f585a20
--- /dev/null
+++ b/llvm/test/TableGen/optional-chain.td
@@ -0,0 +1,46 @@
+// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include %s 2>&1 | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def TestTargetInstrInfo : InstrInfo;
+
+
+def TestTarget : Target {
+ let InstructionSet = TestTargetInstrInfo;
+}
+
+def R0 : Register<"r0"> { let Namespace = "MyTarget"; }
+def GPR : RegisterClass<"MyTarget", [i32, f32], 32, (add R0)>;
+
+def a_nearbyint : SDNode<
+ "MyTargetISD::A_NEARBYINT",
+ SDTypeProfile<1, 1, [SDTCisFP<0>, SDTCisFP<1>]>,
+ [SDNPMayHaveChain]
+>;
+
+def I_NEARBYINT : Instruction {
+ let OutOperandList = (outs GPR:$out);
+ let InOperandList = (ins GPR:$x);
+}
+
+def : Pat<(a_nearbyint GPR:$x), (I_NEARBYINT GPR:$x)>;
+def : Pat<(a_nearbyint (a_nearbyint GPR:$x)), (I_NEARBYINT GPR:$x)>;
+
+// CHECK-LABEL: OPC_CheckOpcode, TARGET_VAL(MyTargetISD::A_NEARBYINT),
+// CHECK-NEXT: OPC_RecordOptionalChain,
+// CHECK-NEXT: OPC_Scope, 17
+// CHECK-NEXT: OPC_MoveChild0,
+// CHECK-NEXT: OPC_CheckOpcode, TARGET_VAL(MyTargetISD::A_NEARBYINT),
+// CHECK-NEXT: OPC_RecordOptionalChain,
+// CHECK-NEXT: OPC_CheckFoldableChainNode,
+// CHECK-NEXT: OPC_RecordChild0,
+// CHECK-NEXT: OPC_MoveParent,
+// CHECK-NEXT: OPC_EmitMergeInputChains, 0,
+// CHECK-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::I_NEARBYINT), 0,
+// CHECK-NEXT: /*MVT::f32*/12, 1/*#Ops*/, 2,
+
+// CHECK: /*Scope*/ 10,
+// CHECK-NEXT: OPC_RecordChild0,
+// CHECK-NEXT: OPC_EmitMergeInputChains, 0,
+// CHECK-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::I_NEARBYINT), 0,
+// CHECK-NEXT: /*MVT::f32*/12, 1/*#Ops*/, 1,
diff --git a/llvm/utils/TableGen/Basic/SDNodeProperties.cpp b/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
index 46babbbc41944..9a37db6d19b19 100644
--- a/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
+++ b/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
@@ -28,6 +28,7 @@ unsigned llvm::parseSDPatternOperatorProperties(const Record *R) {
.Case("SDNPSideEffect", SDNPSideEffect)
.Case("SDNPMemOperand", SDNPMemOperand)
.Case("SDNPVariadic", SDNPVariadic)
+ .Case("SDNPMayHaveChain", SDNPMayHaveChain)
.Default(-1u);
if (Offset != -1u)
Properties |= 1 << Offset;
diff --git a/llvm/utils/TableGen/Basic/SDNodeProperties.h b/llvm/utils/TableGen/Basic/SDNodeProperties.h
index 97813067341fc..516f45e118b36 100644
--- a/llvm/utils/TableGen/Basic/SDNodeProperties.h
+++ b/llvm/utils/TableGen/Basic/SDNodeProperties.h
@@ -28,6 +28,7 @@ enum SDNP {
SDNPSideEffect,
SDNPMemOperand,
SDNPVariadic,
+ SDNPMayHaveChain
};
unsigned parseSDPatternOperatorProperties(const Record *R);
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index 8076ce2486f56..9d82a271a465d 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -3681,6 +3681,7 @@ class InstAnalyzer {
bool isBitcast = false;
bool isVariadic = false;
bool hasChain = false;
+ bool mayHaveChain = false;
InstAnalyzer(const CodeGenDAGPatterns &cdp) : CDP(cdp) {}
@@ -3744,6 +3745,8 @@ class InstAnalyzer {
isVariadic = true;
if (N.NodeHasProperty(SDNPHasChain, CDP))
hasChain = true;
+ if (N.NodeHasProperty(SDNPMayHaveChain, CDP))
+ mayHaveChain = true;
if (const CodeGenIntrinsic *IntInfo = N.getIntrinsicInfo(CDP)) {
ModRefInfo MR = IntInfo->ME.getModRef();
@@ -3812,6 +3815,7 @@ static bool InferFromPattern(CodeGenInstruction &InstInfo,
InstInfo.isBitcast |= PatInfo.isBitcast;
InstInfo.hasChain |= PatInfo.hasChain;
InstInfo.hasChain_Inferred = true;
+ InstInfo.mayHaveChain |= PatInfo.mayHaveChain;
}
// Don't infer isVariadic. This flag means something different on SDNodes and
diff --git a/llvm/utils/TableGen/Common/CodeGenInstruction.h b/llvm/utils/TableGen/Common/CodeGenInstruction.h
index ed0bfa7098eb7..870afb22d72c4 100644
--- a/llvm/utils/TableGen/Common/CodeGenInstruction.h
+++ b/llvm/utils/TableGen/Common/CodeGenInstruction.h
@@ -268,6 +268,7 @@ class CodeGenInstruction {
bool hasChain_Inferred : 1;
bool variadicOpsAreDefs : 1;
bool isAuthenticated : 1;
+ bool mayHaveChain : 1;
std::string DeprecatedReason;
bool HasComplexDeprecationPredicate;
diff --git a/llvm/utils/TableGen/Common/CodeGenTarget.cpp b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
index 3db0d07eec88f..c11304d971a1b 100644
--- a/llvm/utils/TableGen/Common/CodeGenTarget.cpp
+++...
[truncated]
|
|
|
||
| if (HasChain && MayHaveChain) | ||
| reportNodeError( | ||
| DAG, N, "Flags 'HasChain' and 'MayHaveChain' cannot be both specified"); |
There was a problem hiding this comment.
This should be diagnosed in TableGen when parsing SDNode (and asserted here).
| } | ||
| } | ||
|
|
||
| /// Test if this node has an input chain. |
There was a problem hiding this comment.
Suggested change
| /// Test if this node has an input chain. | |
| /// Test if this node has a chain. |
|
|
||
| /// Test if this node has an input chain. | ||
| bool hasChain() const { | ||
| return NumOperands > 0 && OperandList[0].getValueType() == MVT::Other; |
There was a problem hiding this comment.
There are other operands that have MVT::Other type. In particular, setcc predicates. Some targets may use this type for other custom operands.
I wish we had a dedicated MVT::Chain type, like we have MVT::Glue.
There was a problem hiding this comment.
Maybe we should be checking for a chain result? I don't think that's ambiguous. Unfortunately, there might be a Glue result after the chain result so it requires checking the last and possibly second to last result.
|
|
||
| /// Test if this node is a floating-point operation which can exist in two | ||
| /// forms, - with chain or without it. | ||
| bool isFPOperation() const { |
There was a problem hiding this comment.
Are there FP operations that exist in one form? If so, the method name should be more specific.
| def SDNPSideEffect : SDNodeProperty; // Sets 'HasUnmodelledSideEffects'. | ||
| def SDNPMemOperand : SDNodeProperty; // Touches memory, has assoc MemOperand | ||
| def SDNPVariadic : SDNodeProperty; // Node has variable arguments. | ||
| def SDNPMayHaveChain: SDNodeProperty; // Optionally has chain operand/result. |
There was a problem hiding this comment.
There is a correlation with SDNode::IsStrictFP flag. Do we still need it?
| // CHECK-NEXT: NODE_1 = ISD::BUILTIN_OP_END, | ||
| // CHECK-NEXT: NODE_2, | ||
| // CHECK-NEXT: NODE_3, | ||
| // CHECK-NEXT: NODE_4 |
There was a problem hiding this comment.
Suggested change
| // CHECK-NEXT: NODE_4 | |
| // CHECK-NEXT: NODE_4, |
| SDNPOptInGlue, | ||
| SDNPMemOperand, | ||
| SDNPVariadic, | ||
| SDNPMayHaveChain |
There was a problem hiding this comment.
Suggested change
| SDNPMayHaveChain | |
| SDNPMayHaveChain, |
| SDNPSideEffect, | ||
| SDNPMemOperand, | ||
| SDNPVariadic, | ||
| SDNPMayHaveChain |
There was a problem hiding this comment.
Suggested change
| SDNPMayHaveChain | |
| SDNPMayHaveChain, |
| Properties |= 1 << SDNPMemOperand; | ||
| } else if (Prop->getName() == "SDNPVariadic") { | ||
| Properties |= 1 << SDNPVariadic; | ||
| } else if (Prop->getName() == "SDNPMayHaveChain") { |
There was a problem hiding this comment.
My understanding is that the property is not a valid property for patterns.
| bool hasChain_Inferred : 1; | ||
| bool variadicOpsAreDefs : 1; | ||
| bool isAuthenticated : 1; | ||
| bool mayHaveChain : 1; |
topperc
reviewed
Oct 13, 2025
|
|
||
| /// Test if this node has an input chain. | ||
| bool hasChain() const { | ||
| return NumOperands > 0 && OperandList[0].getValueType() == MVT::Other; |
There was a problem hiding this comment.
Maybe we should be checking for a chain result? I don't think that's ambiguous. Unfortunately, there might be a Glue result after the chain result so it requires checking the last and possibly second to last result.
|
|
||
| void RecordOptionalChainMatcher::printImpl(raw_ostream &OS, | ||
| indent Indent) const { | ||
| OS << Indent << "CaptureOptionalChain\n"; |
There was a problem hiding this comment.
Text doesn't match the class name.
|
What is the migration plan? I assume if we start generating nodes with chains sometimes, existing code will start breaking when the operands aren't where they expect. |
|
I think this will strictly be more dangerous than having a mirrored chain node. I think we'd be better off with tablegen generating a separate opcode that includes the chain and a mapping function between the two |
- Property SDNPMayHaveChain is renamed to SDNPOptChain, - Changed implementation of `hasChain`, - Removed method `isFPOperation`, - Conflict HasChain with OptChain is detected during parsing, - Use 'SDNPHasChain' and 'SDNPOptChain' in diagnostics,
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Did you consider making FP nodes always have a chain and using EntryNode as the chain input for non-strict FP operations? You could use a bit from SDNodeBitfields, which I think is unused for arithmetic nodes to indicate the node is strict. This would give a consistent operand order. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
This change adds a new property for Selection DAG nodes used in pattern descriptions: SDNPMayHaveChain. A node with this property may have or may not have a chain operand. For example, both of the following variants become valid:
The specific variant is determined during pattern matching, based on whether the first operand is a chain (i.e. has the type MVT::Other).
This feature is intended to be used for floating point operations. They have side effects in a strictfp environment and are pure functions in the default FP environment. Currently each such operation requires two opcodes - one for each kind of FP environment. These opcodes represent the same operation and are processed similarly, which increase amount of code. With this feature the support of strictfp environment should be easier, as it can use the same opcode as the default environment.