Skip to content

Commit 355562d

Browse files
yoneymeta-codesync[bot]
authored andcommitted
Validate HIR instruction operand type declarations
Summary: This adds static checks that validate `INSTR_CLASS` operand type lists against their `Operands<N>` declarations at compile time, with an explicit dynamic operand typing marker for instructions that implement custom `GetOperandTypeImpl()` logic. Reviewed By: alexmalyshev Differential Revision: D110447034 fbshipit-source-id: d7cb61b5530f493db9a9436e838b2d69c7026d96
1 parent f202e33 commit 355562d

1 file changed

Lines changed: 60 additions & 11 deletions

File tree

cinderx/Jit/hir/hir.h

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <span>
1919
#include <string>
2020
#include <string_view>
21+
#include <type_traits>
2122
#include <unordered_map>
2223
#include <unordered_set>
2324
#include <vector>
@@ -99,6 +100,16 @@ struct OperandType {
99100

100101
std::ostream& operator<<(std::ostream& os, OperandType kind);
101102

103+
// Sentinel used in place of an operand-type list for instructions whose
104+
// operand types depend on instruction-specific state (e.g. Return's `type_`
105+
// member) and so must define their own `getOperandTypeImpl()`.
106+
struct DynamicOperandTypes {};
107+
constexpr DynamicOperandTypes kDynamicOperandTypes{};
108+
109+
constexpr DynamicOperandTypes makeTypeArray(DynamicOperandTypes) {
110+
return {};
111+
}
112+
102113
template <typename... Args>
103114
inline auto makeTypeArray(Args&&... args) {
104115
return std::array<OperandType, sizeof...(Args)>{
@@ -529,18 +540,48 @@ class InstrT<T, opcode, HasOutput, Tys...> : public InstrT<T, opcode, Tys...> {
529540
}
530541
};
531542

532-
// TASK(T105350013): Add a compile-time op_types size check.
533-
template <auto GetOperandTypes>
543+
template <typename... Ts>
544+
struct OperandArity : std::integral_constant<int, 0> {};
545+
546+
template <int N, typename... Ts>
547+
struct OperandArity<Operands<N>, Ts...> : std::integral_constant<int, N> {};
548+
549+
template <typename T, typename... Ts>
550+
struct OperandArity<T, Ts...> : OperandArity<Ts...> {};
551+
552+
template <auto GetOperandTypes, typename... Args>
534553
struct OperandTypes {
535-
OperandType getOperandTypeImpl(std::size_t i) const {
554+
using Types = decltype(GetOperandTypes());
555+
static constexpr int kArity = OperandArity<Args...>::value;
556+
static constexpr bool kDynamic = std::is_same_v<Types, DynamicOperandTypes>;
557+
static constexpr std::size_t kNumTypes = [] {
558+
if constexpr (kDynamic) {
559+
return std::size_t{0};
560+
} else {
561+
return std::tuple_size_v<Types>;
562+
}
563+
}();
564+
565+
static_assert(
566+
kDynamic || (kArity == kVariadic ? kNumTypes >= 1 : kArity == kNumTypes),
567+
"INSTR_CLASS operand type count does not match Operands<N>. TYPES "
568+
"describes input operands, not outputs. Use kDynamicOperandTypes for "
569+
"instruction-specific getOperandTypeImpl() logic.");
570+
571+
// If operand types are dynamic, the instruction class must define its own
572+
// getOperandTypeImpl().
573+
OperandType getOperandTypeImpl(std::size_t i) const
574+
requires(!kDynamic)
575+
{
536576
static const auto op_types = GetOperandTypes();
537577
return i < op_types.size() ? op_types[i] : op_types.back();
538578
}
539579
};
540580

541-
#define INSTR_CLASS(NAME, TYPES, ...) \
542-
NAME final : public InstrT<NAME, Opcode::k##NAME, __VA_ARGS__>, \
543-
public OperandTypes<[] { return makeTypeArray TYPES; }>
581+
#define INSTR_CLASS(NAME, TYPES, ...) \
582+
NAME final \
583+
: public InstrT<NAME, Opcode::k##NAME, __VA_ARGS__>, \
584+
public OperandTypes<[] { return makeTypeArray TYPES; }, __VA_ARGS__>
544585

545586
#define DEFINE_SIMPLE_INSTR(NAME, TYPES, ...) \
546587
class INSTR_CLASS(NAME, TYPES, __VA_ARGS__) { \
@@ -2117,7 +2158,11 @@ PrimitiveCompareOp ParsePrimitiveCompareOpName(std::string_view name);
21172158
// Convert a CompareOp into an equivalent PrimitiveCompareOp, if it exists.
21182159
std::optional<PrimitiveCompareOp> toPrimitiveCompareOp(CompareOp op);
21192160

2120-
class INSTR_CLASS(PrimitiveCompare, (), HasOutput, Operands<2>) {
2161+
class INSTR_CLASS(
2162+
PrimitiveCompare,
2163+
(kDynamicOperandTypes),
2164+
HasOutput,
2165+
Operands<2>) {
21212166
public:
21222167
PrimitiveCompare(
21232168
Register* dst,
@@ -2156,7 +2201,7 @@ DEFINE_SIMPLE_INSTR(PrimitiveBoxBool, (TCBool), HasOutput, Operands<1>);
21562201

21572202
class INSTR_CLASS(
21582203
PrimitiveBox,
2159-
(TPrimitive),
2204+
(kDynamicOperandTypes),
21602205
HasOutput,
21612206
Operands<1>,
21622207
DeoptBase) {
@@ -2188,7 +2233,11 @@ class INSTR_CLASS(
21882233
Type type_;
21892234
};
21902235

2191-
class INSTR_CLASS(PrimitiveUnbox, (), HasOutput, Operands<1>) {
2236+
class INSTR_CLASS(
2237+
PrimitiveUnbox,
2238+
(kDynamicOperandTypes),
2239+
HasOutput,
2240+
Operands<1>) {
21922241
public:
21932242
PrimitiveUnbox(Register* dst, Register* value, Type type)
21942243
: InstrT(dst, value), type_(type) {}
@@ -2834,7 +2883,7 @@ class INSTR_CLASS(RefineType, (TTop), HasOutput, Operands<1>) {
28342883
};
28352884

28362885
// Return from the function
2837-
class INSTR_CLASS(Return, (), Operands<1>) {
2886+
class INSTR_CLASS(Return, (kDynamicOperandTypes), Operands<1>) {
28382887
public:
28392888
explicit Return(Register* val) : InstrT(val), type_(TObject) {}
28402889
Return(Register* val, Type type) : InstrT(val), type_(type) {}
@@ -2857,7 +2906,7 @@ class INSTR_CLASS(Return, (), Operands<1>) {
28572906
//
28582907
// Ensures that we don't accidentally remove a type check (such as in GuardType)
28592908
// despite a register not having any explicit users
2860-
class INSTR_CLASS(UseType, (), Operands<1>) {
2909+
class INSTR_CLASS(UseType, (kDynamicOperandTypes), Operands<1>) {
28612910
public:
28622911
UseType(Register* val, Type type) : InstrT(val), type_(type) {}
28632912

0 commit comments

Comments
 (0)