Skip to content

Commit 8a13595

Browse files
dnovillodnovillo
authored
spirv-val: Allow spec constants as DebugTypeArray component counts (KhronosGroup#6608)
This updates the validator to match the spec update in KhronosGroup/SPIRV-Registry#371 to allow any constant instruction as the component count operand of `DebugTypeArray`. It fixes the glslang issue KhronosGroup/glslang#4186. The constant check now determines whether it's checking for OpenCL.DebugInfo.100 or NSDI. The new rule only affects NSDI.
1 parent 93cde4f commit 8a13595

File tree

2 files changed

+107
-14
lines changed

2 files changed

+107
-14
lines changed

‎source/val/validate_extensions.cpp‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "source/extensions.h"
2626
#include "source/latest_version_glsl_std_450_header.h"
2727
#include "source/latest_version_opencl_std_header.h"
28+
#include "source/opcode.h"
2829
#include "source/spirv_constant.h"
2930
#include "source/table2.h"
3031
#include "source/val/instruction.h"
@@ -1044,6 +1045,14 @@ bool IsConstIntScalarTypeWith32Or64Bits(ValidationState_t& _,
10441045
return size_in_bits == 32 || size_in_bits == 64;
10451046
}
10461047

1048+
bool IsSpecConstIntScalarTypeWith32Or64Bits(ValidationState_t& _,
1049+
Instruction* instr) {
1050+
if (!spvOpcodeIsSpecConstant(instr->opcode())) return false;
1051+
if (!_.IsIntScalarType(instr->type_id())) return false;
1052+
uint32_t size_in_bits = _.GetBitWidth(instr->type_id());
1053+
return size_in_bits == 32 || size_in_bits == 64;
1054+
}
1055+
10471056
bool IsConstWithIntScalarType(ValidationState_t& _, const Instruction* inst,
10481057
uint32_t word_index) {
10491058
auto* int_scalar_const = _.FindDef(inst->word(word_index));
@@ -3424,6 +3433,10 @@ spv_result_t ValidateExtInstDebugInfo(ValidationState_t& _,
34243433
if (!vulkanDebugInfo && !component_count->word(3)) {
34253434
invalid = true;
34263435
}
3436+
} else if (vulkanDebugInfo && IsSpecConstIntScalarTypeWith32Or64Bits(
3437+
_, component_count)) {
3438+
// Spec constants are valid component counts for
3439+
// NonSemantic.Shader.DebugInfo.
34273440
} else if (component_count->words().size() > 6 &&
34283441
(CommonDebugInfoInstructions(component_count->word(4)) ==
34293442
CommonDebugInfoDebugLocalVariable ||
@@ -3461,8 +3474,9 @@ spv_result_t ValidateExtInstDebugInfo(ValidationState_t& _,
34613474
if (invalid) {
34623475
return _.diag(SPV_ERROR_INVALID_DATA, inst)
34633476
<< GetExtInstName(_, inst) << ": Component Count must be "
3464-
<< "OpConstant with a 32- or 64-bits integer scalar type "
3465-
"or "
3477+
<< (vulkanDebugInfo ? "a constant instruction"
3478+
: "OpConstant")
3479+
<< " with a 32- or 64-bits integer scalar type or "
34663480
<< "DebugGlobalVariable or DebugLocalVariable with a 32- "
34673481
"or "
34683482
<< "64-bits unsigned integer scalar type";

‎test/val/val_ext_inst_debug_test.cpp‎

Lines changed: 91 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,36 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugTypeArrayFailVariableSizeTypeFloat) {
12371237
"integer scalar type"));
12381238
}
12391239

1240+
TEST_F(ValidateOpenCL100DebugInfo,
1241+
DebugTypeArrayOpSpecConstantComponentCountFail) {
1242+
const std::string src = R"(
1243+
%src = OpString "simple.hlsl"
1244+
%code = OpString "main() {}"
1245+
%float_name = OpString "float"
1246+
)";
1247+
1248+
const std::string size_const = R"(
1249+
%int_32 = OpConstant %u32 32
1250+
%spec_u32 = OpSpecConstant %u32 4
1251+
)";
1252+
1253+
const std::string dbg_inst_header = R"(
1254+
%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
1255+
%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit 2 4 %dbg_src HLSL
1256+
%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %int_32 Float
1257+
%float_arr_info = OpExtInst %void %DbgExt DebugTypeArray %float_info %spec_u32
1258+
)";
1259+
1260+
CompileSuccessfully(GenerateShaderCodeForDebugInfo(
1261+
src, size_const, dbg_inst_header, "", opencl_extension, "Vertex"));
1262+
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
1263+
EXPECT_THAT(getDiagnosticString(),
1264+
HasSubstr("Component Count must be OpConstant with a 32- or "
1265+
"64-bits integer scalar type or DebugGlobalVariable or "
1266+
"DebugLocalVariable with a 32- or 64-bits unsigned "
1267+
"integer scalar type"));
1268+
}
1269+
12401270
TEST_F(ValidateVulkan100DebugInfo, DebugTypeArray) {
12411271
const std::string src = R"(
12421272
%src = OpString "simple.hlsl"
@@ -1325,10 +1355,10 @@ TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayFailComponentCount) {
13251355
src, "", dbg_inst_header, "", shader_extension, "Vertex"));
13261356
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
13271357
EXPECT_THAT(getDiagnosticString(),
1328-
HasSubstr("Component Count must be OpConstant with a 32- or "
1329-
"64-bits integer scalar type or DebugGlobalVariable or "
1330-
"DebugLocalVariable with a 32- or 64-bits unsigned "
1331-
"integer scalar type"));
1358+
HasSubstr("Component Count must be a constant instruction with a "
1359+
"32- or 64-bits integer scalar type or "
1360+
"DebugGlobalVariable or DebugLocalVariable with a 32- "
1361+
"or 64-bits unsigned integer scalar type"));
13321362
}
13331363

