Skip to content

Commit d344926

Browse files
spirv-val: Check for image declaration with TileImageEXT storage class (KhronosGroup#6662)
Addressing a missing check mentioned [here and closes](KhronosGroup#6593). > The TileImageEXT Storage Class must only be used for declaring tile image variables
1 parent d8cc2f8 commit d344926

3 files changed

Lines changed: 66 additions & 3 deletions

File tree

source/val/validate_memory.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,32 @@ spv_result_t ValidateVariableTileShadingQCOM(ValidationState_t& _,
11281128
return SPV_SUCCESS;
11291129
}
11301130

1131+
spv_result_t ValidateVariableTileImageEXT(ValidationState_t& _,
1132+
const Instruction* inst) {
1133+
bool is_valid_decl = true;
1134+
1135+
auto result_type = _.FindDef(inst->type_id());
1136+
if (result_type->opcode() == spv::Op::OpTypePointer) {
1137+
const auto pointee_type = _.FindDef(result_type->GetOperandAs<uint32_t>(2));
1138+
if (pointee_type && pointee_type->opcode() == spv::Op::OpTypeImage) {
1139+
spv::Dim dim = static_cast<spv::Dim>(pointee_type->word(3));
1140+
if (dim != spv::Dim::TileImageDataEXT) {
1141+
is_valid_decl = false;
1142+
}
1143+
} else {
1144+
is_valid_decl = false;
1145+
}
1146+
}
1147+
1148+
if (!is_valid_decl) {
1149+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
1150+
<< "The TileImageEXT Storage Class must only be used for declaring "
1151+
"tile image variables";
1152+
} else {
1153+
return SPV_SUCCESS;
1154+
}
1155+
}
1156+
11311157
spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) {
11321158
const bool untyped_pointer = inst->opcode() == spv::Op::OpUntypedVariableKHR;
11331159

@@ -1246,6 +1272,11 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) {
12461272
if (auto error = ValidateVariableTileShadingQCOM(_, inst)) return error;
12471273
}
12481274

1275+
if (_.HasCapability(spv::Capability::TileImageColorReadAccessEXT) &&
1276+
storage_class == spv::StorageClass::TileImageEXT) {
1277+
if (auto error = ValidateVariableTileImageEXT(_, inst)) return error;
1278+
}
1279+
12491280
return SPV_SUCCESS;
12501281
}
12511282

test/val/val_image_test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11477,12 +11477,12 @@ TEST_F(ValidateImage, TileImageNotFragment) {
1147711477
%void = OpTypeVoid
1147811478
%func = OpTypeFunction %void
1147911479
%float = OpTypeFloat 32
11480-
%v4float = OpTypeVector %float 4
11481-
%ptr = OpTypePointer TileImageEXT %v4float
11480+
%image = OpTypeImage %float TileImageDataEXT 0 0 0 2 Unknown
11481+
%ptr = OpTypePointer TileImageEXT %image
1148211482
%var = OpVariable %ptr TileImageEXT
1148311483
%main = OpFunction %void None %func
1148411484
%label = OpLabel
11485-
%val = OpLoad %v4float %var
11485+
%val = OpLoad %image %var
1148611486
OpReturn
1148711487
OpFunctionEnd
1148811488
)";

test/val/val_storage_test.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,38 @@ TEST_F(ValidateStorage, TileAttachmentQCOMBad6) {
437437
HasSubstr("requires one of these capabilities: TileShadingQCOM"));
438438
}
439439

440+
TEST_F(ValidateStorage, WrongDimTileImageEXT) {
441+
const std::string spirv = R"(
442+
OpCapability Shader
443+
OpCapability Sampled1D
444+
OpCapability TileImageColorReadAccessEXT
445+
OpExtension "SPV_EXT_shader_tile_image"
446+
OpMemoryModel Logical GLSL450
447+
OpEntryPoint Fragment %main "main"
448+
OpExecutionMode %main OriginUpperLeft
449+
OpSource GLSL 450
450+
%void = OpTypeVoid
451+
%int = OpTypeInt 32 1
452+
%img = OpTypeImage %int 1D 0 0 0 2 Rgba32i
453+
%ptr_img = OpTypePointer TileImageEXT %img
454+
%color1 = OpVariable %ptr_img TileImageEXT
455+
%3 = OpTypeFunction %void
456+
%main = OpFunction %void None %3
457+
%5 = OpLabel
458+
OpReturn
459+
OpFunctionEnd
460+
)";
461+
462+
spv_target_env env = SPV_ENV_VULKAN_1_4;
463+
CompileSuccessfully(spirv, env);
464+
EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions(env));
465+
EXPECT_THAT(
466+
getDiagnosticString(),
467+
HasSubstr(
468+
"The TileImageEXT Storage Class must only be used for declaring "
469+
"tile image variables"));
470+
}
471+
440472
std::string GenerateExecutionModelCode(const std::string& execution_model,
441473
const std::string& storage_class,
442474
bool store) {

0 commit comments

Comments
 (0)