Status Update
Comments
bc...@google.com <bc...@google.com> #2
The current implementation of SPIR-V loops always iterates one too many times
I don't think this is true. It depends on the CFG produced.
If last branch out of the loop is at the end of the loop body, you will not see the loop execute any more than is necessary.
It is true that there are cases where the branch-out happens at the top of the loop, and the main body is executed with no active lanes. I have seen both with dEQP tests, and I am not sure what is more common with glslang / dxc.
Actually jumping on an OpBranchConditional when all lanes are inactive would prevent this.
What you are discussing here must be classified as an optimization. It's an early out, nothing more. Any invalid accesses are bugs, pure and simple.
ni...@google.com <ni...@google.com>
ap...@google.com <ap...@google.com> #3
Branch: master
commit 9b83ddb1c1599d331d9fb3004e54a074d663bc8f
Author: Nicolas Capens <capn@google.com>
Date: Mon Apr 06 15:54:03 2020
Skip image sampling if no SIMD lanes are active
Currently all conditional branches in SPIR-V shaders are 'flattened',
resulting in the execution of all instructions in both code paths, even
in the case where none of the SIMD lanes are active. If a logically
never taken code path contains an image sampling instruction which
accesses a sampler or image array out of bounds, this could lead to
reading invalid descriptor data, or a page fault.
This is fixed by checking if any SIMD lanes are active, and if not,
jumping over the actual sampling operations which access the
descriptor data.
Bug:
Change-Id: I8e0cf7ef855e1250ce6dce9ebf7a47e29da7814d
Reviewed-on:
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
M src/Pipeline/SpirvShader.hpp
M src/Pipeline/SpirvShaderImage.cpp
M src/Pipeline/SpirvShaderSampling.cpp
ni...@google.com <ni...@google.com> #4
Filed
Description
The current implementation of SPIR-V loops always iterates one too many times. That's because at the end of the last logical iteration, there are still active SIMD lanes. It jumps back to the loop header which performs the loop test, but when it returns false for all lanes this merely affects the active lane mask. Just like for any other OpBranchConditional case, the branch is 'flattened' and in case of a loop the whole body is evaluated once more.
This is an issue for sampling instructions, which will access the sampler and image view descriptors regardless of whether any lanes are active. Specifically, when looping over an array of samplers and images we access one element beyond the intended range. This affects the implementation of the shaderSampledImageArrayDynamicIndexing feature ( b/146166966 ).
This issue is very similar to b/137890408 , where we may logically jump over an out-of-bounds access but still end up executing it.
Actually jumping on an OpBranchConditional when all lanes are inactive would prevent this. We may still want to flatten very small branches, but when image sampling instructions are involved it is definitely faster to jump.