Skip to content

[SelectionDAG] Virtualize isTargetStrictFPOpcode / isTargetMemoryOpcode #119969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions llvm/include/llvm/CodeGen/ISDOpcodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1490,17 +1490,6 @@ enum NodeType {
BUILTIN_OP_END
};

/// FIRST_TARGET_STRICTFP_OPCODE - Target-specific pre-isel operations
/// which cannot raise FP exceptions should be less than this value.
/// Those that do must not be less than this value.
static const int FIRST_TARGET_STRICTFP_OPCODE = BUILTIN_OP_END + 400;

/// FIRST_TARGET_MEMORY_OPCODE - Target-specific pre-isel operations
/// which do not reference a specific memory location should be less than
/// this value. Those that do must not be less than this value, and can
/// be used with SelectionDAG::getMemIntrinsicNode.
static const int FIRST_TARGET_MEMORY_OPCODE = BUILTIN_OP_END + 500;

/// Whether this is bitwise logic opcode.
inline bool isBitwiseLogicOp(unsigned Opcode) {
return Opcode == ISD::AND || Opcode == ISD::OR || Opcode == ISD::XOR;
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/CodeGen/SelectionDAG.h
Original file line number Diff line number Diff line change
Expand Up @@ -1330,8 +1330,8 @@ class SelectionDAG {

/// Creates a MemIntrinsicNode that may produce a
/// result and takes a list of operands. Opcode may be INTRINSIC_VOID,
/// INTRINSIC_W_CHAIN, or a target-specific opcode with a value not
/// less than FIRST_TARGET_MEMORY_OPCODE.
/// INTRINSIC_W_CHAIN, or a target-specific memory-referencing opcode
// (see `SelectionDAGTargetInfo::isTargetMemoryOpcode`).
SDValue getMemIntrinsicNode(
unsigned Opcode, const SDLoc &dl, SDVTList VTList, ArrayRef<SDValue> Ops,
EVT MemVT, MachinePointerInfo PtrInfo, Align Alignment,
Expand Down
29 changes: 4 additions & 25 deletions llvm/include/llvm/CodeGen/SelectionDAGNodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ class SDValue {
inline const SDValue &getOperand(unsigned i) const;
inline uint64_t getConstantOperandVal(unsigned i) const;
inline const APInt &getConstantOperandAPInt(unsigned i) const;
inline bool isTargetMemoryOpcode() const;
inline bool isTargetOpcode() const;
inline bool isMachineOpcode() const;
inline bool isUndef() const;
Expand Down Expand Up @@ -691,22 +690,6 @@ END_TWO_BYTE_PACK()
/// \<target\>ISD namespace).
bool isTargetOpcode() const { return NodeType >= ISD::BUILTIN_OP_END; }

/// Test if this node has a target-specific opcode that may raise
/// FP exceptions (in the \<target\>ISD namespace and greater than
/// FIRST_TARGET_STRICTFP_OPCODE). Note that all target memory
/// opcode are currently automatically considered to possibly raise
/// FP exceptions as well.
bool isTargetStrictFPOpcode() const {
return NodeType >= ISD::FIRST_TARGET_STRICTFP_OPCODE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to start table generating the enum values for this, can you just set a high bit in the enum value for memory / strictfp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I misunderstood you, it would require clearing these bits on any use of opcode, e.g. when creating a node or using the enum value as an index into the [generated] table.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the enum values would just be larger values. For table compression you could implicitly know the chain status and truncate the table value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point now. This may work well for strict-fp opcodes, but it will be more difficult to implement for memory opcodes.

Not all memory opcodes will be generated, because some targets do not provide a tablegen description for all memory nodes. This means that some enum members will have to be assigned a value by hand, and this won't look pretty.

Some target that provide tablegen descriptons for memory nodes do not set SDNPMemOperand property on them. As a result, the generated enum values will not have the "memory bit" set, and there is no way to change the enum value included from a generated file.

In the future, we may want to add more properties (e.g., isBinOp), which I think is by itself a good justification for adding a separate field for them rather than encoding them in the opcode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all memory opcodes will be generated, because some targets do not provide a tablegen description for all memory nodes.

This sounds more like a bug. Your RFC sold not requiring targets to use this as a feature, but I'd consider it a defect. It would be better if targets were required to generate this kind of information from tablegen

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all memory opcodes will be generated, because some targets do not provide a tablegen description for all memory nodes.

This sounds more like a bug.

Depends how you look at it. It wasn't a bug before the proposed changes; targets were not required to describe all target-specific opcodes in *.td files.

Your RFC sold not requiring targets to use this as a feature, but I'd consider it a defect.

Yes, otherwise it wouldn't be sold. This feature/defect allows gradual migration.

It would be better if targets were required to generate this kind of information from tablegen

I guess we can do that once all in-tree targets have migrated. Probably need to give some time to downstream targets, too.

}

/// Test if this node has a target-specific
/// memory-referencing opcode (in the \<target\>ISD namespace and
/// greater than FIRST_TARGET_MEMORY_OPCODE).
bool isTargetMemoryOpcode() const {
return NodeType >= ISD::FIRST_TARGET_MEMORY_OPCODE;
}

/// Return true if the type of the node type undefined.
bool isUndef() const { return NodeType == ISD::UNDEF; }

Expand Down Expand Up @@ -1255,10 +1238,6 @@ inline bool SDValue::isTargetOpcode() const {
return Node->isTargetOpcode();
}

inline bool SDValue::isTargetMemoryOpcode() const {
return Node->isTargetMemoryOpcode();
}

inline bool SDValue::isMachineOpcode() const {
return Node->isMachineOpcode();
}
Expand Down Expand Up @@ -1615,10 +1594,10 @@ class AtomicSDNode : public MemSDNode {
}
};

/// This SDNode is used for target intrinsics that touch
/// memory and need an associated MachineMemOperand. Its opcode may be
/// INTRINSIC_VOID, INTRINSIC_W_CHAIN, PREFETCH, or a target-specific opcode
/// with a value not less than FIRST_TARGET_MEMORY_OPCODE.
/// This SDNode is used for target intrinsics that touch memory and need
/// an associated MachineMemOperand. Its opcode may be INTRINSIC_VOID,
/// INTRINSIC_W_CHAIN, PREFETCH, or a target-specific memory-referencing
/// opcode (see `SelectionDAGTargetInfo::isTargetMemoryOpcode`).
class MemIntrinsicSDNode : public MemSDNode {
public:
MemIntrinsicSDNode(unsigned Opc, unsigned Order, const DebugLoc &dl,
Expand Down
13 changes: 13 additions & 0 deletions llvm/include/llvm/CodeGen/SelectionDAGTargetInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ class SelectionDAGTargetInfo {
SelectionDAGTargetInfo &operator=(const SelectionDAGTargetInfo &) = delete;
virtual ~SelectionDAGTargetInfo();

/// Returns true if a node with the given target-specific opcode has
/// a memory operand. Nodes with such opcodes can only be created with
/// `SelectionDAG::getMemIntrinsicNode`.
virtual bool isTargetMemoryOpcode(unsigned Opcode) const { return false; }

/// Returns true if a node with the given target-specific opcode has
/// strict floating-point semantics.
virtual bool isTargetStrictFPOpcode(unsigned Opcode) const { return false; }

/// Returns true if a node with the given target-specific opcode
/// may raise a floating-point exception.
virtual bool mayRaiseFPException(unsigned Opcode) const;

/// Emit target-specific code that performs a memcpy.
/// This can be used by targets to provide code sequences for cases
/// that don't fit the target's parameters for simple loads/stores and can be
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9040,12 +9040,12 @@ SDValue SelectionDAG::getMemIntrinsicNode(unsigned Opcode, const SDLoc &dl,
SDVTList VTList,
ArrayRef<SDValue> Ops, EVT MemVT,
MachineMemOperand *MMO) {
assert((Opcode == ISD::INTRINSIC_VOID ||
Opcode == ISD::INTRINSIC_W_CHAIN ||
Opcode == ISD::PREFETCH ||
(Opcode <= (unsigned)std::numeric_limits<int>::max() &&
(int)Opcode >= ISD::FIRST_TARGET_MEMORY_OPCODE)) &&
"Opcode is not a memory-accessing opcode!");
assert(
(Opcode == ISD::INTRINSIC_VOID || Opcode == ISD::INTRINSIC_W_CHAIN ||
Opcode == ISD::PREFETCH ||
(Opcode <= (unsigned)std::numeric_limits<int>::max() &&
Opcode >= ISD::BUILTIN_OP_END && TSI->isTargetMemoryOpcode(Opcode))) &&
"Opcode is not a memory-accessing opcode!");

// Memoize the node unless it returns a glue result.
MemIntrinsicSDNode *N;
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "llvm/CodeGen/SchedulerRegistry.h"
#include "llvm/CodeGen/SelectionDAG.h"
#include "llvm/CodeGen/SelectionDAGNodes.h"
#include "llvm/CodeGen/SelectionDAGTargetInfo.h"
#include "llvm/CodeGen/StackMaps.h"
#include "llvm/CodeGen/StackProtector.h"
#include "llvm/CodeGen/SwiftErrorValueTracking.h"
Expand Down Expand Up @@ -4382,8 +4383,10 @@ bool SelectionDAGISel::mayRaiseFPException(SDNode *N) const {

// For ISD opcodes, only StrictFP opcodes may raise an FP
// exception.
if (N->isTargetOpcode())
return N->isTargetStrictFPOpcode();
if (N->isTargetOpcode()) {
const SelectionDAGTargetInfo &TSI = CurDAG->getSelectionDAGInfo();
return TSI.mayRaiseFPException(N->getOpcode());
}
return N->isStrictFPOpcode();
}

Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGTargetInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,9 @@
using namespace llvm;

SelectionDAGTargetInfo::~SelectionDAGTargetInfo() = default;

bool SelectionDAGTargetInfo::mayRaiseFPException(unsigned Opcode) const {
// FIXME: All target memory opcodes are currently automatically considered
// to possibly raise FP exceptions. See rev. 63336795.
return isTargetStrictFPOpcode(Opcode) || isTargetMemoryOpcode(Opcode);
}
8 changes: 6 additions & 2 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -477,11 +477,14 @@ enum NodeType : unsigned {
MSRR,

// Strict (exception-raising) floating point comparison
STRICT_FCMP = ISD::FIRST_TARGET_STRICTFP_OPCODE,
FIRST_STRICTFP_OPCODE,
STRICT_FCMP = FIRST_STRICTFP_OPCODE,
STRICT_FCMPE,
LAST_STRICTFP_OPCODE = STRICT_FCMPE,

// NEON Load/Store with post-increment base updates
LD2post = ISD::FIRST_TARGET_MEMORY_OPCODE,
FIRST_MEMORY_OPCODE,
LD2post = FIRST_MEMORY_OPCODE,
LD3post,
LD4post,
ST2post,
Expand Down Expand Up @@ -516,6 +519,7 @@ enum NodeType : unsigned {
STP,
STILP,
STNP,
LAST_MEMORY_OPCODE = STNP,

// SME ZA loads and stores
SME_ZA_LDR,
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ static cl::opt<bool>
"to lower to librt functions"),
cl::init(true));

bool AArch64SelectionDAGInfo::isTargetMemoryOpcode(unsigned Opcode) const {
return Opcode >= AArch64ISD::FIRST_MEMORY_OPCODE &&
Opcode <= AArch64ISD::LAST_MEMORY_OPCODE;
}

bool AArch64SelectionDAGInfo::isTargetStrictFPOpcode(unsigned Opcode) const {
return Opcode >= AArch64ISD::FIRST_STRICTFP_OPCODE &&
Opcode <= AArch64ISD::LAST_STRICTFP_OPCODE;
}

SDValue AArch64SelectionDAGInfo::EmitMOPS(unsigned Opcode, SelectionDAG &DAG,
const SDLoc &DL, SDValue Chain,
SDValue Dst, SDValue SrcOrValue,
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ namespace llvm {

class AArch64SelectionDAGInfo : public SelectionDAGTargetInfo {
public:
bool isTargetMemoryOpcode(unsigned Opcode) const override;

bool isTargetStrictFPOpcode(unsigned Opcode) const override;

SDValue EmitMOPS(unsigned Opcode, SelectionDAG &DAG, const SDLoc &DL,
SDValue Chain, SDValue Dst, SDValue SrcOrValue, SDValue Size,
Align Alignment, bool isVolatile,
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5555,7 +5555,6 @@ const char* AMDGPUTargetLowering::getTargetNodeName(unsigned Opcode) const {
NODE_NAME_CASE(PC_ADD_REL_OFFSET)
NODE_NAME_CASE(LDS)
NODE_NAME_CASE(DUMMY_CHAIN)
case AMDGPUISD::FIRST_MEM_OPCODE_NUMBER: break;
NODE_NAME_CASE(LOAD_D16_HI)
NODE_NAME_CASE(LOAD_D16_LO)
NODE_NAME_CASE(LOAD_D16_HI_I8)
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,9 @@ enum NodeType : unsigned {
LDS,

DUMMY_CHAIN,
FIRST_MEM_OPCODE_NUMBER = ISD::FIRST_TARGET_MEMORY_OPCODE,
LOAD_D16_HI,

FIRST_MEMORY_OPCODE,
LOAD_D16_HI = FIRST_MEMORY_OPCODE,
LOAD_D16_LO,
LOAD_D16_HI_I8,
LOAD_D16_HI_U8,
Expand Down Expand Up @@ -603,6 +604,7 @@ enum NodeType : unsigned {
BUFFER_ATOMIC_FMIN,
BUFFER_ATOMIC_FMAX,
BUFFER_ATOMIC_COND_SUB_U32,
LAST_MEMORY_OPCODE = BUFFER_ATOMIC_COND_SUB_U32,
};

} // End namespace AMDGPUISD
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/AMDGPU/AMDGPUSelectionDAGInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
//===----------------------------------------------------------------------===//

#include "AMDGPUSelectionDAGInfo.h"
#include "AMDGPUISelLowering.h"

using namespace llvm;

AMDGPUSelectionDAGInfo::~AMDGPUSelectionDAGInfo() = default;

bool AMDGPUSelectionDAGInfo::isTargetMemoryOpcode(unsigned Opcode) const {
return Opcode >= AMDGPUISD::FIRST_MEMORY_OPCODE &&
Opcode <= AMDGPUISD::LAST_MEMORY_OPCODE;
}
2 changes: 2 additions & 0 deletions llvm/lib/Target/AMDGPU/AMDGPUSelectionDAGInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace llvm {
class AMDGPUSelectionDAGInfo : public SelectionDAGTargetInfo {
public:
~AMDGPUSelectionDAGInfo() override;

bool isTargetMemoryOpcode(unsigned Opcode) const override;
};

} // namespace llvm
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/ARM/ARMISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ class VectorType;
CSINC, // Conditional select increment.

// Vector load N-element structure to all lanes:
VLD1DUP = ISD::FIRST_TARGET_MEMORY_OPCODE,
FIRST_MEMORY_OPCODE,
VLD1DUP = FIRST_MEMORY_OPCODE,
VLD2DUP,
VLD3DUP,
VLD4DUP,
Expand Down Expand Up @@ -356,7 +357,8 @@ class VectorType;

// Load/Store of dual registers
LDRD,
STRD
STRD,
LAST_MEMORY_OPCODE = STRD,
};

} // end namespace ARMISD
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Target/ARM/ARMSelectionDAGInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ cl::opt<TPLoop::MemTransfer> EnableMemtransferTPLoop(
"Allow (may be subject to certain conditions) "
"conversion of memcpy to TP loop.")));

bool ARMSelectionDAGInfo::isTargetMemoryOpcode(unsigned Opcode) const {
return Opcode >= ARMISD::FIRST_MEMORY_OPCODE &&
Opcode <= ARMISD::LAST_MEMORY_OPCODE;
}

// Emit, if possible, a specialized version of the given Libcall. Typically this
// means selecting the appropriately aligned version, but we also convert memset
// of 0 into memclr.
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/ARM/ARMSelectionDAGInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ namespace ARM_AM {

class ARMSelectionDAGInfo : public SelectionDAGTargetInfo {
public:
bool isTargetMemoryOpcode(unsigned Opcode) const override;

SDValue EmitTargetCodeForMemcpy(SelectionDAG &DAG, const SDLoc &dl,
SDValue Chain, SDValue Dst, SDValue Src,
SDValue Size, Align Alignment,
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Target/Mips/MipsISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,16 @@ class TargetRegisterClass;
DOUBLE_SELECT_I64,

// Load/Store Left/Right nodes.
LWL = ISD::FIRST_TARGET_MEMORY_OPCODE,
FIRST_MEMORY_OPCODE,
LWL = FIRST_MEMORY_OPCODE,
LWR,
SWL,
SWR,
LDL,
LDR,
SDL,
SDR
SDR,
LAST_MEMORY_OPCODE = SDR,
};

} // ene namespace MipsISD
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/Mips/MipsSelectionDAGInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
//===----------------------------------------------------------------------===//

#include "MipsSelectionDAGInfo.h"
#include "MipsISelLowering.h"

using namespace llvm;

MipsSelectionDAGInfo::~MipsSelectionDAGInfo() = default;

bool MipsSelectionDAGInfo::isTargetMemoryOpcode(unsigned Opcode) const {
return Opcode >= MipsISD::FIRST_MEMORY_OPCODE &&
Opcode <= MipsISD::LAST_MEMORY_OPCODE;
}
2 changes: 2 additions & 0 deletions llvm/lib/Target/Mips/MipsSelectionDAGInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace llvm {
class MipsSelectionDAGInfo : public SelectionDAGTargetInfo {
public:
~MipsSelectionDAGInfo() override;

bool isTargetMemoryOpcode(unsigned Opcode) const override;
};

} // namespace llvm
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/Target/NVPTX/NVPTXISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ enum NodeType : unsigned {
BrxEnd,
Dummy,

LoadV2 = ISD::FIRST_TARGET_MEMORY_OPCODE,
FIRST_MEMORY_OPCODE,
LoadV2 = FIRST_MEMORY_OPCODE,
LoadV4,
LDUV2, // LDU.v2
LDUV4, // LDU.v4
Expand All @@ -87,6 +88,7 @@ enum NodeType : unsigned {
StoreRetval,
StoreRetvalV2,
StoreRetvalV4,
LAST_MEMORY_OPCODE = StoreRetvalV4,
};
}

Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/NVPTX/NVPTXSelectionDAGInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
//===----------------------------------------------------------------------===//

#include "NVPTXSelectionDAGInfo.h"
#include "NVPTXISelLowering.h"

using namespace llvm;

NVPTXSelectionDAGInfo::~NVPTXSelectionDAGInfo() = default;

bool NVPTXSelectionDAGInfo::isTargetMemoryOpcode(unsigned Opcode) const {
return Opcode >= NVPTXISD::FIRST_MEMORY_OPCODE &&
Opcode <= NVPTXISD::LAST_MEMORY_OPCODE;
}
2 changes: 2 additions & 0 deletions llvm/lib/Target/NVPTX/NVPTXSelectionDAGInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace llvm {
class NVPTXSelectionDAGInfo : public SelectionDAGTargetInfo {
public:
~NVPTXSelectionDAGInfo() override;

bool isTargetMemoryOpcode(unsigned Opcode) const override;
};

} // namespace llvm
Expand Down
Loading
Loading