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: b/153380916
Change-Id: I8e0cf7ef855e1250ce6dce9ebf7a47e29da7814d
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/43549
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index b61737d..04ca7c6 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -1208,6 +1208,9 @@
EmitResult EmitGroupNonUniform(InsnIterator insn, EmitState *state) const;
EmitResult EmitArrayLength(InsnIterator insn, EmitState *state) const;
+ // Emits code to sample an image, regardless of whether any SIMD lanes are active.
+ void EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageInstruction instruction, InsnIterator insn, EmitState *state) const;
+
void GetImageDimensions(EmitState const *state, Type const &resultTy, Object::ID imageId, Object::ID lodId, Intermediate &dst) const;
SIMD::Pointer GetTexelAddress(EmitState const *state, SIMD::Pointer base, Operand const &coordinate, Type const &imageType, Pointer<Byte> descriptor, int texelSize, Object::ID sampleId, bool useStencilAspect) const;
uint32_t GetConstScalarInt(Object::ID id) const;
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index 6da2de9..273502a 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -107,11 +107,28 @@
SpirvShader::EmitResult SpirvShader::EmitImageSample(ImageInstruction instruction, InsnIterator insn, EmitState *state) const
{
+ auto &resultType = getType(insn.resultTypeId());
+ auto &result = state->createIntermediate(insn.resultId(), resultType.componentCount);
+ Array<SIMD::Float> out(4);
+
+ // TODO(b/153380916): When we're in a code path that is always executed,
+ // i.e. post-dominators of the entry block, we don't have to dynamically
+ // check whether any lanes are active, and can elide the jump.
+ If(AnyTrue(state->activeLaneMask()))
+ {
+ EmitImageSampleUnconditional(out, instruction, insn, state);
+ }
+
+ for(auto i = 0u; i < resultType.componentCount; i++) { result.move(i, out[i]); }
+
+ return EmitResult::Continue;
+}
+
+void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageInstruction instruction, InsnIterator insn, EmitState *state) const
+{
Object::ID sampledImageId = insn.word(3); // For OpImageFetch this is just an Image, not a SampledImage.
Object::ID coordinateId = insn.word(4);
- auto &resultType = getType(insn.resultTypeId());
- auto &result = state->createIntermediate(insn.resultId(), resultType.componentCount);
auto imageDescriptor = state->getPointer(sampledImageId).base; // vk::SampledImageDescriptor*
// If using a separate sampler, look through the OpSampledImage instruction to find the sampler descriptor
@@ -297,12 +314,7 @@
cache.sampler = sampler;
}
- Array<SIMD::Float> out(4);
Call<ImageSampler>(cache.function, texture, &in[0], &out[0], state->routine->constants);
-
- for(auto i = 0u; i < resultType.componentCount; i++) { result.move(i, out[i]); }
-
- return EmitResult::Continue;
}
SpirvShader::EmitResult SpirvShader::EmitImageQuerySizeLod(InsnIterator insn, EmitState *state) const
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index f1a44ef..590fc11 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -35,11 +35,10 @@
ImageInstruction instruction(inst);
const auto samplerId = sampler ? sampler->id : 0;
ASSERT(imageDescriptor->imageViewId != 0 && (samplerId != 0 || instruction.samplerMethod == Fetch));
+ ASSERT(imageDescriptor->device);
vk::Device::SamplingRoutineCache::Key key = { inst, imageDescriptor->imageViewId, samplerId };
- ASSERT(imageDescriptor->device);
-
vk::Device::SamplingRoutineCache *cache = imageDescriptor->device->getSamplingRoutineCache();
auto createSamplingRoutine = [&](const vk::Device::SamplingRoutineCache::Key &key) {
