[SelectionDAG] Simplify classof of MemSDNode and MemIntrinsicSDNode (NFC) #115720
Conversation
…NFC) `SDNodeBits.IsMemIntrinsic` is set if and only if the node is an instance of `MemIntrinsicSDNode`. Thus, to check if a node is an instance of `MemIntrinsicSDNode` we only need to check this bit.
|
@llvm/pr-subscribers-llvm-selectiondag Author: Sergei Barannikov (s-barannikov) Changes
Full diff: https://github.com/llvm/llvm-project/pull/115720.diff 1 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 739ce05e947346..677b59e0c8fbeb 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -708,15 +708,7 @@ END_TWO_BYTE_PACK()
bool isUndef() const { return NodeType == ISD::UNDEF; }
/// Test if this node is a memory intrinsic (with valid pointer information).
- /// INTRINSIC_W_CHAIN and INTRINSIC_VOID nodes are sometimes created for
- /// non-memory intrinsics (with chains) that are not really instances of
- /// MemSDNode. For such nodes, we need some extra state to determine the
- /// proper classof relationship.
- bool isMemIntrinsic() const {
- return (NodeType == ISD::INTRINSIC_W_CHAIN ||
- NodeType == ISD::INTRINSIC_VOID) &&
- SDNodeBits.IsMemIntrinsic;
- }
+ bool isMemIntrinsic() const { return SDNodeBits.IsMemIntrinsic; }
/// Test if this node is a strict floating point pseudo-op.
bool isStrictFPOpcode() {
@@ -1464,7 +1456,6 @@ class MemSDNode : public SDNode {
switch (N->getOpcode()) {
case ISD::LOAD:
case ISD::STORE:
- case ISD::PREFETCH:
case ISD::ATOMIC_CMP_SWAP:
case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
case ISD::ATOMIC_SWAP:
@@ -1504,7 +1495,7 @@ class MemSDNode : public SDNode {
case ISD::EXPERIMENTAL_VECTOR_HISTOGRAM:
return true;
default:
- return N->isMemIntrinsic() || N->isTargetMemoryOpcode();
+ return N->isMemIntrinsic();
}
}
};
@@ -1596,9 +1587,7 @@ class MemIntrinsicSDNode : public MemSDNode {
static bool classof(const SDNode *N) {
// We lower some target intrinsics to their target opcode
// early a node with a target opcode can be of this class
- return N->isMemIntrinsic() ||
- N->getOpcode() == ISD::PREFETCH ||
- N->isTargetMemoryOpcode();
+ return N->isMemIntrinsic();
}
};
|
| NodeType == ISD::INTRINSIC_VOID) && | ||
| SDNodeBits.IsMemIntrinsic; | ||
| } | ||
| bool isMemIntrinsic() const { return SDNodeBits.IsMemIntrinsic; } |
There was a problem hiding this comment.
Isn't this SDNodeBits part of a union? Isn't the opcode check to make sure that part of the union is active?
There was a problem hiding this comment.
structs other than SDNodeBitfields have first NumSDNodeBits bits reserved.
That is, IsMemIntrinsic is shared between all structs in the union.
I think the opcode check is a historical artifact.
There was a problem hiding this comment.
Note that this opcode check doesn't really tell us which union member is active because INTRINSIC_W_CHAIN / INTRINSIC_VOID can either be an ordinary SDNode or MemIntrinsicSDNode. If I read the removed comment correctly, this bit was introduced to disambiguate these cases, but introducing it also made the opcode check redundant.
Assuming there is no (pre-existing) undefined behavior, just checking the bit is more robust as the method will now return false for ordinary SDNodes erroneously created with isTargetMemoryOpcode opcodes.
There was a problem hiding this comment.
Thanks for the clarification.
Assuming there is no (pre-existing) undefined behavior, just checking the bit is more robust as the method will now return false for ordinary SDNodes erroneously created with isTargetMemoryOpcode opcodes.
I think that would be considered a bug to not use SDMemIntrinsicSDNode for anything that isTargetMemoryOpcode()
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/43/builds/10080 Here is the relevant piece of the build log for the reference |
…NFC) (llvm#115720) `SDNodeBits.IsMemIntrinsic` is set if and only if the node is an instance of `MemIntrinsicSDNode`. Thus, to check if a node is an instance of `MemIntrinsicSDNode` we only need to check this bit.
SDNodeBits.IsMemIntrinsicis set if and only if the node is an instance ofMemIntrinsicSDNode. Thus, to check if a node is an instance ofMemIntrinsicSDNodewe only need to check this bit.