Marshal image instruction parameters to the sampling trampoline
The SPIR-V 1.3 spec states that the Coordinate operand of
OpImageSample* instructions "may be a vector larger than needed, but all
unused components will appear after all used components."
The function for generating the sampling routine previously determined
the number of coordinates from the image view descriptor, which may not
match what's passed in at the SPIR-V sample instruction call site.
This change passes a 32-bit run-time constant to the trampoline, which
can contain the number of coordinates as well as the instruction type.
Hence we can eliminate the need for multiple static functions to encode
the latter.
Bug: b/129523279
Change-Id: I625b7396be3da770024a858d11e49b63ac457bed
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/30151
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
diff --git a/src/Pipeline/SamplerCore.hpp b/src/Pipeline/SamplerCore.hpp
index e6fe4ed..030cb97 100644
--- a/src/Pipeline/SamplerCore.hpp
+++ b/src/Pipeline/SamplerCore.hpp
@@ -27,14 +27,15 @@
{
using namespace rr;
- enum SamplerMethod
+ enum SamplerMethod : uint32_t
{
Implicit, // Compute gradients (pixel shader only).
Bias, // Compute gradients and add provided bias.
Lod, // Use provided LOD.
Grad, // Use provided gradients.
Fetch, // Use provided integer coordinates.
- Base // Sample base level.
+ Base, // Sample base level.
+ SAMPLER_METHOD_LAST = Base,
};
enum SamplerOption
@@ -43,6 +44,7 @@
Offset // Offset sample location by provided integer coordinates.
};
+ // TODO(b/129523279): Eliminate and use SpirvShader::ImageInstruction instead.
struct SamplerFunction
{
SamplerFunction(SamplerMethod method, SamplerOption option = None) : method(method), option(option) {}
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index c7e489f..57bbd47 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -4456,15 +4456,19 @@
SpirvShader::EmitResult SpirvShader::EmitImageSampleImplicitLod(InsnIterator insn, EmitState *state) const
{
- return EmitImageSample(getImageSamplerImplicitLod, insn, state);
+ ImageInstruction imageInstruction(Implicit);
+
+ return EmitImageSample(imageInstruction, insn, state);
}
SpirvShader::EmitResult SpirvShader::EmitImageSampleExplicitLod(InsnIterator insn, EmitState *state) const
{
- return EmitImageSample(getImageSamplerExplicitLod, insn, state);
+ ImageInstruction imageInstruction(Lod);
+
+ return EmitImageSample(imageInstruction, insn, state);
}
- SpirvShader::EmitResult SpirvShader::EmitImageSample(GetImageSampler getImageSampler, InsnIterator insn, EmitState *state) const
+ SpirvShader::EmitResult SpirvShader::EmitImageSample(ImageInstruction instruction, InsnIterator insn, EmitState *state) const
{
Type::ID resultTypeId = insn.word(1);
Object::ID resultId = insn.word(2);
@@ -4477,13 +4481,12 @@
auto coordinate = GenericValue(this, state->routine, coordinateId);
auto &coordinateType = getType(coordinate.type);
- Pointer<Byte> constants; // FIXME(b/129523279)
-
auto descriptor = sampledImage.base; // vk::SampledImageDescriptor*
auto sampler = *Pointer<Pointer<Byte>>(descriptor + OFFSET(vk::SampledImageDescriptor, sampler)); // vk::Sampler*
auto imageView = *Pointer<Pointer<Byte>>(descriptor + OFFSET(vk::SampledImageDescriptor, imageView)); // vk::ImageView*
- auto samplerFunc = Call(getImageSampler, imageView, sampler);
+ instruction.coordinates = coordinateType.sizeInComponents;
+ auto samplerFunc = Call(getImageSampler, instruction.parameters, imageView, sampler);
uint32_t imageOperands = spv::ImageOperandsMaskNone;
bool bias = false;
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index d40bd27..beb21b9 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -470,6 +470,36 @@
inline operator Object::ID() const { return Object::ID(value()); }
};
+ // Compact representation of image instruction parameters that is passed to the
+ // trampoline function for retrieving/generating the corresponding sampling routine.
+ struct ImageInstruction
+ {
+ ImageInstruction(SamplerMethod samplerMethod) : samplerMethod(samplerMethod)
+ {
+ }
+
+ // Unmarshal from raw 32-bit data
+ ImageInstruction(uint32_t parameters) : parameters(parameters) {}
+
+ SamplerMethod getSamplerMethod() const
+ {
+ return static_cast<SamplerMethod>(samplerMethod);
+ }
+
+ union
+ {
+ struct
+ {
+ uint32_t samplerMethod : BITS(SAMPLER_METHOD_LAST);
+ uint32_t coordinates : 3;
+ };
+
+ uint32_t parameters = 0;
+ };
+ };
+
+ static_assert(sizeof(ImageInstruction) == 4, "ImageInstruction must be 32-bit");
+
int getSerialID() const
{
return serialID;
@@ -840,7 +870,7 @@
EmitResult EmitPhi(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageSampleImplicitLod(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageSampleExplicitLod(InsnIterator insn, EmitState *state) const;
- EmitResult EmitImageSample(GetImageSampler getImageSampler, InsnIterator insn, EmitState *state) const;
+ EmitResult EmitImageSample(ImageInstruction instruction, InsnIterator insn, EmitState *state) const;
EmitResult EmitImageQuerySize(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageRead(InsnIterator insn, EmitState *state) const;
EmitResult EmitImageWrite(InsnIterator insn, EmitState *state) const;
@@ -867,11 +897,9 @@
// Returns the pair <significand, exponent>
std::pair<SIMD::Float, SIMD::Int> Frexp(RValue<SIMD::Float> val) const;
- static ImageSampler *getImageSamplerImplicitLod(const vk::ImageView *imageView, const vk::Sampler *sampler);
- static ImageSampler *getImageSamplerExplicitLod(const vk::ImageView *imageView, const vk::Sampler *sampler);
- static ImageSampler *getImageSampler(SamplerMethod samplerMethod, const vk::ImageView *imageView, const vk::Sampler *sampler);
+ static ImageSampler *getImageSampler(uint32_t instruction, const vk::ImageView *imageView, const vk::Sampler *sampler);
static void emitSamplerFunction(
- SamplerMethod samplerMethod,
+ ImageInstruction instruction,
const vk::ImageView *imageView, const vk::Sampler *sampler,
Pointer<Byte> image, Pointer<SIMD::Float> in, Pointer<Byte> out, Pointer<Byte> constants);
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index 6abb34c..baf5c32 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-
#include "SpirvShader.hpp"
#include "SamplerCore.hpp" // TODO: Figure out what's needed.
@@ -29,7 +28,6 @@
#include <spirv/unified1/spirv.hpp>
#include <spirv/unified1/GLSL.std.450.h>
-
#include <mutex>
#ifdef Bool
@@ -39,17 +37,7 @@
namespace sw {
-SpirvShader::ImageSampler *SpirvShader::getImageSamplerImplicitLod(const vk::ImageView *imageView, const vk::Sampler *sampler)
-{
- return getImageSampler(Implicit, imageView, sampler);
-}
-
-SpirvShader::ImageSampler *SpirvShader::getImageSamplerExplicitLod(const vk::ImageView *imageView, const vk::Sampler *sampler)
-{
- return getImageSampler(Lod, imageView, sampler);
-}
-
-SpirvShader::ImageSampler *SpirvShader::getImageSampler(SamplerMethod samplerMethod, const vk::ImageView *imageView, const vk::Sampler *sampler)
+SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t instruction, const vk::ImageView *imageView, const vk::Sampler *sampler)
{
// TODO(b/129523279): Move somewhere sensible.
static std::unordered_map<uint64_t, ImageSampler*> cache;
@@ -68,14 +56,16 @@
Pointer<SIMD::Float> in = function.Arg<1>();
Pointer<SIMD::Float> out = function.Arg<2>();
Pointer<Byte> constants = function.Arg<3>();
- emitSamplerFunction(samplerMethod, imageView, sampler, image, in, out, constants);
+
+ emitSamplerFunction({instruction}, imageView, sampler, image, in, out, constants);
+
auto fptr = reinterpret_cast<ImageSampler*>((void *)function("sampler")->getEntry());
cache.emplace(key, fptr);
return fptr;
}
void SpirvShader::emitSamplerFunction(
- SamplerMethod samplerMethod,
+ ImageInstruction instruction,
const vk::ImageView *imageView, const vk::Sampler *sampler,
Pointer<Byte> image, Pointer<SIMD::Float> in, Pointer<Byte> out, Pointer<Byte> constants)
{
@@ -114,40 +104,26 @@
Vector4f dsx; // TODO(b/129523279)
Vector4f dsy; // TODO(b/129523279)
Vector4f offset; // TODO(b/129523279)
- SamplerFunction samplerFunction = { samplerMethod, None }; // TODO(b/129523279)
+ SamplerFunction samplerFunction = { instruction.getSamplerMethod(), None }; // TODO(b/129523279)
- int coordinateCount = 0;
- switch(imageView->getType())
- {
- case VK_IMAGE_VIEW_TYPE_1D: coordinateCount = 1; break;
- case VK_IMAGE_VIEW_TYPE_2D: coordinateCount = 2; break;
-// case VK_IMAGE_VIEW_TYPE_3D: coordinateCount = 3; break;
- case VK_IMAGE_VIEW_TYPE_CUBE: coordinateCount = 3; break;
-// case VK_IMAGE_VIEW_TYPE_1D_ARRAY: coordinateCount = 2; break;
- case VK_IMAGE_VIEW_TYPE_2D_ARRAY: coordinateCount = 3; break;
-// case VK_IMAGE_VIEW_TYPE_CUBE_ARRAY: coordinateCount = 4; break;
- default:
- UNIMPLEMENTED("imageView type %d", imageView->getType());
- }
-
- for(int i = 0; i < coordinateCount; i++)
+ for(uint32_t i = 0; i < instruction.coordinates; i++)
{
uvw[i] = in[i];
}
// TODO(b/129523279): Currently 1D textures are treated as 2D by setting the second coordinate to 0.
// Implement optimized 1D sampling.
- If(imageView->getType() == VK_IMAGE_VIEW_TYPE_1D ||
+ if(imageView->getType() == VK_IMAGE_VIEW_TYPE_1D ||
imageView->getType() == VK_IMAGE_VIEW_TYPE_1D_ARRAY)
{
uvw[1] = SIMD::Float(0);
}
- if(samplerMethod == Lod)
+ if(instruction.samplerMethod == Lod)
{
// Lod is the second optional image operand, and is incompatible with the first one (Bias),
// so it always comes after the coordinates.
- bias = in[coordinateCount];
+ bias = in[instruction.coordinates];
}
Vector4f sample = s.sampleTexture(texture, uvw[0], uvw[1], uvw[2], q, bias, dsx, dsy, offset, samplerFunction);
diff --git a/src/Reactor/Reactor.hpp b/src/Reactor/Reactor.hpp
index 25682ab..7f458d4 100644
--- a/src/Reactor/Reactor.hpp
+++ b/src/Reactor/Reactor.hpp
@@ -3078,11 +3078,12 @@
template<typename T>
struct CToReactor;
- template<> struct CToReactor<void> { using type = Void; };
- template<> struct CToReactor<int> { using type = Int; };
- template<> struct CToReactor<float> { using type = Float; };
- template<> struct CToReactor<int*> { using type = Pointer<Int>; };
- template<> struct CToReactor<float*> { using type = Pointer<Float>; };
+ template<> struct CToReactor<void> { using type = Void; };
+ template<> struct CToReactor<int> { using type = Int; };
+ template<> struct CToReactor<unsigned int> { using type = UInt; };
+ template<> struct CToReactor<float> { using type = Float; };
+ template<> struct CToReactor<int*> { using type = Pointer<Int>; };
+ template<> struct CToReactor<float*> { using type = Pointer<Float>; };
// Pointers to non-reactor types are treated as uint8_t*.
template<typename T>
