Validate NonWritable decoration (#2263)

Also permit NonWritable on members of structs used for UBO and SSBO.
(That seems inadvertently removed in recent revisions of the spec.)
This commit is contained in:
David Neto 2019-01-28 12:44:13 -08:00 committed by GitHub
parent 9ab1c0ddd0
commit 7f3679a8b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 399 additions and 6 deletions

View File

@ -950,6 +950,14 @@ spv_result_t CheckDecorationsOfBuffers(ValidationState_t& vstate) {
const bool bufferRules =
(uniform && bufferDeco) || (push_constant && blockDeco) ||
((storage_buffer || phys_storage_buffer) && blockDeco);
if (uniform && blockDeco) {
vstate.RegisterPointerToUniformBlock(ptrInst->id());
vstate.RegisterStructForUniformBlock(id);
}
if ((uniform && bufferDeco) || (storage_buffer && blockDeco)) {
vstate.RegisterPointerToStorageBuffer(ptrInst->id());
vstate.RegisterStructForStorageBuffer(id);
}
if (blockRules || bufferRules) {
const char* deco_str = blockDeco ? "Block" : "BufferBlock";
spv_result_t recursive_status = SPV_SUCCESS;
@ -1219,6 +1227,49 @@ spv_result_t CheckFPRoundingModeForShaders(ValidationState_t& vstate,
return SPV_SUCCESS;
}
// Returns SPV_SUCCESS if validation rules are satisfied for the NonWritable
// decoration. Otherwise emits a diagnostic and returns something other than
// SPV_SUCCESS. The |inst| parameter is the object being decorated. This must
// be called after TypePass and AnnotateCheckDecorationsOfBuffers are called.
spv_result_t CheckNonWritableDecoration(ValidationState_t& vstate,
const Instruction& inst,
const Decoration& decoration) {
assert(inst.id() && "Parser ensures the target of the decoration has an ID");
if (decoration.struct_member_index() == Decoration::kInvalidMember) {
// The target must be a memory object declaration.
// First, it must be a variable or function parameter.
if (inst.opcode() != SpvOpVariable &&
inst.opcode() != SpvOpFunctionParameter) {
return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
<< "Target of NonWritable decoration must be a memory object "
"declaration (a variable or a function parameter)";
}
// Second, it must point to a UBO, SSBO, or storage image.
const auto type_id = inst.type_id();
if (!vstate.IsPointerToUniformBlock(type_id) &&
!vstate.IsPointerToStorageBuffer(type_id) &&
!vstate.IsPointerToStorageImage(type_id)) {
return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
<< "Target of NonWritable decoration is invalid: must point to a "
"storage image, uniform block, or storage buffer";
}
} else {
// The target is a struct member. The annotations pass already checks that
// it is a structure. So now fall through to ensure that the structure is
// used as a UBO or SSBO.
const auto type_id = inst.id();
if (!vstate.IsStructForUniformBlock(type_id) &&
!vstate.IsStructForStorageBuffer(type_id)) {
return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
<< "Target of NonWritable member decoration is invalid: must be "
"the struct type of a uniform block or storage buffer";
}
}
return SPV_SUCCESS;
}
// Returns SPV_SUCCESS if validation rules are satisfied for Uniform
// decorations. Otherwise emits a diagnostic and returns something other than
// SPV_SUCCESS. Assumes each decoration on a group has been propagated down to
@ -1314,6 +1365,9 @@ spv_result_t CheckDecorationsFromDecoration(ValidationState_t& vstate) {
if (is_shader)
PASS_OR_BAIL(CheckFPRoundingModeForShaders(vstate, *inst));
break;
case SpvDecorationNonWritable:
PASS_OR_BAIL(CheckNonWritableDecoration(vstate, *inst, decoration));
break;
case SpvDecorationUniform:
PASS_OR_BAIL(CheckUniformDecoration(vstate, *inst, decoration));
break;

View File

@ -269,6 +269,16 @@ spv_result_t ValidateTypePointer(ValidationState_t& _,
<< "OpTypePointer Type <id> '" << _.getIdName(type_id)
<< "' is not a type.";
}
// See if this points to a storage image.
if (type->opcode() == SpvOpTypeImage) {
const auto storage_class = inst->GetOperandAs<uint32_t>(1);
if (storage_class == SpvStorageClassUniformConstant) {
const auto sampled = type->GetOperandAs<uint32_t>(6);
// 2 indicates this image is known to be be used without a sampler, i.e.
// a storage image.
if (sampled == 2) _.RegisterPointerToStorageImage(inst->id());
}
}
return SPV_SUCCESS;
}

View File

@ -562,6 +562,63 @@ class ValidationState_t {
bool GetPointerTypeInfo(uint32_t id, uint32_t* data_type,
uint32_t* storage_class) const;
// Is the ID the type of a pointer to a uniform block: Block-decorated struct
// in uniform storage class? The result is only valid after internal method
// CheckDecorationsOfBuffers has been called.
bool IsPointerToUniformBlock(uint32_t type_id) const {
return pointer_to_uniform_block_.find(type_id) !=
pointer_to_uniform_block_.cend();
}
// Save the ID of a pointer to uniform block.
void RegisterPointerToUniformBlock(uint32_t type_id) {
pointer_to_uniform_block_.insert(type_id);
}
// Is the ID the type of a struct used as a uniform block?
// The result is only valid after internal method CheckDecorationsOfBuffers
// has been called.
bool IsStructForUniformBlock(uint32_t type_id) const {
return struct_for_uniform_block_.find(type_id) !=
struct_for_uniform_block_.cend();
}
// Save the ID of a struct of a uniform block.
void RegisterStructForUniformBlock(uint32_t type_id) {
struct_for_uniform_block_.insert(type_id);
}
// Is the ID the type of a pointer to a storage buffer: BufferBlock-decorated
// struct in uniform storage class, or Block-decorated struct in StorageBuffer
// storage class? The result is only valid after internal method
// CheckDecorationsOfBuffers has been called.
bool IsPointerToStorageBuffer(uint32_t type_id) const {
return pointer_to_storage_buffer_.find(type_id) !=
pointer_to_storage_buffer_.cend();
}
// Save the ID of a pointer to a storage buffer.
void RegisterPointerToStorageBuffer(uint32_t type_id) {
pointer_to_storage_buffer_.insert(type_id);
}
// Is the ID the type of a struct for storage buffer?
// The result is only valid after internal method CheckDecorationsOfBuffers
// has been called.
bool IsStructForStorageBuffer(uint32_t type_id) const {
return struct_for_storage_buffer_.find(type_id) !=
struct_for_storage_buffer_.cend();
}
// Save the ID of a struct of a storage buffer.
void RegisterStructForStorageBuffer(uint32_t type_id) {
struct_for_storage_buffer_.insert(type_id);
}
// Is the ID the type of a pointer to a storage image? That is, the pointee
// type is an image type which is known to not use a sampler.
bool IsPointerToStorageImage(uint32_t type_id) const {
return pointer_to_storage_image_.find(type_id) !=
pointer_to_storage_image_.cend();
}
// Save the ID of a pointer to a storage image.
void RegisterPointerToStorageImage(uint32_t type_id) {
pointer_to_storage_image_.insert(type_id);
}
// Tries to evaluate a 32-bit signed or unsigned scalar integer constant.
// Returns tuple <is_int32, is_const_int32, value>.
// OpSpecConstant* return |is_const_int32| as false since their values cannot
@ -701,6 +758,23 @@ class ValidationState_t {
std::unordered_map<uint32_t, std::vector<uint32_t>> function_to_entry_points_;
const std::vector<uint32_t> empty_ids_;
// The IDs of types of pointers to Block-decorated structs in Uniform storage
// class. This is populated at the start of ValidateDecorations.
std::unordered_set<uint32_t> pointer_to_uniform_block_;
// The IDs of struct types for uniform blocks.
// This is populated at the start of ValidateDecorations.
std::unordered_set<uint32_t> struct_for_uniform_block_;
// The IDs of types of pointers to BufferBlock-decorated structs in Uniform
// storage class, or Block-decorated structs in StorageBuffer storage class.
// This is populated at the start of ValidateDecorations.
std::unordered_set<uint32_t> pointer_to_storage_buffer_;
// The IDs of struct types for storage buffers.
// This is populated at the start of ValidateDecorations.
std::unordered_set<uint32_t> struct_for_storage_buffer_;
// The IDs of types of pointers to storage images. This is populated in the
// TypePass.
std::unordered_set<uint32_t> pointer_to_storage_image_;
/// Maps ids to friendly names.
std::unique_ptr<spvtools::FriendlyNameMapper> friendly_mapper_;
spvtools::NameMapper name_mapper_;

View File

@ -1129,9 +1129,14 @@ std::make_pair(std::string(kOpenCLMemoryModel) +
"%intt = OpTypeInt 32 0\n" + std::string(kVoidFVoid),
AllCapabilities()),
std::make_pair(std::string(kOpenCLMemoryModel) +
// NonWritable must target something valid, such as a storage image.
"OpEntryPoint Kernel %func \"compute\" \n"
"OpDecorate %intt NonWritable\n"
"%intt = OpTypeInt 32 0\n" + std::string(kVoidFVoid),
"OpDecorate %var NonWritable "
"%float = OpTypeFloat 32 "
"%imstor = OpTypeImage %float 2D 0 0 0 2 Unknown "
"%ptr = OpTypePointer UniformConstant %imstor "
"%var = OpVariable %ptr UniformConstant "
+ std::string(kVoidFVoid),
AllCapabilities()),
std::make_pair(std::string(kOpenCLMemoryModel) +
"OpEntryPoint Kernel %func \"compute\" \n"

View File

@ -115,7 +115,7 @@ TEST_F(ValidateDecorations, ValidateGroupDecorateRegistration) {
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %1 DescriptorSet 0
OpDecorate %1 NonWritable
OpDecorate %1 RelaxedPrecision
OpDecorate %1 Restrict
%1 = OpDecorationGroup
OpGroupDecorate %1 %2 %3
@ -136,9 +136,10 @@ TEST_F(ValidateDecorations, ValidateGroupDecorateRegistration) {
EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState());
// Decoration group has 3 decorations.
auto expected_decorations = std::vector<Decoration>{
Decoration(SpvDecorationDescriptorSet, {0}),
Decoration(SpvDecorationNonWritable), Decoration(SpvDecorationRestrict)};
auto expected_decorations =
std::vector<Decoration>{Decoration(SpvDecorationDescriptorSet, {0}),
Decoration(SpvDecorationRelaxedPrecision),
Decoration(SpvDecorationRestrict)};
// Decoration group is applied to id 1, 2, 3, and 4. Note that id 1 (which is
// the decoration group id) also has all the decorations.
@ -5491,6 +5492,255 @@ OpFunctionEnd
"improperly straddling vector at offset 28"));
}
// NonWritable
// Returns a SPIR-V shader module with variables in various storage classes,
// parameterizable by which ID should be decorated as NonWritable.
std::string ShaderWithNonWritableTarget(const std::string& target,
bool member_decorate = false) {
const std::string decoration_inst =
std::string(member_decorate ? "OpMemberDecorate " : "OpDecorate ") +
target + (member_decorate ? " 0" : "");
return std::string(R"(
OpCapability Shader
OpExtension "SPV_KHR_storage_buffer_storage_class"
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main"
OpName %label "label"
OpName %param_f "param_f"
OpName %param_p "param_p"
OpName %_ptr_imstor "_ptr_imstor"
OpName %_ptr_imsam "_ptr_imsam"
OpName %var_wg "var_wg"
OpName %var_imsam "var_imsam"
OpName %var_priv "var_priv"
OpName %var_func "var_func"
OpName %struct_bad "struct_bad"
OpDecorate %struct_b Block
OpDecorate %struct_bb BufferBlock
OpDecorate %struct_b_rtarr Block
OpMemberDecorate %struct_b 0 Offset 0
OpMemberDecorate %struct_bb 0 Offset 0
OpMemberDecorate %struct_b_rtarr 0 Offset 0
OpDecorate %rtarr ArrayStride 4
)") + decoration_inst +
R"( NonWritable
%void = OpTypeVoid
%void_fn = OpTypeFunction %void
%float = OpTypeFloat 32
%float_0 = OpConstant %float 0
%struct_b = OpTypeStruct %float
%struct_bb = OpTypeStruct %float
%rtarr = OpTypeRuntimeArray %float
%struct_b_rtarr = OpTypeStruct %rtarr
%struct_bad = OpTypeStruct %float
; storage image
%imstor = OpTypeImage %float 2D 0 0 0 2 R32f
; sampled image
%imsam = OpTypeImage %float 2D 0 0 0 1 R32f
%_ptr_Uniform_stb = OpTypePointer Uniform %struct_b
%_ptr_Uniform_stbb = OpTypePointer Uniform %struct_bb
%_ptr_StorageBuffer_stb = OpTypePointer StorageBuffer %struct_b
%_ptr_StorageBuffer_stb_rtarr = OpTypePointer StorageBuffer %struct_b_rtarr
%_ptr_Workgroup = OpTypePointer Workgroup %float
%_ptr_Private = OpTypePointer Private %float
%_ptr_Function = OpTypePointer Function %float
%_ptr_imstor = OpTypePointer UniformConstant %imstor
%_ptr_imsam = OpTypePointer UniformConstant %imsam
%extra_fn = OpTypeFunction %void %float %_ptr_Private %_ptr_imstor
%var_ubo = OpVariable %_ptr_Uniform_stb Uniform
%var_ssbo_u = OpVariable %_ptr_Uniform_stbb Uniform
%var_ssbo_sb = OpVariable %_ptr_StorageBuffer_stb StorageBuffer
%var_ssbo_sb_rtarr = OpVariable %_ptr_StorageBuffer_stb_rtarr StorageBuffer
%var_wg = OpVariable %_ptr_Workgroup Workgroup
%var_priv = OpVariable %_ptr_Private Private
%var_imstor = OpVariable %_ptr_imstor UniformConstant
%var_imsam = OpVariable %_ptr_imsam UniformConstant
%helper = OpFunction %void None %extra_fn
%param_f = OpFunctionParameter %float
%param_p = OpFunctionParameter %_ptr_Private
%param_pimstor = OpFunctionParameter %_ptr_imstor
%helper_label = OpLabel
%helper_func_var = OpVariable %_ptr_Function Function
OpReturn
OpFunctionEnd
%main = OpFunction %void None %void_fn
%label = OpLabel
%var_func = OpVariable %_ptr_Function Function
OpReturn
OpFunctionEnd
)";
}
TEST_F(ValidateDecorations, NonWritableLabelTargetBad) {
std::string spirv = ShaderWithNonWritableTarget("%label");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration must be a "
"memory object declaration (a variable or a function "
"parameter)\n %label = OpLabel"));
}
TEST_F(ValidateDecorations, NonWritableTypeTargetBad) {
std::string spirv = ShaderWithNonWritableTarget("%void");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration must be a "
"memory object declaration (a variable or a function "
"parameter)\n %void = OpTypeVoid"));
}
TEST_F(ValidateDecorations, NonWritableValueTargetBad) {
std::string spirv = ShaderWithNonWritableTarget("%float_0");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration must be a "
"memory object declaration (a variable or a function "
"parameter)\n %float_0 = OpConstant %float 0"));
}
TEST_F(ValidateDecorations, NonWritableValueParamBad) {
std::string spirv = ShaderWithNonWritableTarget("%param_f");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %param_f = OpFunctionParameter %float"));
}
TEST_F(ValidateDecorations, NonWritablePointerParamButWrongTypeBad) {
std::string spirv = ShaderWithNonWritableTarget("%param_p");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %param_p = OpFunctionParameter %_ptr_Private_float"));
}
TEST_F(ValidateDecorations, NonWritablePointerParamStorageImageGood) {
std::string spirv = ShaderWithNonWritableTarget("%param_pimstor");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableVarStorageImageGood) {
std::string spirv = ShaderWithNonWritableTarget("%var_imstor");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableVarSampledImageBad) {
std::string spirv = ShaderWithNonWritableTarget("%var_imsam");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %var_imsam"));
}
TEST_F(ValidateDecorations, NonWritableVarUboGood) {
std::string spirv = ShaderWithNonWritableTarget("%var_ubo");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableVarSsboInUniformGood) {
std::string spirv = ShaderWithNonWritableTarget("%var_ssbo_u");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableVarSsboInStorageBufferGood) {
std::string spirv = ShaderWithNonWritableTarget("%var_ssbo_sb");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableMemberOfSsboInStorageBufferGood) {
std::string spirv = ShaderWithNonWritableTarget("%struct_b_rtarr", true);
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), Eq(""));
}
TEST_F(ValidateDecorations, NonWritableMemberOfNonBlockStructBad) {
std::string spirv = ShaderWithNonWritableTarget("%struct_bad", true);
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("Target of NonWritable member decoration is invalid: must be "
"the struct type of a uniform block or storage buffer"));
}
TEST_F(ValidateDecorations, NonWritableVarWorkgroupBad) {
std::string spirv = ShaderWithNonWritableTarget("%var_wg");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %var_wg"));
}
TEST_F(ValidateDecorations, NonWritableVarPrivateBad) {
std::string spirv = ShaderWithNonWritableTarget("%var_priv");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %var_priv"));
}
TEST_F(ValidateDecorations, NonWritableVarFunctionBad) {
std::string spirv = ShaderWithNonWritableTarget("%var_func");
CompileSuccessfully(spirv);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Target of NonWritable decoration is invalid: must "
"point to a storage image, uniform block, or storage "
"buffer\n %var_func"));
}
} // namespace
} // namespace val
} // namespace spvtools