Skip to content

S
swiftshader
Commit cda86eff by Nicolas Capens Committed by Nicolas Capens
Options

Implement shaderStorageImageMultisample support

This fixes a bug in the descriptor data where the sample pitch was set to the layer pitch for array images (it should always be the slice pitch), and the descriptor's slice pitch (which stores the layer pitch in case of array images), was taking the sample count into account twice. Note that while the image robustness extensions still allow read operations to return undefined values "if only the sample index was invalid", so clamping that value would suffice (as currently done in our OpImageFetch implementation), write operations always have to be no-op. This change also 'nullifies' the OpImageRead instructions for invalid sample indices. Bug: b/163142358 Tests: dEQP-VK.* Change-Id: I9d816c249e14a17722d29f56015c8dc8cc7d51d4 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/47548 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent c4f3b713
Showing with 56 additions and 24 deletions
src/Pipeline/SpirvShader.cpp
......@@ -361,15 +361,17 @@ SpirvShader::SpirvShader(
{
case spv::CapabilityMatrix: capabilities.Matrix = true; break;
case spv::CapabilityShader: capabilities.Shader = true; break;
case spv::CapabilityStorageImageMultisample: capabilities.StorageImageMultisample = true; break;
case spv::CapabilityClipDistance: capabilities.ClipDistance = true; break;
case spv::CapabilityCullDistance: capabilities.CullDistance = true; break;
case spv::CapabilityImageCubeArray: capabilities.ImageCubeArray = true; break;
case spv::CapabilityInputAttachment: capabilities.InputAttachment = true; break;
case spv::CapabilitySampled1D: capabilities.Sampled1D = true; break;
case spv::CapabilityImage1D: capabilities.Image1D = true; break;
case spv::CapabilityImageCubeArray: capabilities.ImageCubeArray = true; break;
case spv::CapabilitySampledBuffer: capabilities.SampledBuffer = true; break;
case spv::CapabilitySampledCubeArray: capabilities.SampledCubeArray = true; break;
case spv::CapabilityImageBuffer: capabilities.ImageBuffer = true; break;
case spv::CapabilityImageMSArray: capabilities.ImageMSArray = true; break;
case spv::CapabilityStorageImageExtendedFormats: capabilities.StorageImageExtendedFormats = true; break;
case spv::CapabilityImageQuery: capabilities.ImageQuery = true; break;
case spv::CapabilityDerivativeControl: capabilities.DerivativeControl = true; break;
......
src/Pipeline/SpirvShader.hpp
......@@ -569,15 +569,17 @@ public:
{
bool Matrix : 1;
bool Shader : 1;
bool StorageImageMultisample : 1;
bool ClipDistance : 1;
bool CullDistance : 1;
bool ImageCubeArray : 1;
bool InputAttachment : 1;
bool Sampled1D : 1;
bool Image1D : 1;
bool ImageCubeArray : 1;
bool SampledBuffer : 1;
bool SampledCubeArray : 1;
bool ImageBuffer : 1;
bool ImageMSArray : 1;
bool StorageImageExtendedFormats : 1;
bool ImageQuery : 1;
bool DerivativeControl : 1;
......
src/Pipeline/SpirvShaderImage.cpp
......@@ -115,7 +115,7 @@ SpirvShader::EmitResult SpirvShader::EmitImageSampleExplicitLod(Variant variant,
return EmitImageSample({ variant, Grad }, insn, state);
}
else
UNSUPPORTED("Image Operands %x", imageOperands);
UNSUPPORTED("Image operands 0x%08X", imageOperands);
return EmitResult::Continue;
}
......@@ -234,7 +234,7 @@ void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageIns
if(imageOperands != 0)
{
UNSUPPORTED("Image operand %x", imageOperands);
UNSUPPORTED("Image operands 0x%08X", imageOperands);
}
}
......@@ -558,10 +558,12 @@ SIMD::Pointer SpirvShader::GetTexelAddress(EmitState const *state, Pointer<Byte>
ptrOffset += SIMD::Int(routine->viewID) * slicePitch;
}
SIMD::Int n = 0;
if(sampleId.value())
{
Operand sample(this, state, sampleId);
ptrOffset += sample.Int(0) * samplePitch;
n = sample.Int(0);
ptrOffset += n * samplePitch;
}
// If the out-of-bounds behavior is set to nullify, then each coordinate must be tested individually.
......@@ -581,7 +583,13 @@ SIMD::Pointer SpirvShader::GetTexelAddress(EmitState const *state, Pointer<Byte>
{
auto depth = *Pointer<UInt>(descriptor + OFFSET(vk::StorageImageDescriptor, extent.depth));
auto arrayLayers = *Pointer<UInt>(descriptor + OFFSET(vk::StorageImageDescriptor, arrayLayers));
oobMask |= As<SIMD::Int>(CmpNLT(As<SIMD::UInt>(w), SIMD::UInt(depth * arrayLayers)));
oobMask |= As<SIMD::Int>(CmpNLT(As<SIMD::UInt>(w), SIMD::UInt(depth * arrayLayers))); // TODO: Precompute extent. 3D image can't have layers.
}
if(sampleId.value())
{
SIMD::UInt sampleCount = *Pointer<UInt>(descriptor + OFFSET(vk::StorageImageDescriptor, sampleCount));
oobMask |= As<SIMD::Int>(CmpNLT(As<SIMD::UInt>(n), sampleCount));
}
constexpr int32_t OOB_OFFSET = 0x7FFFFFFF - 16; // SIMD pointer offsets are signed 32-bit, so this is the largest offset (for 16-byte texels).
......@@ -605,7 +613,7 @@ SpirvShader::EmitResult SpirvShader::EmitImageRead(InsnIterator insn, EmitState
if(insn.wordCount() > 5)
{
int operand = 6;
auto imageOperands = insn.word(5);
uint32_t imageOperands = insn.word(5);
if(imageOperands & spv::ImageOperandsSampleMask)
{
sampleId = insn.word(operand++);
......@@ -613,7 +621,10 @@ SpirvShader::EmitResult SpirvShader::EmitImageRead(InsnIterator insn, EmitState
}
// Should be no remaining image operands.
ASSERT(!imageOperands);
if(imageOperands != 0)
{
UNSUPPORTED("Image operands 0x%08X", imageOperands);
}
}
ASSERT(imageType.definition.opcode() == spv::OpTypeImage);
......@@ -990,8 +1001,24 @@ SpirvShader::EmitResult SpirvShader::EmitImageWrite(InsnIterator insn, EmitState
ASSERT(imageType.definition.opcode() == spv::OpTypeImage);
// TODO(b/131171141): Not handling any image operands yet.
ASSERT(insn.wordCount() == 4);
Object::ID sampleId = 0;
if(insn.wordCount() > 4)
{
int operand = 5;
uint32_t imageOperands = insn.word(4);
if(imageOperands & spv::ImageOperandsSampleMask)
{
sampleId = insn.word(operand++);
imageOperands &= ~spv::ImageOperandsSampleMask;
}
// Should be no remaining image operands.
if(imageOperands != 0)
{
UNSUPPORTED("Image operands 0x%08X", (int)imageOperands);
}
}
auto coordinate = Operand(this, state, insn.word(2));
auto texel = Operand(this, state, insn.word(3));
......@@ -1176,7 +1203,7 @@ SpirvShader::EmitResult SpirvShader::EmitImageWrite(InsnIterator insn, EmitState
// - https://www.khronos.org/registry/vulkan/specs/1.2/html/chap16.html#textures-output-coordinate-validation
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, 0, false, robustness);
auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, sampleId, false, robustness);
// Scatter packed texel data.
// TODO(b/160531165): Provide scatter abstractions for various element sizes.
......@@ -1235,6 +1262,7 @@ SpirvShader::EmitResult SpirvShader::EmitImageTexelPointer(InsnIterator insn, Em
ASSERT(getType(resultType.element).opcode() == spv::OpTypeInt);
auto coordinate = Operand(this, state, insn.word(4));
Object::ID sampleId = insn.word(5);
Pointer<Byte> binding = state->getPointer(imageId).base;
Pointer<Byte> imageBase = *Pointer<Pointer<Byte>>(binding + OFFSET(vk::StorageImageDescriptor, ptr));
......@@ -1244,7 +1272,7 @@ SpirvShader::EmitResult SpirvShader::EmitImageTexelPointer(InsnIterator insn, Em
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
auto ptr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, sizeof(uint32_t), 0, false, robustness);
auto ptr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, sizeof(uint32_t), sampleId, false, robustness);
state->createPointer(resultId, ptr);
......
src/Vulkan/VkDescriptorSetLayout.cpp
......@@ -435,10 +435,10 @@ void DescriptorSetLayout::WriteDescriptorSet(Device *device, DescriptorSet *dstS
descriptor[i].ptr = imageView->getOffsetPointer({ 0, 0, 0 }, VK_IMAGE_ASPECT_COLOR_BIT, 0, 0);
descriptor[i].extent = imageView->getMipLevelExtent(0);
descriptor[i].rowPitchBytes = imageView->rowPitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0);
descriptor[i].samplePitchBytes = imageView->getSubresourceRange().layerCount > 1
? imageView->layerPitchBytes(VK_IMAGE_ASPECT_COLOR_BIT)
: imageView->slicePitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0);
descriptor[i].slicePitchBytes = descriptor[i].samplePitchBytes * imageView->getSampleCount();
descriptor[i].samplePitchBytes = imageView->slicePitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0);
descriptor[i].slicePitchBytes = imageView->getSubresourceRange().layerCount > 1
? imageView->layerPitchBytes(VK_IMAGE_ASPECT_COLOR_BIT)
: imageView->slicePitchBytes(VK_IMAGE_ASPECT_COLOR_BIT, 0);
descriptor[i].arrayLayers = imageView->getSubresourceRange().layerCount;
descriptor[i].sampleCount = imageView->getSampleCount();
descriptor[i].sizeInBytes = static_cast<int>(imageView->getSizeInBytes());
......@@ -448,10 +448,10 @@ void DescriptorSetLayout::WriteDescriptorSet(Device *device, DescriptorSet *dstS
{
descriptor[i].stencilPtr = imageView->getOffsetPointer({ 0, 0, 0 }, VK_IMAGE_ASPECT_STENCIL_BIT, 0, 0);
descriptor[i].stencilRowPitchBytes = imageView->rowPitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT, 0);
descriptor[i].stencilSamplePitchBytes = (imageView->getSubresourceRange().layerCount > 1)
? imageView->layerPitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT)
: imageView->slicePitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT, 0);
descriptor[i].stencilSlicePitchBytes = descriptor[i].stencilSamplePitchBytes * imageView->getSampleCount();
descriptor[i].stencilSamplePitchBytes = imageView->slicePitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT, 0);
descriptor[i].stencilSlicePitchBytes = (imageView->getSubresourceRange().layerCount > 1)
? imageView->layerPitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT)
: imageView->slicePitchBytes(VK_IMAGE_ASPECT_STENCIL_BIT, 0);
}
}
}
......
src/Vulkan/VkDescriptorSetLayout.hpp
......@@ -59,7 +59,7 @@ struct alignas(16) StorageImageDescriptor
void *ptr;
VkExtent3D extent;
int rowPitchBytes;
int slicePitchBytes;
int slicePitchBytes; // Layer pitch in case of array image
int samplePitchBytes;
int arrayLayers;
int sampleCount;
......@@ -67,7 +67,7 @@ struct alignas(16) StorageImageDescriptor
void *stencilPtr;
int stencilRowPitchBytes;
int stencilSlicePitchBytes;
int stencilSlicePitchBytes; // Layer pitch in case of array image
int stencilSamplePitchBytes;
ImageView *memoryOwner; // Pointer to the view which owns the memory used by the descriptor set
......
src/Vulkan/VkPhysicalDevice.cpp
......@@ -98,7 +98,7 @@ const VkPhysicalDeviceFeatures &PhysicalDevice::getFeatures() const
VK_FALSE, // shaderTessellationAndGeometryPointSize
VK_FALSE, // shaderImageGatherExtended
VK_TRUE, // shaderStorageImageExtendedFormats
VK_FALSE, // shaderStorageImageMultisample
VK_TRUE, // shaderStorageImageMultisample
VK_FALSE, // shaderStorageImageReadWithoutFormat
VK_FALSE, // shaderStorageImageWriteWithoutFormat
VK_TRUE, // shaderUniformBufferArrayDynamicIndexing
......@@ -295,7 +295,7 @@ const VkPhysicalDeviceLimits &PhysicalDevice::getLimits() const
sampleCounts, // sampledImageIntegerSampleCounts
sampleCounts, // sampledImageDepthSampleCounts
sampleCounts, // sampledImageStencilSampleCounts
VK_SAMPLE_COUNT_1_BIT, // storageImageSampleCounts (unsupported)
sampleCounts, // storageImageSampleCounts
1, // maxSampleMaskWords
VK_FALSE, // timestampComputeAndGraphics
60, // timestampPeriod
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment