[SelectionDAG] Introducing a new ISD::POISON SDNode to represent the poison value in the IR. #125883
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
|
@llvm/pr-subscribers-backend-systemz @llvm/pr-subscribers-llvm-selectiondag Author: zhijian lin (diggerlin) ChangesA new ISD::POISON SDNode is introduced to represent the poison value in the IR, replacing the previous use of ISD::UNDEF. Full diff: https://github.com/llvm/llvm-project/pull/125883.diff 12 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h
index 604dc9419025b09..cdb0d4b92bbf709 100644
--- a/llvm/include/llvm/CodeGen/ISDOpcodes.h
+++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h
@@ -217,6 +217,9 @@ enum NodeType {
/// UNDEF - An undefined node.
UNDEF,
+ /// POISON - A poison node.
+ POISON,
+
/// FREEZE - FREEZE(VAL) returns an arbitrary value if VAL is UNDEF (or
/// is evaluated to UNDEF), or returns VAL otherwise. Note that each
/// read of UNDEF can yield different value, but FREEZE(UNDEF) cannot.
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index b31ad11c3ee0ee8..04d72eca9a35be2 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -871,7 +871,7 @@ class SelectionDAG {
/// for integers, a type wider than) VT's element type.
SDValue getSplatBuildVector(EVT VT, const SDLoc &DL, SDValue Op) {
// VerifySDNode (via InsertNode) checks BUILD_VECTOR later.
- if (Op.getOpcode() == ISD::UNDEF) {
+ if (Op.isUndef()) {
assert((VT.getVectorElementType() == Op.getValueType() ||
(VT.isInteger() &&
VT.getVectorElementType().bitsLE(Op.getValueType()))) &&
@@ -887,7 +887,7 @@ class SelectionDAG {
// Return a splat ISD::SPLAT_VECTOR node, consisting of Op splatted to all
// elements.
SDValue getSplatVector(EVT VT, const SDLoc &DL, SDValue Op) {
- if (Op.getOpcode() == ISD::UNDEF) {
+ if (Op.isUndef()) {
assert((VT.getVectorElementType() == Op.getValueType() ||
(VT.isInteger() &&
VT.getVectorElementType().bitsLE(Op.getValueType()))) &&
@@ -1128,6 +1128,9 @@ class SelectionDAG {
return getNode(ISD::UNDEF, SDLoc(), VT);
}
+ /// Return an POISON node. POISON does not have a useful SDLoc.
+ SDValue getPoison(EVT VT) { return getNode(ISD::POISON, SDLoc(), VT); }
+
/// Return a node that represents the runtime scaling 'MulImm * RuntimeVL'.
SDValue getVScale(const SDLoc &DL, EVT VT, APInt MulImm,
bool ConstantFold = true);
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 03899493847b394..c98c9da43a30804 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -690,8 +690,17 @@ END_TWO_BYTE_PACK()
/// \<target\>ISD namespace).
bool isTargetOpcode() const { return NodeType >= ISD::BUILTIN_OP_END; }
- /// Return true if the type of the node type undefined.
- bool isUndef() const { return NodeType == ISD::UNDEF; }
+ /// Returns true if the node type is UNDEF or, when UndefOnly is false,
+ /// POISON.
+ /// - When UndefOnly is true, returns true only for UNDEF.
+ /// - When UndefOnly is false, returns true for both UNDEF and POISON.
+ /// @param UndefOnly Determines whether to check only for UNDEF.
+ bool isUndef(bool UndefOnly = false) const {
+ return NodeType == ISD::UNDEF || (!UndefOnly && NodeType == ISD::POISON);
+ }
+
+ /// Return true if the type of the node type poison.
+ bool isPoison() const { return NodeType == ISD::POISON; }
/// Test if this node is a memory intrinsic (with valid pointer information).
bool isMemIntrinsic() const { return SDNodeBits.IsMemIntrinsic; }
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 02b79c67af3ee07..ce776214df1f1e4 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -16156,7 +16156,7 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
// also recursively replace t184 by t150.
SDValue MaybePoisonOperand = N->getOperand(0).getOperand(OpNo);
// Don't replace every single UNDEF everywhere with frozen UNDEF, though.
- if (MaybePoisonOperand.getOpcode() == ISD::UNDEF)
+ if (MaybePoisonOperand.isUndef())
continue;
// First, freeze each offending operand.
SDValue FrozenMaybePoisonOperand = DAG.getFreeze(MaybePoisonOperand);
@@ -16182,9 +16182,10 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
// Finally, recreate the node, it's operands were updated to use
// frozen operands, so we just need to use it's "original" operands.
SmallVector<SDValue> Ops(N0->ops());
- // Special-handle ISD::UNDEF, each single one of them can be it's own thing.
+ // Special-handle ISD::UNDEF, ISD::POISON, each single one of them can be it's
+ // own thing.
for (SDValue &Op : Ops) {
- if (Op.getOpcode() == ISD::UNDEF)
+ if (Op.isUndef())
Op = DAG.getFreeze(Op);
}
@@ -24320,7 +24321,7 @@ static SDValue combineConcatVectorOfScalars(SDNode *N, SelectionDAG &DAG) {
if (ISD::BITCAST == Op.getOpcode() &&
!Op.getOperand(0).getValueType().isVector())
Ops.push_back(Op.getOperand(0));
- else if (ISD::UNDEF == Op.getOpcode())
+ else if (Op.isUndef())
Ops.push_back(DAG.getNode(ISD::UNDEF, DL, SVT));
else
return SDValue();
@@ -24715,7 +24716,7 @@ SDValue DAGCombiner::visitCONCAT_VECTORS(SDNode *N) {
// fold (concat_vectors (BUILD_VECTOR A, B, ...), (BUILD_VECTOR C, D, ...))
// -> (BUILD_VECTOR A, B, ..., C, D, ...)
auto IsBuildVectorOrUndef = [](const SDValue &Op) {
- return ISD::UNDEF == Op.getOpcode() || ISD::BUILD_VECTOR == Op.getOpcode();
+ return Op.isUndef() || ISD::BUILD_VECTOR == Op.getOpcode();
};
if (llvm::all_of(N->ops(), IsBuildVectorOrUndef)) {
SmallVector<SDValue, 8> Opnds;
@@ -24739,7 +24740,7 @@ SDValue DAGCombiner::visitCONCAT_VECTORS(SDNode *N) {
EVT OpVT = Op.getValueType();
unsigned NumElts = OpVT.getVectorNumElements();
- if (ISD::UNDEF == Op.getOpcode())
+ if (Op.isUndef())
Opnds.append(NumElts, DAG.getUNDEF(MinVT));
if (ISD::BUILD_VECTOR == Op.getOpcode()) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index c6475f02199033d..7a84d695ccc5a57 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -977,6 +977,22 @@ void SelectionDAGLegalize::LegalizeOp(SDNode *Node) {
TargetLowering::LegalizeAction Action = TargetLowering::Legal;
bool SimpleFinishLegalizing = true;
switch (Node->getOpcode()) {
+ // FIXME: If the node represents a poison value, replace it with an undef
+ // value.
+ // A poison value results from an erroneous operation but does not cause
+ // immediate undefined behavior, allowing speculative execution.
+ // Since most operations propagate poison, it is valid to replace poison
+ // with an undef value, which can take any legal value of the same type.
+ // This ensures that downstream computations do not rely on poison semantics.
+ // Poison is more restrictive than undef. Since we replace poison with undef
+ // here, the poison information will be lost after the code is executed. In
+ // the futher, If we need to retain the poison information after the code is
+ // executed, we will need to modify the code accordingly.
+ case ISD::POISON: {
+ SDValue UndefNode = DAG.getUNDEF(Node->getValueType(0));
+ ReplaceNode(Node, UndefNode.getNode());
+ break;
+ }
case ISD::INTRINSIC_W_CHAIN:
case ISD::INTRINSIC_WO_CHAIN:
case ISD::INTRINSIC_VOID:
@@ -3136,6 +3152,7 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
for (unsigned i = 0; i < Node->getNumValues(); i++)
Results.push_back(Node->getOperand(i));
break;
+ case ISD::POISON:
case ISD::UNDEF: {
EVT VT = Node->getValueType(0);
if (VT.isInteger())
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 71f100bfa034343..1c28722f3f0a284 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -164,6 +164,7 @@ void DAGTypeLegalizer::SoftenFloatResult(SDNode *N, unsigned ResNo) {
case ISD::STRICT_UINT_TO_FP:
case ISD::SINT_TO_FP:
case ISD::UINT_TO_FP: R = SoftenFloatRes_XINT_TO_FP(N); break;
+ case ISD::POISON:
case ISD::UNDEF: R = SoftenFloatRes_UNDEF(N); break;
case ISD::VAARG: R = SoftenFloatRes_VAARG(N); break;
case ISD::VECREDUCE_FADD:
@@ -1474,6 +1475,7 @@ void DAGTypeLegalizer::ExpandFloatResult(SDNode *N, unsigned ResNo) {
report_fatal_error("Do not know how to expand the result of this "
"operator!");
// clang-format off
+ case ISD::POISON:
case ISD::UNDEF: SplitRes_UNDEF(N, Lo, Hi); break;
case ISD::SELECT: SplitRes_Select(N, Lo, Hi); break;
case ISD::SELECT_CC: SplitRes_SELECT_CC(N, Lo, Hi); break;
@@ -2783,6 +2785,7 @@ void DAGTypeLegalizer::PromoteFloatResult(SDNode *N, unsigned ResNo) {
case ISD::SINT_TO_FP:
case ISD::UINT_TO_FP: R = PromoteFloatRes_XINT_TO_FP(N); break;
+ case ISD::POISON:
case ISD::UNDEF: R = PromoteFloatRes_UNDEF(N); break;
case ISD::ATOMIC_SWAP: R = BitcastToInt_ATOMIC_SWAP(N); break;
case ISD::VECREDUCE_FADD:
@@ -3242,6 +3245,7 @@ void DAGTypeLegalizer::SoftPromoteHalfResult(SDNode *N, unsigned ResNo) {
case ISD::STRICT_UINT_TO_FP:
case ISD::SINT_TO_FP:
case ISD::UINT_TO_FP: R = SoftPromoteHalfRes_XINT_TO_FP(N); break;
+ case ISD::POISON:
case ISD::UNDEF: R = SoftPromoteHalfRes_UNDEF(N); break;
case ISD::ATOMIC_SWAP: R = BitcastToInt_ATOMIC_SWAP(N); break;
case ISD::VECREDUCE_FADD:
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
index be7521f34168503..4da7bf9a680abe1 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
@@ -118,6 +118,7 @@ void DAGTypeLegalizer::PromoteIntegerResult(SDNode *N, unsigned ResNo) {
case ISD::VP_SRL: Res = PromoteIntRes_SRL(N); break;
case ISD::VP_TRUNCATE:
case ISD::TRUNCATE: Res = PromoteIntRes_TRUNCATE(N); break;
+ case ISD::POISON:
case ISD::UNDEF: Res = PromoteIntRes_UNDEF(N); break;
case ISD::VAARG: Res = PromoteIntRes_VAARG(N); break;
case ISD::VSCALE: Res = PromoteIntRes_VSCALE(N); break;
@@ -2840,6 +2841,7 @@ void DAGTypeLegalizer::ExpandIntegerResult(SDNode *N, unsigned ResNo) {
case ISD::MERGE_VALUES: SplitRes_MERGE_VALUES(N, ResNo, Lo, Hi); break;
case ISD::SELECT: SplitRes_Select(N, Lo, Hi); break;
case ISD::SELECT_CC: SplitRes_SELECT_CC(N, Lo, Hi); break;
+ case ISD::POISON:
case ISD::UNDEF: SplitRes_UNDEF(N, Lo, Hi); break;
case ISD::FREEZE: SplitRes_FREEZE(N, Lo, Hi); break;
case ISD::SETCC: ExpandIntRes_SETCC(N, Lo, Hi); break;
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 5117eb8d91dfb21..37aad1e21f7bd2b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -71,6 +71,7 @@ void DAGTypeLegalizer::ScalarizeVectorResult(SDNode *N, unsigned ResNo) {
case ISD::SELECT: R = ScalarizeVecRes_SELECT(N); break;
case ISD::SELECT_CC: R = ScalarizeVecRes_SELECT_CC(N); break;
case ISD::SETCC: R = ScalarizeVecRes_SETCC(N); break;
+ case ISD::POISON:
case ISD::UNDEF: R = ScalarizeVecRes_UNDEF(N); break;
case ISD::VECTOR_SHUFFLE: R = ScalarizeVecRes_VECTOR_SHUFFLE(N); break;
case ISD::IS_FPCLASS: R = ScalarizeVecRes_IS_FPCLASS(N); break;
@@ -1117,6 +1118,7 @@ void DAGTypeLegalizer::SplitVectorResult(SDNode *N, unsigned ResNo) {
case ISD::VP_MERGE:
case ISD::VP_SELECT: SplitRes_Select(N, Lo, Hi); break;
case ISD::SELECT_CC: SplitRes_SELECT_CC(N, Lo, Hi); break;
+ case ISD::POISON:
case ISD::UNDEF: SplitRes_UNDEF(N, Lo, Hi); break;
case ISD::BITCAST: SplitVecRes_BITCAST(N, Lo, Hi); break;
case ISD::BUILD_VECTOR: SplitVecRes_BUILD_VECTOR(N, Lo, Hi); break;
@@ -4523,6 +4525,7 @@ void DAGTypeLegalizer::WidenVectorResult(SDNode *N, unsigned ResNo) {
case ISD::SELECT_CC: Res = WidenVecRes_SELECT_CC(N); break;
case ISD::VP_SETCC:
case ISD::SETCC: Res = WidenVecRes_SETCC(N); break;
+ case ISD::POISON:
case ISD::UNDEF: Res = WidenVecRes_UNDEF(N); break;
case ISD::VECTOR_SHUFFLE:
Res = WidenVecRes_VECTOR_SHUFFLE(cast<ShuffleVectorSDNode>(N));
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 0dfd0302ae5438c..6d62760d25dfeda 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5458,6 +5458,9 @@ bool SelectionDAG::isGuaranteedNotToBeUndefOrPoison(SDValue Op,
case ISD::CopyFromReg:
return true;
+ case ISD::POISON:
+ return false;
+
case ISD::UNDEF:
return PoisonOnly;
@@ -6307,9 +6310,10 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
Flags.setNonNeg(N1->getFlags().hasNonNeg());
return getNode(OpOpcode, DL, VT, N1.getOperand(0), Flags);
}
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
// sext(undef) = 0, because the top bits will all be the same.
return getConstant(0, DL, VT);
+
break;
case ISD::ZERO_EXTEND:
assert(VT.isInteger() && N1.getValueType().isInteger() &&
@@ -6327,7 +6331,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
Flags.setNonNeg(N1->getFlags().hasNonNeg());
return getNode(ISD::ZERO_EXTEND, DL, VT, N1.getOperand(0), Flags);
}
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
// zext(undef) = 0, because the top bits will be zero.
return getConstant(0, DL, VT);
@@ -6369,7 +6373,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
// (ext (zext x)) -> (zext x) and (ext (sext x)) -> (sext x)
return getNode(OpOpcode, DL, VT, N1.getOperand(0), Flags);
}
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
// (ext (trunc x)) -> x
@@ -6404,7 +6408,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
return getNode(ISD::TRUNCATE, DL, VT, N1.getOperand(0));
return N1.getOperand(0);
}
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
if (OpOpcode == ISD::VSCALE && !NewNodesMustHaveLegalTypes)
return getVScale(DL, VT,
@@ -6422,14 +6426,14 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
break;
case ISD::ABS:
assert(VT.isInteger() && VT == N1.getValueType() && "Invalid ABS!");
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getConstant(0, DL, VT);
break;
case ISD::BSWAP:
assert(VT.isInteger() && VT == N1.getValueType() && "Invalid BSWAP!");
assert((VT.getScalarSizeInBits() % 16 == 0) &&
"BSWAP types must be a multiple of 16 bits!");
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
// bswap(bswap(X)) -> X.
if (OpOpcode == ISD::BSWAP)
@@ -6437,7 +6441,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
break;
case ISD::BITREVERSE:
assert(VT.isInteger() && VT == N1.getValueType() && "Invalid BITREVERSE!");
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
break;
case ISD::BITCAST:
@@ -6446,7 +6450,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
if (VT == N1.getValueType()) return N1; // noop conversion.
if (OpOpcode == ISD::BITCAST) // bitconv(bitconv(x)) -> bitconv(x)
return getNode(ISD::BITCAST, DL, VT, N1.getOperand(0));
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
break;
case ISD::SCALAR_TO_VECTOR:
@@ -6456,7 +6460,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
N1.getValueType().isInteger() &&
VT.getVectorElementType().bitsLE(N1.getValueType()))) &&
"Illegal SCALAR_TO_VECTOR node!");
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
// scalar_to_vector(extract_vector_elt V, 0) -> V, top bits are undefined.
if (OpOpcode == ISD::EXTRACT_VECTOR_ELT &&
@@ -6467,7 +6471,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
break;
case ISD::FNEG:
// Negation of an unknown bag of bits is still completely undefined.
- if (OpOpcode == ISD::UNDEF)
+ if (N1.isUndef())
return getUNDEF(VT);
if (OpOpcode == ISD::FNEG) // --X -> X
@@ -9237,6 +9241,11 @@ SDValue SelectionDAG::getLoad(ISD::MemIndexedMode AM, ISD::LoadExtType ExtType,
SDVTList VTs = Indexed ?
getVTList(VT, Ptr.getValueType(), MVT::Other) : getVTList(VT, MVT::Other);
+
+ // Lower poison to undef.
+ if (Ptr.getNode()->isPoison())
+ Ptr = getUNDEF(Ptr.getValueType());
+
SDValue Ops[] = { Chain, Ptr, Offset };
FoldingSetNodeID ID;
AddNodeIDNode(ID, ISD::LOAD, VTs, Ops);
@@ -13374,7 +13383,7 @@ void BuildVectorSDNode::recastRawBits(bool IsLittleEndian,
bool BuildVectorSDNode::isConstant() const {
for (const SDValue &Op : op_values()) {
unsigned Opc = Op.getOpcode();
- if (Opc != ISD::UNDEF && Opc != ISD::Constant && Opc != ISD::ConstantFP)
+ if (!Op.isUndef() && Opc != ISD::Constant && Opc != ISD::ConstantFP)
return false;
}
return true;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index f8d7c3ef7bbe71a..fe4e3c00d4260ee 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1819,7 +1819,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
return DAG.getConstantFP(*CFP, getCurSDLoc(), VT);
if (isa<UndefValue>(C) && !V->getType()->isAggregateType())
- return DAG.getUNDEF(VT);
+ return isa<PoisonValue>(C) ? DAG.getPoison(VT) : DAG.getUNDEF(VT);
if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(C)) {
visit(CE->getOpcode(), *CE);
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
index 580ff19065557ba..4434b1203451f9e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
@@ -188,6 +188,7 @@ std::string SDNode::getOperationName(const SelectionDAG *G) const {
case ISD::CopyToReg: return "CopyToReg";
case ISD::CopyFromReg: return "CopyFromReg";
case ISD::UNDEF: return "undef";
+ case ISD::POISON: return "poison";
case ISD::VSCALE: return "vscale";
case ISD::MERGE_VALUES: return "merge_values";
case ISD::INLINEASM: return "inlineasm";
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index d64a90bcaae7dac..48fd1e1e4591952 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -3275,6 +3275,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
case ISD::WRITE_REGISTER:
Select_WRITE_REGISTER(NodeToMatch);
return;
+ case ISD::POISON:
case ISD::UNDEF:
Select_UNDEF(NodeToMatch);
return;
|
diggerlin
requested review from
arsenm,
s-barannikov,
topperc and
hubert-reinterpretcast
|
✅ With the latest revision this PR passed the undef deprecator. |
arsenm
reviewed
Feb 6, 2025
| @@ -2783,6 +2785,7 @@ void DAGTypeLegalizer::PromoteFloatResult(SDNode *N, unsigned ResNo) { | |||
|
|
|||
| case ISD::SINT_TO_FP: | |||
| case ISD::UINT_TO_FP: R = PromoteFloatRes_XINT_TO_FP(N); break; | |||
| case ISD::POISON: | |||
There was a problem hiding this comment.
This should get tests that hit all of these legalization paths
There was a problem hiding this comment.
I delete all the case ISD::POISON: in the file since they are not hit by current test case. otherwise the patch will be too big. They will be added in a later patch with new test case.
RKSimon
reviewed
Feb 6, 2025
| @@ -871,7 +871,7 @@ class SelectionDAG { | |||
| /// for integers, a type wider than) VT's element type. | |||
| SDValue getSplatBuildVector(EVT VT, const SDLoc &DL, SDValue Op) { | |||
| // VerifySDNode (via InsertNode) checks BUILD_VECTOR later. | |||
| if (Op.getOpcode() == ISD::UNDEF) { | |||
| if (Op.isUndef()) { | |||
There was a problem hiding this comment.
It might be useful to to pull out the isUndef() changes into a real NFC patch.
There was a problem hiding this comment.
since it is to isUndefOrPoison(), it is not a NFC now
There was a problem hiding this comment.
With the updates isUndef() semantics this comment applies again, it would be good to split off this NFC change.
There was a problem hiding this comment.
the isUndef() is bool isUndef(bool DoNotIncludeExplicitPoison = false), it is not a NFC, do we still need to split off this NFC change?
There was a problem hiding this comment.
You can introduce the uses of isUndef() without changing the signature.
bjope
reviewed
Feb 6, 2025
| /// - When UndefOnly is true, returns true only for UNDEF. | ||
| /// - When UndefOnly is false, returns true for both UNDEF and POISON. | ||
| /// @param UndefOnly Determines whether to check only for UNDEF. | ||
| bool isUndef(bool UndefOnly = false) const { |
There was a problem hiding this comment.
Should this be called something else?
Since we have ISD::UNDEF it would be easy to think that isUndef() checks if the type is ISD::UNDEF. Just like isPoison() checks for ISD::POISON.
Would be clearer to have an isUndefOrPoison helper (or something totally generic not mapping to an ISD type, such as isUndefinedType()) when it isn't a 1-1 mapping to the ISD type name.
There was a problem hiding this comment.
I think that it also is easier for downstream targets etc to adapt, if not reusing the old name for something new.
There was a problem hiding this comment.
Don't we usually want to keep treating poison as undef to avoid updating a bunch of places? In IR, PoisonValue inherits from UndefValue so isa<UndefValue> returns true for poison or undef. I believe SelectionDAGBuilder was previously creating ISD::UNDEF for poison. So continuing to treat ISD::POISON like ISD::UNDEF in many cases would be consistent.
@nikic what do you think?
There was a problem hiding this comment.
Yes, I think this should reproduce the IR behavior. isUndef = poison or undef, and places that care specifically about poison check isPoison
There was a problem hiding this comment.
So Poison can be seen as a specialization of Undef.
It felt a bit confusing (terminology wise) to have
isUndef(bool UndefOnly = false)
Since if Poison is a subset of Undef, then setting UndefOnly=true kind of would include Poison as well.
Maybe it should say something like
isUndef(bool DoNotIncludeExplicitPoison = false)
since one wouldn't know if a plain ISD::Undef actually is poison (if Poison is a subset of Undef)?
There was a problem hiding this comment.
It is a bit confusing, but it's internally consistent. But yes, poison is a special more aggressive undef
There was a problem hiding this comment.
I agree that isUndef() should include poison by default.
There was a problem hiding this comment.
I updated the patch based on the discussion.
diggerlin
changed the title
diggerlin
force-pushed
the
digger/ISD-POISON
branch
from
a3963e2 to
15c2c17
Compare
|
gentle ping for feedback. |
diggerlin
force-pushed
the
digger/ISD-POISON
branch
from
253cb67 to
94201df
Compare
kuhar
added a commit
to iree-org/llvm-project
that referenced
this pull request
Apr 8, 2025
…ent the poison value in the IR. (llvm#125883)" This reverts commit 8fddef8. This PR breaks `math_ops_llvm-cpu.mlir` on RISC-V.
|
Hi @diggerlin, We've had an assertion failure on RISC-V after integrating this PR: |
kuhar
added a commit
to iree-org/iree
that referenced
this pull request
Apr 8, 2025
The changes are caused by: * llvm/llvm-project#134206 Reverts: * llvm/llvm-project#125883. This breaks the RISC-V test: `e2e/math/math_ops_llvm-cpu.mlir`. --------- Signed-off-by: Jakub Kuderski <jakub@nod-labs.com>
kuhar
added a commit
to iree-org/llvm-project
that referenced
this pull request
Apr 8, 2025
…ent the poison value in the IR. (llvm#125883)" This reverts commit 8fddef8. This PR breaks `math_ops_llvm-cpu.mlir` on RISC-V.
kuhar
added a commit
to iree-org/iree
that referenced
this pull request
Apr 9, 2025
Still carrying the revert for: * llvm/llvm-project#125883 from #20494 Signed-off-by: Jakub Kuderski <jakub@nod-labs.com>
|
This crashes with this patch: |
|
there is a quick fix of the assert |
can we land a fix quickly or should we revert this PR? |
I don't understand... Do you want to revert this PR or not? |
I think you have reverted this PR (36cb81cced6c) . we need to revert your revert first, is it correct ? |
|
I have not -- my revert was only in a downstream fork. |
|
I created a fix PR as #135056 |
|
The fix doesn't resolve my crash -- I'm going to revert this for the time being. |
kuhar
added a commit
that referenced
this pull request
Apr 9, 2025
…ent the poison value in the IR." (#135060) Reverts #125883 This PR causes crashes in RISC-V codegen around f16/f64 poison values: #125883 (comment)
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 9, 2025
…e to represent the poison value in the IR." (#135060) Reverts llvm/llvm-project#125883 This PR causes crashes in RISC-V codegen around f16/f64 poison values: llvm/llvm-project#125883 (comment)
AllinLeeYL
pushed a commit
to AllinLeeYL/llvm-project
that referenced
this pull request
Apr 10, 2025
…ent the poison value in the IR." (llvm#135060) Reverts llvm#125883 This PR causes crashes in RISC-V codegen around f16/f64 poison values: llvm#125883 (comment)
diggerlin
added a commit
to diggerlin/llvm-project
that referenced
this pull request
Apr 10, 2025
…poison value in the IR. (llvm#125883) A new ISD::POISON SDNode is introduced to represent the `poison value` in the IR, replacing the previous use of ISD::UNDEF.
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…ent the poison value in the IR." (llvm#135060) Reverts llvm#125883 This PR causes crashes in RISC-V codegen around f16/f64 poison values: llvm#125883 (comment)
diggerlin
added a commit
that referenced
this pull request
Apr 17, 2025
…nput is poison. (#135387) Propagation to poison in function `SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,SDValue N1, SDValue N2, const SDNodeFlags Flags) ` if one of the input is poison. The patch also revert the test cases llvm/test/CodeGen/X86/pr119158.ll llvm/test/CodeGen/X86/half.ll which are mentioned in #125883 (comment) --------- Co-authored-by: Amy Kwan <amy.kwan1@ibm.com>
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 17, 2025
…ds if the input is poison. (#135387) Propagation to poison in function `SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,SDValue N1, SDValue N2, const SDNodeFlags Flags) ` if one of the input is poison. The patch also revert the test cases llvm/test/CodeGen/X86/pr119158.ll llvm/test/CodeGen/X86/half.ll which are mentioned in llvm/llvm-project#125883 (comment) --------- Co-authored-by: Amy Kwan <amy.kwan1@ibm.com>
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Apr 29, 2025
Local branch origin/amd-gfx 2a16854 Merged main:9fe6f6a0d430 into origin/amd-gfx:3be6366ceae6 Remote branch main 8fddef8 [SelectionDAG] Introducing a new ISD::POISON SDNode to represent the poison value in the IR. (llvm#125883)
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…nput is poison. (llvm#135387) Propagation to poison in function `SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,SDValue N1, SDValue N2, const SDNodeFlags Flags) ` if one of the input is poison. The patch also revert the test cases llvm/test/CodeGen/X86/pr119158.ll llvm/test/CodeGen/X86/half.ll which are mentioned in llvm#125883 (comment) --------- Co-authored-by: Amy Kwan <amy.kwan1@ibm.com>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…nput is poison. (llvm#135387) Propagation to poison in function `SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,SDValue N1, SDValue N2, const SDNodeFlags Flags) ` if one of the input is poison. The patch also revert the test cases llvm/test/CodeGen/X86/pr119158.ll llvm/test/CodeGen/X86/half.ll which are mentioned in llvm#125883 (comment) --------- Co-authored-by: Amy Kwan <amy.kwan1@ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A new ISD::POISON SDNode is introduced to represent the
poison valuein the IR, replacing the previous use of ISD::UNDEF.