Skip to content

Commit 6aa5c1d

Browse files
kpetkpet
authored
Add validation for Vulkan standalone env rules for SPV_ARM_graph (KhronosGroup#6694)
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
1 parent 2d7060b commit 6aa5c1d

3 files changed

Lines changed: 127 additions & 4 deletions

File tree

‎source/val/validate_graph.cpp‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <deque>
1818

1919
#include "source/opcode.h"
20+
#include "source/spirv_target_env.h"
2021
#include "source/val/validate.h"
2122
#include "source/val/validation_state.h"
2223

@@ -57,6 +58,11 @@ bool IsGraphType(ValidationState_t& _, uint32_t id) {
5758
return true;
5859
}
5960

61+
bool IsConstantInstruction(ValidationState_t& _, uint32_t id) {
62+
auto def = _.FindDef(id);
63+
return def && spvOpcodeIsConstant(def->opcode());
64+
}
65+
6066
const uint32_t kGraphTypeIOStartWord = 3;
6167

6268
uint32_t GraphTypeInstNumIO(const Instruction* inst) {
@@ -214,8 +220,9 @@ spv_result_t ValidateGraph(ValidationState_t& _, const Instruction* inst) {
214220
}
215221

216222
spv_result_t ValidateGraphInput(ValidationState_t& _, const Instruction* inst) {
223+
const uint32_t input_index_id = inst->GetOperandAs<uint32_t>(2);
217224
// Check type of InputIndex
218-
auto input_index_inst = _.FindDef(inst->GetOperandAs<uint32_t>(2));
225+
auto input_index_inst = _.FindDef(input_index_id);
219226
if (!input_index_inst ||
220227
!_.IsIntScalarType(input_index_inst->type_id(), 32)) {
221228
return _.diag(SPV_ERROR_INVALID_DATA, inst)
@@ -226,8 +233,10 @@ spv_result_t ValidateGraphInput(ValidationState_t& _, const Instruction* inst) {
226233
bool has_element_index = inst->operands().size() > 3;
227234

228235
// Check type of ElementIndex
236+
uint32_t element_index_id = 0;
229237
if (has_element_index) {
230-
auto element_index_inst = _.FindDef(inst->GetOperandAs<uint32_t>(3));
238+
element_index_id = inst->GetOperandAs<uint32_t>(3);
239+
auto element_index_inst = _.FindDef(element_index_id);
231240
if (!element_index_inst ||
232241
!_.IsIntScalarType(element_index_inst->type_id(), 32)) {
233242
return _.diag(SPV_ERROR_INVALID_DATA, inst)
@@ -236,6 +245,19 @@ spv_result_t ValidateGraphInput(ValidationState_t& _, const Instruction* inst) {
236245
}
237246
}
238247

248+
if (spvIsVulkanEnv(_.context()->target_env)) {
249+
if (!IsConstantInstruction(_, input_index_id)) {
250+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
251+
<< _.VkErrorID(9931) << "OpGraphInputARM InputIndex must be the "
252+
<< "<id> of a constant instruction.";
253+
}
254+
if (has_element_index && !IsConstantInstruction(_, element_index_id)) {
255+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
256+
<< _.VkErrorID(9931) << "OpGraphInputARM ElementIndex must be "
257+
<< "the <id> of a constant instruction.";
258+
}
259+
}
260+
239261
// Find graph definition
240262
size_t inst_num = inst->LineNum() - 1;
241263
auto graph_inst = &_.ordered_instructions()[inst_num];
@@ -328,8 +350,9 @@ spv_result_t ValidateGraphInput(ValidationState_t& _, const Instruction* inst) {
328350

329351
spv_result_t ValidateGraphSetOutput(ValidationState_t& _,
330352
const Instruction* inst) {
353+
const uint32_t output_index_id = inst->GetOperandAs<uint32_t>(1);
331354
// Check type of OutputIndex
332-
auto output_index_inst = _.FindDef(inst->GetOperandAs<uint32_t>(1));
355+
auto output_index_inst = _.FindDef(output_index_id);
333356
if (!output_index_inst ||
334357
!_.IsIntScalarType(output_index_inst->type_id(), 32)) {
335358
return _.diag(SPV_ERROR_INVALID_DATA, inst)
@@ -340,8 +363,10 @@ spv_result_t ValidateGraphSetOutput(ValidationState_t& _,
340363
bool has_element_index = inst->operands().size() > 2;
341364

342365
// Check type of ElementIndex
366+
uint32_t element_index_id = 0;
343367
if (has_element_index) {
344-
auto element_index_inst = _.FindDef(inst->GetOperandAs<uint32_t>(2));
368+
element_index_id = inst->GetOperandAs<uint32_t>(2);
369+
auto element_index_inst = _.FindDef(element_index_id);
345370
if (!element_index_inst ||
346371
!_.IsIntScalarType(element_index_inst->type_id(), 32)) {
347372
return _.diag(SPV_ERROR_INVALID_DATA, inst)
@@ -350,6 +375,19 @@ spv_result_t ValidateGraphSetOutput(ValidationState_t& _,
350375
}
351376
}
352377

378+
if (spvIsVulkanEnv(_.context()->target_env)) {
379+
if (!IsConstantInstruction(_, output_index_id)) {
380+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
381+
<< _.VkErrorID(9932) << "OpGraphSetOutputARM OutputIndex must "
382+
<< "be the <id> of a constant instruction.";
383+
}
384+
if (has_element_index && !IsConstantInstruction(_, element_index_id)) {
385+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
386+
<< _.VkErrorID(9932) << "OpGraphSetOutputARM ElementIndex must "
387+
<< "be the <id> of a constant instruction.";
388+
}
389+
}
390+
353391
// Find graph definition
354392
size_t inst_num = inst->LineNum() - 1;
355393
auto graph_inst = &_.ordered_instructions()[inst_num];

‎source/val/validation_state.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3285,6 +3285,10 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
32853285
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-09658);
32863286
case 9659:
32873287
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-09659);
3288+
case 9931:
3289+
return VUID_WRAP(VUID-StandaloneSpirv-OpGraphInputARM-09931);
3290+
case 9932:
3291+
return VUID_WRAP(VUID-StandaloneSpirv-OpGraphSetOutputARM-09932);
32883292
case 10151:
32893293
return VUID_WRAP(VUID-StandaloneSpirv-DerivativeGroupQuadsKHR-10151);
32903294
case 10152:

‎test/val/val_graph_test.cpp‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ std::string GenerateModuleWithExtInstImports(const std::string& imports,
6464
%uint_2 = OpConstant %uint 2
6565
%uint_3 = OpConstant %uint 3
6666
%uint_4 = OpConstant %uint 4
67+
%undef_uint = OpUndef %uint
6768
%float_1 = OpConstant %float 1.0
6869
%int8tensor = OpTypeTensorARM %int8 %uint_4
6970
%int8r3tensor = OpTypeTensorARM %int8 %uint_3
@@ -778,6 +779,26 @@ TEST_F(ValidateGraph, InvalidGraphInputIndexWrongType) {
778779
HasSubstr("GraphInputARM InputIndex must be a 32-bit integer"));
779780
}
780781

782+
TEST_F(ValidateGraph, InvalidGraphInputIndexNotConstantVulkan) {
783+
const std::string src = R"(
784+
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor
785+
OpGraphEntryPointARM %graph "longname" %var_int8tensor %var_int8tensor
786+
%graph = OpGraphARM %graph_type
787+
%in = OpGraphInputARM %int8tensor %undef_uint
788+
OpGraphSetOutputARM %in %uint_0
789+
OpGraphEndARM
790+
)";
791+
std::string spvasm = GenerateModule(src);
792+
793+
CompileSuccessfully(spvasm, SPVENV);
794+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPVENV));
795+
EXPECT_THAT(getDiagnosticString(),
796+
HasSubstr("VUID-StandaloneSpirv-OpGraphInputARM-09931"));
797+
EXPECT_THAT(getDiagnosticString(),
798+
HasSubstr("OpGraphInputARM InputIndex must be the <id> of a "
799+
"constant instruction."));
800+
}
801+
781802
TEST_F(ValidateGraph, InvalidGraphInputElementIndexWrongType) {
782803
const std::string src = R"(
783804
%graph_type = OpTypeGraphARM 1 %int8tensor_array3 %int8tensor
@@ -795,6 +816,26 @@ TEST_F(ValidateGraph, InvalidGraphInputElementIndexWrongType) {
795816
HasSubstr("GraphInputARM ElementIndex must be a 32-bit integer"));
796817
}
797818

819+
TEST_F(ValidateGraph, InvalidGraphInputElementIndexNotConstantVulkan) {
820+
const std::string src = R"(
821+
%graph_type = OpTypeGraphARM 1 %int8tensor_array3 %int8tensor
822+
OpGraphEntryPointARM %graph "longname" %var_int8tensor_array3 %var_int8tensor
823+
%graph = OpGraphARM %graph_type
824+
%in = OpGraphInputARM %int8tensor %uint_0 %undef_uint
825+
OpGraphSetOutputARM %in %uint_0
826+
OpGraphEndARM
827+
)";
828+
std::string spvasm = GenerateModule(src);
829+
830+
CompileSuccessfully(spvasm, SPVENV);
831+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPVENV));
832+
EXPECT_THAT(getDiagnosticString(),
833+
HasSubstr("VUID-StandaloneSpirv-OpGraphInputARM-09931"));
834+
EXPECT_THAT(getDiagnosticString(),
835+
HasSubstr("OpGraphInputARM ElementIndex must be the <id> of a "
836+
"constant instruction."));
837+
}
838+
798839
TEST_F(ValidateGraph, InvalidGraphInputIndexDuplicate) {
799840
const std::string src = R"(
800841
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor
@@ -1123,6 +1164,26 @@ TEST_F(ValidateGraph, InvalidGraphOutputIndexWrongType) {
11231164
HasSubstr("GraphSetOutputARM OutputIndex must be a 32-bit integer"));
11241165
}
11251166

1167+
TEST_F(ValidateGraph, InvalidGraphOutputIndexNotConstantVulkan) {
1168+
const std::string src = R"(
1169+
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor
1170+
OpGraphEntryPointARM %graph "longname" %var_int8tensor %var_int8tensor
1171+
%graph = OpGraphARM %graph_type
1172+
%in = OpGraphInputARM %int8tensor %uint_0
1173+
OpGraphSetOutputARM %in %undef_uint
1174+
OpGraphEndARM
1175+
)";
1176+
std::string spvasm = GenerateModule(src);
1177+
1178+
CompileSuccessfully(spvasm, SPVENV);
1179+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPVENV));
1180+
EXPECT_THAT(getDiagnosticString(),
1181+
HasSubstr("VUID-StandaloneSpirv-OpGraphSetOutputARM-09932"));
1182+
EXPECT_THAT(getDiagnosticString(),
1183+
HasSubstr("OpGraphSetOutputARM OutputIndex must be the <id> of "
1184+
"a constant instruction."));
1185+
}
1186+
11261187
TEST_F(ValidateGraph, InvalidGraphOutputElementIndexWrongType) {
11271188
const std::string src = R"(
11281189
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor_array3
@@ -1141,6 +1202,26 @@ TEST_F(ValidateGraph, InvalidGraphOutputElementIndexWrongType) {
11411202
HasSubstr("GraphSetOutputARM ElementIndex must be a 32-bit integer"));
11421203
}
11431204

