Introducing a new ISD::POISON SDNode to represent the poison value in the IR. #125883
+74
−22
Conversation
This file contains 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
You can test this locally with the following command:git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 7eaaa4e9415fc7533e688a9ed877aff75b7dfce4 e6be89c17e8b3e0783f707f8a4359ec9c30b0531 llvm/include/llvm/CodeGen/ISDOpcodes.h llvm/include/llvm/CodeGen/SelectionDAG.h llvm/include/llvm/CodeGen/SelectionDAGNodes.h llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cppThe following files introduce new uses of undef:
Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields In tests, avoid using For example, this is considered a bad practice: define void @fn() {
...
br i1 undef, ...
}Please use the following instead: define void @fn(i1 %cond) {
...
br i1 %cond, ...
}Please refer to the Undefined Behavior Manual for more information. |
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
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
…t hit by test cases currently.
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.