Parse OpImageRead and OpImageWrite into an ImageInstruction
This change unifies the parsing of instructions which access images or
texel buffers, into the ImageInstruction class. Specifically it removes
the custom parsing of OpImageRead and OpImageWrite instructions.
Bug: b/203730083
Change-Id: Ie5927710318ffb75a3a78ccfc1a94e1a5920632c
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/59368
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Device/Sampler.hpp b/src/Device/Sampler.hpp
index 06c59ef..ae02a8a 100644
--- a/src/Device/Sampler.hpp
+++ b/src/Device/Sampler.hpp
@@ -130,9 +130,8 @@
return false;
default:
UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return false;
}
-
- return false;
}
bool is2D() const
@@ -150,9 +149,8 @@
return false;
default:
UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return false;
}
-
- return false;
}
bool is3D() const
@@ -170,9 +168,8 @@
return false;
default:
UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return false;
}
-
- return false;
}
bool isCube() const
@@ -190,9 +187,8 @@
return false;
default:
UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return false;
}
-
- return false;
}
bool isArrayed() const
@@ -210,9 +206,30 @@
return false;
default:
UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return false;
}
+ }
- return false;
+ // Returns the number of coordinates required to sample the image,
+ // not including any array coordinate, which is indicated by isArrayed().
+ unsigned int dimensionality() const
+ {
+ switch(textureType)
+ {
+ case VK_IMAGE_VIEW_TYPE_1D:
+ case VK_IMAGE_VIEW_TYPE_1D_ARRAY:
+ return 1;
+ case VK_IMAGE_VIEW_TYPE_2D:
+ case VK_IMAGE_VIEW_TYPE_2D_ARRAY:
+ return 2;
+ case VK_IMAGE_VIEW_TYPE_3D:
+ case VK_IMAGE_VIEW_TYPE_CUBE:
+ case VK_IMAGE_VIEW_TYPE_CUBE_ARRAY:
+ return 3;
+ default:
+ UNSUPPORTED("VkImageViewType %d", (int)textureType);
+ return 0;
+ }
}
};
diff --git a/src/Pipeline/SamplerCore.hpp b/src/Pipeline/SamplerCore.hpp
index a59cce8..4cfd8fa 100644
--- a/src/Pipeline/SamplerCore.hpp
+++ b/src/Pipeline/SamplerCore.hpp
@@ -34,7 +34,9 @@
Base, // Sample base level.
Query, // Return implicit LOD.
Gather, // Return one channel of each texel in footprint.
- SAMPLER_METHOD_LAST = Gather,
+ Read, // Read a texel from an image without a sampler.
+ Write, // Write a texel to an image without a sampler.
+ SAMPLER_METHOD_LAST = Write,
};
// TODO(b/129523279): Eliminate and use SpirvShader::ImageInstruction instead.
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index f4d1d3f..2666ef5 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -1991,7 +1991,7 @@
case spv::OpImageDrefGather:
case spv::OpImageFetch:
case spv::OpImageQueryLod:
- return EmitImageSample(insn, state);
+ return EmitImageSample(ImageInstruction(insn, *this), state);
case spv::OpImageQuerySizeLod:
return EmitImageQuerySizeLod(insn, state);
@@ -2006,10 +2006,10 @@
return EmitImageQuerySamples(insn, state);
case spv::OpImageRead:
- return EmitImageRead(insn, state);
+ return EmitImageRead(ImageInstruction(insn, *this), state);
case spv::OpImageWrite:
- return EmitImageWrite(insn, state);
+ return EmitImageWrite(ImageInstruction(insn, *this), state);
case spv::OpImageTexelPointer:
return EmitImageTexelPointer(insn, state);
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 08c71a6..1d8e5f6 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -578,9 +578,11 @@
const uint32_t position;
+ Type::ID resultTypeId = 0;
Object::ID resultId = 0;
Object::ID sampledImageId = 0;
Object::ID coordinateId = 0;
+ Object::ID texelId = 0;
Object::ID drefId = 0;
Object::ID lodOrBiasId = 0;
Object::ID gradDxId = 0;
@@ -590,7 +592,8 @@
private:
static ImageInstructionState parseVariantAndMethod(InsnIterator insn);
- static uint32_t getImageOperands(InsnIterator insn);
+ static uint32_t getImageOperandsIndex(InsnIterator insn);
+ static uint32_t getImageOperandsMask(InsnIterator insn);
};
// This method is for retrieving an ID that uniquely identifies the
@@ -1284,13 +1287,13 @@
EmitResult EmitKill(InsnIterator insn, EmitState *state) const;
EmitResult EmitFunctionCall(InsnIterator insn, EmitState *state) const;
EmitResult EmitPhi(InsnIterator insn, EmitState *state) const;
- EmitResult EmitImageSample(InsnIterator insn, EmitState *state) const;
+ EmitResult EmitImageSample(const ImageInstruction &instruction, EmitState *state) const;
EmitResult EmitImageQuerySizeLod(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageQuerySize(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageQueryLevels(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageQuerySamples(InsnIterator insn, EmitState *state) const;
- EmitResult EmitImageRead(InsnIterator insn, EmitState *state) const;
- EmitResult EmitImageWrite(InsnIterator insn, EmitState *state) const;
+ EmitResult EmitImageRead(const ImageInstruction &instruction, EmitState *state) const;
+ EmitResult EmitImageWrite(const ImageInstruction &instruction, EmitState *state) const;
EmitResult EmitImageTexelPointer(InsnIterator insn, EmitState *state) const;
EmitResult EmitAtomicOp(InsnIterator insn, EmitState *state) const;
EmitResult EmitAtomicCompareExchange(InsnIterator insn, EmitState *state) const;
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index 1eddd25..c048426 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -91,9 +91,19 @@
: ImageInstructionState(parseVariantAndMethod(insn))
, position(insn.distanceFrom(spirv.begin()))
{
- resultId = insn.resultId(); // word(2)
- sampledImageId = insn.word(3); // For OpImageFetch this is just an Image, not a SampledImage.
- coordinateId = insn.word(4);
+ if(samplerMethod == Write)
+ {
+ sampledImageId = insn.word(1); // For OpImageWrite this is just an Image, not a SampledImage.
+ coordinateId = insn.word(2);
+ texelId = insn.word(3);
+ }
+ else
+ {
+ resultTypeId = insn.resultTypeId(); // word(1)
+ resultId = insn.resultId(); // word(2)
+ sampledImageId = insn.word(3); // For OpImageFetch/Read this is just an Image, not a SampledImage.
+ coordinateId = insn.word(4);
+ }
const Object &coordinateObject = spirv.getObject(coordinateId);
const Type &coordinateType = spirv.getType(coordinateObject);
@@ -109,31 +119,33 @@
gatherComponent = !isDref() ? spirv.getObject(insn.word(5)).constantValue[0] : 0;
}
- uint32_t imageOperands = getImageOperands(insn); // The mask which indicates which operands are provided.
- uint32_t operand = (isDref() || samplerMethod == Gather) ? 7 : 6; // The first actual operand <id> location.
+ uint32_t operandsIndex = getImageOperandsIndex(insn);
+ uint32_t imageOperands = (operandsIndex != 0) ? insn.word(operandsIndex) : 0; // The mask which indicates which operands are provided.
+
+ operandsIndex += 1; // Advance to the first actual operand <id> location.
if(imageOperands & spv::ImageOperandsBiasMask)
{
ASSERT(samplerMethod == Bias);
- lodOrBiasId = insn.word(operand);
- operand++;
+ lodOrBiasId = insn.word(operandsIndex);
+ operandsIndex += 1;
imageOperands &= ~spv::ImageOperandsBiasMask;
}
if(imageOperands & spv::ImageOperandsLodMask)
{
ASSERT(samplerMethod == Lod || samplerMethod == Fetch);
- lodOrBiasId = insn.word(operand);
- operand++;
+ lodOrBiasId = insn.word(operandsIndex);
+ operandsIndex += 1;
imageOperands &= ~spv::ImageOperandsLodMask;
}
if(imageOperands & spv::ImageOperandsGradMask)
{
ASSERT(samplerMethod == Grad);
- gradDxId = insn.word(operand + 0);
- gradDyId = insn.word(operand + 1);
- operand += 2;
+ gradDxId = insn.word(operandsIndex + 0);
+ gradDyId = insn.word(operandsIndex + 1);
+ operandsIndex += 2;
imageOperands &= ~spv::ImageOperandsGradMask;
grad = spirv.getObjectType(gradDxId).componentCount;
@@ -141,8 +153,8 @@
if(imageOperands & spv::ImageOperandsConstOffsetMask)
{
- offsetId = insn.word(operand);
- operand++;
+ offsetId = insn.word(operandsIndex);
+ operandsIndex += 1;
imageOperands &= ~spv::ImageOperandsConstOffsetMask;
offset = spirv.getObjectType(offsetId).componentCount;
@@ -150,13 +162,26 @@
if(imageOperands & spv::ImageOperandsSampleMask)
{
- ASSERT(samplerMethod == Fetch);
- sampleId = insn.word(operand);
+ ASSERT(samplerMethod == Fetch || samplerMethod == Read || samplerMethod == Write);
+ sampleId = insn.word(operandsIndex);
+ operandsIndex += 1;
imageOperands &= ~spv::ImageOperandsSampleMask;
sample = true;
}
+ // TODO(b/174475384)
+ if(imageOperands & spv::ImageOperandsZeroExtendMask)
+ {
+ ASSERT(samplerMethod == Read || samplerMethod == Write);
+ imageOperands &= ~spv::ImageOperandsZeroExtendMask;
+ }
+ else if(imageOperands & spv::ImageOperandsSignExtendMask)
+ {
+ ASSERT(samplerMethod == Read || samplerMethod == Write);
+ imageOperands &= ~spv::ImageOperandsSignExtendMask;
+ }
+
if(imageOperands != 0)
{
UNSUPPORTED("Image operands 0x%08X", imageOperands);
@@ -165,7 +190,7 @@
SpirvShader::ImageInstructionState SpirvShader::ImageInstruction::parseVariantAndMethod(InsnIterator insn)
{
- uint32_t imageOperands = getImageOperands(insn);
+ uint32_t imageOperands = getImageOperandsMask(insn);
bool bias = imageOperands & spv::ImageOperandsBiasMask;
bool grad = imageOperands & spv::ImageOperandsGradMask;
@@ -183,6 +208,8 @@
case spv::OpImageDrefGather: return { Dref, Gather };
case spv::OpImageFetch: return { None, Fetch };
case spv::OpImageQueryLod: return { None, Query };
+ case spv::OpImageRead: return { None, Read };
+ case spv::OpImageWrite: return { None, Write };
default:
ASSERT(false);
@@ -190,30 +217,35 @@
}
}
-uint32_t SpirvShader::ImageInstruction::getImageOperands(InsnIterator insn)
+// Returns the instruction word index at which the Image Operands mask is located, or 0 if not present.
+uint32_t SpirvShader::ImageInstruction::getImageOperandsIndex(InsnIterator insn)
{
switch(insn.opcode())
{
case spv::OpImageSampleImplicitLod:
case spv::OpImageSampleProjImplicitLod:
- return insn.wordCount() > 5 ? insn.word(5) : 0; // Optional
+ return insn.wordCount() > 5 ? 5 : 0; // Optional
case spv::OpImageSampleExplicitLod:
case spv::OpImageSampleProjExplicitLod:
- return insn.word(5); // "Either Lod or Grad image operands must be present."
+ return 5; // "Either Lod or Grad image operands must be present."
case spv::OpImageSampleDrefImplicitLod:
case spv::OpImageSampleProjDrefImplicitLod:
- return insn.wordCount() > 6 ? insn.word(6) : 0; // Optional
+ return insn.wordCount() > 6 ? 6 : 0; // Optional
case spv::OpImageSampleDrefExplicitLod:
case spv::OpImageSampleProjDrefExplicitLod:
- return insn.word(6); // "Either Lod or Grad image operands must be present."
+ return 6; // "Either Lod or Grad image operands must be present."
case spv::OpImageGather:
case spv::OpImageDrefGather:
- return insn.wordCount() > 6 ? insn.word(6) : 0; // Optional
+ return insn.wordCount() > 6 ? 6 : 0; // Optional
case spv::OpImageFetch:
- return insn.wordCount() > 5 ? insn.word(5) : 0; // Optional
+ return insn.wordCount() > 5 ? 5 : 0; // Optional
case spv::OpImageQueryLod:
ASSERT(insn.wordCount() == 5);
- return 0;
+ return 0; // No image operands.
+ case spv::OpImageRead:
+ return insn.wordCount() > 5 ? 5 : 0; // Optional
+ case spv::OpImageWrite:
+ return insn.wordCount() > 4 ? 4 : 0; // Optional
default:
ASSERT(false);
@@ -221,13 +253,17 @@
}
}
-SpirvShader::EmitResult SpirvShader::EmitImageSample(InsnIterator insn, EmitState *state) const
+uint32_t SpirvShader::ImageInstruction::getImageOperandsMask(InsnIterator insn)
{
- auto &resultType = getType(insn.resultTypeId());
- auto &result = state->createIntermediate(insn.resultId(), resultType.componentCount);
- Array<SIMD::Float> out(4);
+ uint32_t operandsIndex = getImageOperandsIndex(insn);
+ return (operandsIndex != 0) ? insn.word(operandsIndex) : 0;
+}
- ImageInstruction instruction(insn, *this);
+SpirvShader::EmitResult SpirvShader::EmitImageSample(const ImageInstruction &instruction, EmitState *state) const
+{
+ auto &resultType = getType(instruction.resultTypeId);
+ auto &result = state->createIntermediate(instruction.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
@@ -630,46 +666,17 @@
return SIMD::Pointer(imageBase, imageSizeInBytes, ptrOffset);
}
-SpirvShader::EmitResult SpirvShader::EmitImageRead(InsnIterator insn, EmitState *state) const
+SpirvShader::EmitResult SpirvShader::EmitImageRead(const ImageInstruction &instruction, EmitState *state) const
{
- auto &resultType = getType(Type::ID(insn.word(1)));
- auto imageId = Object::ID(insn.word(3));
- auto &image = getObject(imageId);
+ auto &resultType = getObjectType(instruction.resultId);
+ auto &image = getObject(instruction.sampledImageId);
auto &imageType = getType(image);
- Object::ID sampleId = 0;
-
- if(insn.wordCount() > 5)
- {
- int operand = 6;
- uint32_t imageOperands = insn.word(5);
- if(imageOperands & spv::ImageOperandsSampleMask)
- {
- sampleId = insn.word(operand++);
- imageOperands &= ~spv::ImageOperandsSampleMask;
- }
- // TODO(b/174475384)
- if(imageOperands & spv::ImageOperandsZeroExtendMask)
- {
- imageOperands &= ~spv::ImageOperandsZeroExtendMask;
- }
- else if(imageOperands & spv::ImageOperandsSignExtendMask)
- {
- imageOperands &= ~spv::ImageOperandsSignExtendMask;
- }
-
- // Should be no remaining image operands.
- if(imageOperands != 0)
- {
- UNSUPPORTED("Image operands 0x%08X", imageOperands);
- }
- }
-
ASSERT(imageType.definition.opcode() == spv::OpTypeImage);
auto dim = static_cast<spv::Dim>(imageType.definition.word(3));
- auto coordinate = Operand(this, state, insn.word(4));
- const DescriptorDecorations &d = descriptorDecorations.at(imageId);
+ auto coordinate = Operand(this, state, instruction.coordinateId);
+ const DescriptorDecorations &d = descriptorDecorations.at(instruction.sampledImageId);
// For subpass data, format in the instruction is spv::ImageFormatUnknown. Get it from
// the renderpass data instead. In all other cases, we can use the format in the instruction.
@@ -687,7 +694,7 @@
vkFormat = VK_FORMAT_S8_UINT;
}
- auto pointer = state->getPointer(imageId);
+ auto pointer = state->getPointer(instruction.sampledImageId);
Pointer<Byte> binding = pointer.base;
Pointer<Byte> imageBase = *Pointer<Pointer<Byte>>(binding + (useStencilAspect
? OFFSET(vk::StorageImageDescriptor, stencilPtr)
@@ -695,14 +702,14 @@
auto imageSizeInBytes = *Pointer<Int>(binding + OFFSET(vk::StorageImageDescriptor, sizeInBytes));
- auto &dst = state->createIntermediate(insn.resultId(), resultType.componentCount);
+ auto &dst = state->createIntermediate(instruction.resultId, resultType.componentCount);
// VK_EXT_image_robustness requires replacing out-of-bounds access with zero.
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelSize = vk::Format(vkFormat).bytes();
- auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, sampleId, useStencilAspect, robustness);
+ auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, instruction.sampleId, useStencilAspect, robustness);
// Gather packed texel data. Texels larger than 4 bytes occupy multiple SIMD::Int elements.
// TODO(b/160531165): Provide gather abstractions for various element sizes.
@@ -1071,48 +1078,19 @@
return EmitResult::Continue;
}
-SpirvShader::EmitResult SpirvShader::EmitImageWrite(InsnIterator insn, EmitState *state) const
+SpirvShader::EmitResult SpirvShader::EmitImageWrite(const ImageInstruction &instruction, EmitState *state) const
{
imageWriteEmitted = true;
- auto imageId = Object::ID(insn.word(1));
- auto &image = getObject(imageId);
+ auto &image = getObject(instruction.sampledImageId);
auto &imageType = getType(image);
ASSERT(imageType.definition.opcode() == spv::OpTypeImage);
- Object::ID sampleId = 0;
+ auto coordinate = Operand(this, state, instruction.coordinateId);
+ auto texel = Operand(this, state, instruction.texelId);
- if(insn.wordCount() > 4)
- {
- int operand = 5;
- uint32_t imageOperands = insn.word(4);
- if(imageOperands & spv::ImageOperandsSampleMask)
- {
- sampleId = insn.word(operand++);
- imageOperands &= ~spv::ImageOperandsSampleMask;
- }
- // TODO(b/174475384)
- if(imageOperands & spv::ImageOperandsZeroExtendMask)
- {
- imageOperands &= ~spv::ImageOperandsZeroExtendMask;
- }
- else if(imageOperands & spv::ImageOperandsSignExtendMask)
- {
- imageOperands &= ~spv::ImageOperandsSignExtendMask;
- }
-
- // 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));
-
- Pointer<Byte> binding = state->getPointer(imageId).base;
+ Pointer<Byte> binding = state->getPointer(instruction.sampledImageId).base;
Pointer<Byte> imageBase = *Pointer<Pointer<Byte>>(binding + OFFSET(vk::StorageImageDescriptor, ptr));
auto imageSizeInBytes = *Pointer<Int>(binding + OFFSET(vk::StorageImageDescriptor, sizeInBytes));
@@ -1292,7 +1270,7 @@
// - 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, sampleId, false, robustness);
+ auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, instruction.sampleId, false, robustness);
// Scatter packed texel data.
// TODO(b/160531165): Provide scatter abstractions for various element sizes.
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index 0bcc9e1..2d1a071 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -48,6 +48,7 @@
Sampler samplerState = {};
samplerState.textureType = type;
+ ASSERT(instruction.coordinates >= samplerState.dimensionality()); // "It may be a vector larger than needed, but all unused components appear after all used components."
samplerState.textureFormat = imageViewState.format;
samplerState.addressingModeU = convertAddressingMode(0, vkSamplerState, type);