KhronosGroup / KhronosGroup/SPIRV-Tools

spirv-val: Validator doesn't disallow bool in physical storage buffer storage class

Open
#3,335 0 comments 0 reactions 0 assignees View on GitHub
component:validation
Dominant language
C++
Stars
1.4k
Forks
709
Avg merge
1d 22h
Merged PRs (30d)
28

Description

`OpTypeBool` is only allowed as an abstract type in interfaces like input/output and private:

> OpTypeBool
>
> Declare the Boolean type. Values of this type can only be either true or false. There is no physical size or bit pattern defined for these values. If they are stored (in conjunction with OpVariable), they can only be used with logical addressing operations, not physical, and **only with non-externally visible shader Storage Classes: Workgroup, CrossWorkgroup, Private, Function, Input, and Output.**

But the validator doesn't complain when using it for physical storage buffer pointers. It does complain for uniform, so perhaps it's blacklisting some storage classes that it disallows rather than whitelisting those allowed?

Repro shader:

```
$ cat repro.spvasm
OpCapability Shader
OpCapability Int64
OpCapability PhysicalStorageBufferAddresses
OpExtension "SPV_KHR_physical_storage_buffer"
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 430
OpDecorate %_runtimearr_bool ArrayStride 4
OpMemberDecorate %foo 0 Offset 0
OpDecorate %foo BufferBlock
OpDecorate %addr_1 SpecId 0
OpDecorate %addr_2 SpecId 1
%void = OpTypeVoid
%3 = OpTypeFunction %void
%bool = OpTypeBool
%_runtimearr_bool = OpTypeRuntimeArray %bool
%foo = OpTypeStruct %_runtimearr_bool
%_ptr_PhysicalStorageBuffer_foo = OpTypePointer PhysicalStorageBuffer %foo
%uint = OpTypeInt 32 0
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%addr_1 = OpSpecConstant %uint 0
%addr_2 = OpSpecConstant %uint 0
%v2uint = OpTypeVector %uint 2
%addr = OpSpecConstantComposite %v2uint %addr_1 %addr_2
%bool_true = OpConstantTrue %bool
%_ptr_PhysicalStorageBuffer_bool = OpTypePointer PhysicalStorageBuffer %bool
%main = OpFunction %void None %3
%5 = OpLabel
%bar = OpBitcast %_ptr_PhysicalStorageBuffer_foo %addr
%15 = OpAccessChain %_ptr_PhysicalStorageBuffer_bool %bar %int_0 %int_0
OpStore %15 %bool_true Aligned 4
OpReturn
OpFunctionEnd
```

And it validates cleanly on spirv-val from the latest vulkan SDK:

```
$ spirv-as --target-env vulkan1.0 repro.frag -o repro.spv
$ spirv-val --target-env vulkan1.0 repro.spv
$ spirv-val --version
SPIRV-Tools v2020.2 v2020.2
Targets:
```

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the issue with repro.spvasm and spirv-val using the commands in the report, then trace validation for OpTypeBool used through PhysicalStorageBuffer pointers. Done means spirv-val rejects the provided shader while preserving the existing valid storage-class cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.