1205+
TEST_F(ValidateGraph, InvalidGraphOutputElementIndexNotConstantVulkan) {
1206+
const std::string src = R"(
1207+
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor_array3
1208+
OpGraphEntryPointARM %graph "longname" %var_int8tensor %var_int8tensor_array3
1209+
%graph = OpGraphARM %graph_type
1210+
%in = OpGraphInputARM %int8tensor %uint_0
1211+
OpGraphSetOutputARM %in %uint_0 %undef_uint
1212+
OpGraphEndARM
1213+
)";
1214+
std::string spvasm = GenerateModule(src);
1215+
1216+
CompileSuccessfully(spvasm, SPVENV);
1217+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPVENV));
1218+
EXPECT_THAT(getDiagnosticString(),
1219+
HasSubstr("VUID-StandaloneSpirv-OpGraphSetOutputARM-09932"));
1220+
EXPECT_THAT(getDiagnosticString(),
1221+
HasSubstr("OpGraphSetOutputARM ElementIndex must be the <id> of "
1222+
"a constant instruction."));
1223+
}
1224+
11441225
TEST_F(ValidateGraph, InvalidGraphOutputIndexDuplicate) {
11451226
const std::string src = R"(
11461227
%graph_type = OpTypeGraphARM 1 %int8tensor %int8tensor

0 commit comments

Comments
 (0)