Skip to content

Commit 60a1c85

Browse files
authored
Add HL lower table validation and opcode json dependency (microsoft#8004)
It's easy to get gLowerTable out of order in HLOperationLower.cpp. That can result in lowering an HLSL intrinsic to the wrong DXIL op. This change adds static_assert validation for the opcode table, which will report the specific index where the opcode didn't match. Validation caught two out-of-order ops in the table, which are also fixed in this change. The change also adds hlsl_intrinsic_opcodes.json to dependencies in add_hlsl_hctgen, since the `hlsl::IntrinsicOp` enum is dependent on it.
1 parent 1dd274e commit 60a1c85

2 files changed

Lines changed: 37 additions & 9 deletions

File tree

cmake/modules/HCT.cmake

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,13 @@ function(add_hlsl_hctgen mode)
5858
set(hctgen ${LLVM_SOURCE_DIR}/utils/hct/hctgen.py)
5959
set(hctdb ${LLVM_SOURCE_DIR}/utils/hct/hctdb.py)
6060
set(hctdb_helper ${LLVM_SOURCE_DIR}/utils/hct/hctdb_instrhelp.py)
61+
set(opcodes_json ${LLVM_SOURCE_DIR}/utils/hct/hlsl_intrinsic_opcodes.json)
6162
set(output ${full_output})
6263
set(hct_dependencies ${LLVM_SOURCE_DIR}/utils/hct/gen_intrin_main.txt
6364
${hctgen}
6465
${hctdb}
65-
${hctdb_helper})
66+
${hctdb_helper}
67+
${opcodes_json})
6668

6769
get_filename_component(output_extension ${full_output} LAST_EXT)
6870

lib/HLSL/HLOperationLower.cpp

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6829,7 +6829,7 @@ Value *StreamOutputLower(CallInst *CI, IntrinsicOp IOP, DXIL::OpCode opcode,
68296829
}
68306830

68316831
// This table has to match IntrinsicOp orders
6832-
IntrinsicLower gLowerTable[] = {
6832+
constexpr IntrinsicLower gLowerTable[] = {
68336833
{IntrinsicOp::IOP_AcceptHitAndEndSearch,
68346834
TranslateNoArgNoReturnPreserveOutput, DXIL::OpCode::AcceptHitAndEndSearch},
68356835
{IntrinsicOp::IOP_AddUint64, TranslateAddUint64, DXIL::OpCode::UAddc},
@@ -7440,10 +7440,10 @@ IntrinsicLower gLowerTable[] = {
74407440
DXIL::OpCode::HitObject_MakeNop},
74417441
{IntrinsicOp::IOP_DxMaybeReorderThread, TranslateMaybeReorderThread,
74427442
DXIL::OpCode::MaybeReorderThread},
7443-
{IntrinsicOp::IOP_Vkstatic_pointer_cast, UnsupportedVulkanIntrinsic,
7444-
DXIL::OpCode::NumOpCodes},
74457443
{IntrinsicOp::IOP_Vkreinterpret_pointer_cast, UnsupportedVulkanIntrinsic,
74467444
DXIL::OpCode::NumOpCodes},
7445+
{IntrinsicOp::IOP_Vkstatic_pointer_cast, UnsupportedVulkanIntrinsic,
7446+
DXIL::OpCode::NumOpCodes},
74477447
{IntrinsicOp::MOP_GetBufferContents, UnsupportedVulkanIntrinsic,
74487448
DXIL::OpCode::NumOpCodes},
74497449
{IntrinsicOp::MOP_DxHitObject_FromRayQuery, TranslateHitObjectFromRayQuery,
@@ -7515,10 +7515,10 @@ IntrinsicLower gLowerTable[] = {
75157515

75167516
{IntrinsicOp::IOP_isnormal, TrivialIsSpecialFloat, DXIL::OpCode::IsNormal},
75177517

7518-
{IntrinsicOp::IOP_GetGroupWaveIndex, EmptyLower,
7519-
DXIL::OpCode::GetGroupWaveIndex},
75207518
{IntrinsicOp::IOP_GetGroupWaveCount, EmptyLower,
75217519
DXIL::OpCode::GetGroupWaveCount},
7520+
{IntrinsicOp::IOP_GetGroupWaveIndex, EmptyLower,
7521+
DXIL::OpCode::GetGroupWaveIndex},
75227522

75237523
{IntrinsicOp::IOP_ClusterID, EmptyLower, DXIL::OpCode::ClusterID},
75247524
{IntrinsicOp::MOP_CandidateClusterID, EmptyLower,
@@ -7537,12 +7537,38 @@ IntrinsicLower gLowerTable[] = {
75377537
{IntrinsicOp::MOP_DxHitObject_TriangleObjectPosition, EmptyLower,
75387538
DXIL::OpCode::HitObject_TriangleObjectPosition},
75397539
};
7540-
} // namespace
7540+
constexpr size_t NumLowerTableEntries =
7541+
sizeof(gLowerTable) / sizeof(gLowerTable[0]);
75417542
static_assert(
7542-
sizeof(gLowerTable) / sizeof(gLowerTable[0]) ==
7543-
static_cast<size_t>(IntrinsicOp::Num_Intrinsics),
7543+
NumLowerTableEntries == static_cast<size_t>(IntrinsicOp::Num_Intrinsics),
75447544
"Intrinsic lowering table must be updated to account for new intrinsics.");
75457545

7546+
// Make table-order failures report the bad index via template instantiation
7547+
// parameter in the diagnostic.
7548+
// On failure, use hlsl_intrinsic_opcodes.json to find the mismatch.
7549+
template <size_t I> struct ValidateLowerTableEntry {
7550+
// Instantiate a type that fails if the opcode doesn't match the index.
7551+
static_assert(
7552+
I == static_cast<size_t>(gLowerTable[I].IntriOpcode),
7553+
"Intrinsic lowering table is out of order. "
7554+
"See ValidateLowerTableEntry<I> template instantiation for Index.");
7555+
static constexpr bool Value =
7556+
I == static_cast<size_t>(gLowerTable[I].IntriOpcode);
7557+
};
7558+
7559+
template <size_t I, size_t N> struct ValidateLowerTableImpl {
7560+
static constexpr bool Value = ValidateLowerTableEntry<I>::Value &&
7561+
ValidateLowerTableImpl<I + 1, N>::Value;
7562+
};
7563+
7564+
template <size_t N> struct ValidateLowerTableImpl<N, N> {
7565+
static constexpr bool Value = true;
7566+
};
7567+
7568+
static_assert(ValidateLowerTableImpl<0, NumLowerTableEntries>::Value,
7569+
"Intrinsic lowering table is out of order.");
7570+
} // namespace
7571+
75467572
static void TranslateBuiltinIntrinsic(CallInst *CI,
75477573
HLOperationLowerHelper &helper,
75487574
HLObjectOperationLowerHelper *pObjHelper,

0 commit comments

Comments
 (0)