13341364
TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayFailComponentCountFloat) {
@@ -1349,10 +1379,10 @@ TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayFailComponentCountFloat) {
13491379
src, "", dbg_inst_header, "", shader_extension, "Vertex"));
13501380
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
13511381
EXPECT_THAT(getDiagnosticString(),
1352-
HasSubstr("Component Count must be OpConstant with a 32- or "
1353-
"64-bits integer scalar type or DebugGlobalVariable or "
1354-
"DebugLocalVariable with a 32- or 64-bits unsigned "
1355-
"integer scalar type"));
1382+
HasSubstr("Component Count must be a constant instruction with a "
1383+
"32- or 64-bits integer scalar type or "
1384+
"DebugGlobalVariable or DebugLocalVariable with a 32- "
1385+
"or 64-bits unsigned integer scalar type"));
13561386
}
13571387

13581388
TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayComponentCountZero) {
@@ -1400,10 +1430,59 @@ TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayFailVariableSizeTypeFloat) {
14001430
src, constants, dbg_inst_header, "", shader_extension, "Vertex"));
14011431
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
14021432
EXPECT_THAT(getDiagnosticString(),
1403-
HasSubstr("Component Count must be OpConstant with a 32- or "
1404-
"64-bits integer scalar type or DebugGlobalVariable or "
1405-
"DebugLocalVariable with a 32- or 64-bits unsigned "
1406-
"integer scalar type"));
1433+
HasSubstr("Component Count must be a constant instruction with a "
1434+
"32- or 64-bits integer scalar type or "
1435+
"DebugGlobalVariable or DebugLocalVariable with a 32- "
1436+
"or 64-bits unsigned integer scalar type"));
1437+
}
1438+
1439+
TEST_F(ValidateVulkan100DebugInfo, DebugTypeArrayOpSpecConstantComponentCount) {
1440+
const std::string src = R"(
1441+
%src = OpString "simple.hlsl"
1442+
%code = OpString "main() {}"
1443+
%float_name = OpString "float"
1444+
)";
1445+
1446+
const std::string constants = R"(
1447+
%spec_u32 = OpSpecConstant %u32 4
1448+
)";
1449+
1450+
const std::string dbg_inst_header = R"(
1451+
%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
1452+
%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit %u32_2 %u32_4 %dbg_src %u32_5
1453+
%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %u32_32 %u32_3 %u32_0
1454+
%float_arr_info = OpExtInst %void %DbgExt DebugTypeArray %float_info %spec_u32
1455+
)";
1456+
1457+
CompileSuccessfully(GenerateShaderCodeForDebugInfo(
1458+
src, constants, dbg_inst_header, "", shader_extension, "Vertex"));
1459+
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
1460+
}
1461+
1462+
TEST_F(ValidateVulkan100DebugInfo,
1463+
DebugTypeArrayOpSpecConstantOpComponentCount) {
1464+
const std::string src = R"(
1465+
%src = OpString "simple.hlsl"
1466+
%code = OpString "main() {}"
1467+
%float_name = OpString "float"
1468+
)";
1469+
1470+
const std::string constants = R"(
1471+
%spec_a = OpSpecConstant %u32 2
1472+
%spec_b = OpSpecConstant %u32 2
1473+
%spec_op = OpSpecConstantOp %u32 IMul %spec_a %spec_b
1474+
)";
1475+
1476+
const std::string dbg_inst_header = R"(
1477+
%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
1478+
%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit %u32_2 %u32_4 %dbg_src %u32_5
1479+
%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %u32_32 %u32_3 %u32_0
1480+
%float_arr_info = OpExtInst %void %DbgExt DebugTypeArray %float_info %spec_op
1481+
)";
1482+
1483+
CompileSuccessfully(GenerateShaderCodeForDebugInfo(
1484+
src, constants, dbg_inst_header, "", shader_extension, "Vertex"));
1485+
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
14071486
}
14081487

14091488
TEST_F(ValidateOpenCL100DebugInfo, DebugTypeVector) {

0 commit comments

Comments
 (0)