KhronosGroup / KhronosGroup/SPIRV-Tools
[spirv-opt] OpLoads are not de-duplicated for NonWritable buffers
- Dominant language
- C++
- Stars
- 1.4k
- Forks
- 709
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 28
Description
It's pretty common to loads parameters that are stored in buffers, with a simplified test case being:
```hlsl
StructuredBuffer in_data;
RWStructuredBuffer data_out;
[numthreads(1, 1, 1)]
void test_0(uint index : SV_DispatchThreadID)
{
data_out[0] = in_data[0];
data_out[1] = in_data[0];
}
```
With `-O -Os` we get:
```llvm
; SPIR-V
; Version: 1.0
; Generator: Google spiregg; 0
; Bound: 24
; Schema: 0
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %1 "test_0"
OpExecutionMode %1 LocalSize 1 1 1
OpDecorate %3 DescriptorSet 0
OpDecorate %3 Binding 1
OpDecorate %5 DescriptorSet 0
OpDecorate %5 Binding 2
OpDecorate %_runtimearr_float ArrayStride 4
OpMemberDecorate %_struct_2 0 Offset 0
OpMemberDecorate %_struct_2 0 NonWritable
OpDecorate %_struct_2 BufferBlock
OpMemberDecorate %_struct_4 0 Offset 0
OpDecorate %_struct_4 BufferBlock
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%uint = OpTypeInt 32 0
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
%float = OpTypeFloat 32
%_runtimearr_float = OpTypeRuntimeArray %float
%_struct_2 = OpTypeStruct %_runtimearr_float
%_ptr_Uniform__struct_2 = OpTypePointer Uniform %_struct_2
%_struct_4 = OpTypeStruct %_runtimearr_float
%_ptr_Uniform__struct_4 = OpTypePointer Uniform %_struct_4
%void = OpTypeVoid
%16 = OpTypeFunction %void
%_ptr_Uniform_float = OpTypePointer Uniform %float
%3 = OpVariable %_ptr_Uniform__struct_2 Uniform
%5 = OpVariable %_ptr_Uniform__struct_4 Uniform
%1 = OpFunction %void None %16
%18 = OpLabel
%19 = OpAccessChain %_ptr_Uniform_float %3 %int_0 %uint_0
%20 = OpLoad %float %19
%21 = OpAccessChain %_ptr_Uniform_float %5 %int_0 %uint_0
OpStore %21 %20
%22 = OpLoad %float %19
%23 = OpAccessChain %_ptr_Uniform_float %5 %int_0 %uint_1
OpStore %23 %22
OpReturn
OpFunctionEnd
```
With the following duplicate loads:
```
%20 = OpLoad %float %19
%22 = OpLoad %float %19
```
Despite:
```llvm
OpMemberDecorate %_struct_2 0 Offset 0
OpMemberDecorate %_struct_2 0 NonWritable
%3 = OpVariable %_ptr_Uniform__struct_2 Uniform
%19 = OpAccessChain %_ptr_Uniform_float %3 %int_0 %uint_0
```
I believe, this is because the use of `bool Instruction::IsReadOnlyPointerShaders()` is a bit off, while it does check for `NonWritable`:
```cpp
bool is_nonwritable = false;
context()->get_decoration_mgr()->ForEachDecoration(
result_id(), uint32_t(spv::Decoration::NonWritable),
[&is_nonwritable](const Instruction&) { is_nonwritable = true; });
return is_nonwritable;
```
It's testing against `%3`, which is the `OpVariable`, which would only have decorators for the binding set and descriptor.
Contributor guide
Research direction
Start at Instruction::IsReadOnlyPointerShaders() and trace how it checks NonWritable decorations on the pointer used by the OpLoad. Reproduce the HLSL example with spirv-opt -O -Os, then verify that repeated loads from the NonWritable buffer are de-duplicated and the generated SPIR-V contains one OpLoad.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100