Revert "Obtain all sampler parameters through SamplingRoutineCache::Key"
This reverts commit f6afa76378ad96e7af1733bdd40fd1339395b5c1.
Reason for revert: An MSAN error in SpirvShader::getImageSampler() is blocking the roll
Change-Id: I8a1076832fbc2037970f9f586767e7296bbc4608
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/54050
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index e52ddf3..69335b8 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -46,13 +46,11 @@
namespace vk {
-class Device;
class PipelineLayout;
class ImageView;
class Sampler;
class RenderPass;
struct SampledImageDescriptor;
-struct SamplerState;
namespace dbg {
class Context;
@@ -1293,13 +1291,13 @@
// Returns the pair <significand, exponent>
std::pair<SIMD::Float, SIMD::Int> Frexp(RValue<SIMD::Float> val) const;
- static ImageSampler *getImageSampler(uint32_t instruction, uint32_t imageViewId, uint32_t samplerId, const vk::Device *device);
+ static ImageSampler *getImageSampler(uint32_t instruction, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler);
static std::shared_ptr<rr::Routine> emitSamplerRoutine(ImageInstruction instruction, const Sampler &samplerState);
// TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly.
- static sw::FilterType convertFilterMode(const vk::SamplerState *samplerState, VkImageViewType imageViewType, SamplerMethod samplerMethod);
- static sw::MipmapType convertMipmapMode(const vk::SamplerState *samplerState);
- static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::SamplerState *samplerState, VkImageViewType imageViewType);
+ static sw::FilterType convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod);
+ static sw::MipmapType convertMipmapMode(const vk::Sampler *sampler);
+ static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType);
// Returns 0 when invalid.
static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model);
@@ -1368,7 +1366,7 @@
struct SamplerCache
{
Pointer<Byte> imageDescriptor = nullptr;
- Int samplerId;
+ Pointer<Byte> sampler;
Pointer<Byte> function;
};
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index 3be1fa0..d0ea81a 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -157,15 +157,15 @@
auto coordinate = Operand(this, state, coordinateId);
- rr::Int samplerId = *Pointer<rr::Int>(samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler) + OFFSET(vk::Sampler, id)); // vk::Sampler::id
- Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
+ Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler*
+ Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
// Above we assumed that if the SampledImage operand is not the result of an OpSampledImage,
// it must be a combined image sampler loaded straight from the descriptor set. For OpImageFetch
// it's just an Image operand, so there's no sampler descriptor data.
if(getType(sampledImage).opcode() != spv::OpTypeSampledImage)
{
- samplerId = Int(0);
+ sampler = Pointer<Byte>(nullptr);
}
uint32_t imageOperands = spv::ImageOperandsMaskNone;
@@ -325,15 +325,13 @@
auto cacheIt = state->routine->samplerCache.find(insn.resultId());
ASSERT(cacheIt != state->routine->samplerCache.end());
auto &cache = cacheIt->second;
- auto cacheHit = cache.imageDescriptor == imageDescriptor && cache.samplerId == samplerId;
+ auto cacheHit = cache.imageDescriptor == imageDescriptor && cache.sampler == sampler;
If(!cacheHit)
{
- rr::Int imageViewId = *Pointer<rr::Int>(imageDescriptor + OFFSET(vk::SampledImageDescriptor, imageViewId));
- Pointer<Byte> device = *Pointer<Pointer<Byte>>(imageDescriptor + OFFSET(vk::SampledImageDescriptor, device));
- cache.function = Call(getImageSampler, instruction.parameters, imageViewId, samplerId, device);
+ cache.function = Call(getImageSampler, instruction.parameters, imageDescriptor, sampler);
cache.imageDescriptor = imageDescriptor;
- cache.samplerId = samplerId;
+ cache.sampler = sampler;
}
Call<ImageSampler>(cache.function, texture, &in[0], &out[0], state->routine->constants);
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index c36e43c..ecd2f74 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -30,20 +30,19 @@
namespace sw {
-SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, uint32_t imageViewId, uint32_t samplerId, const vk::Device *device)
+SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler)
{
ImageInstruction instruction(inst);
- ASSERT(imageViewId != 0 && (samplerId != 0 || instruction.samplerMethod == Fetch));
- ASSERT(device);
+ 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, samplerId, imageViewId };
+ vk::Device::SamplingRoutineCache::Key key = { inst, imageDescriptor->imageViewId, samplerId };
- vk::Device::SamplingRoutineCache *cache = device->getSamplingRoutineCache();
+ vk::Device::SamplingRoutineCache *cache = imageDescriptor->device->getSamplingRoutineCache();
- auto createSamplingRoutine = [&device](const vk::Device::SamplingRoutineCache::Key &key) {
- ImageInstruction instruction(key.instruction);
- const vk::Identifier::State imageViewState = vk::Identifier(key.imageView).getState();
- const vk::SamplerState *vkSamplerState = (key.sampler != 0) ? device->findSampler(key.sampler) : nullptr;
+ auto createSamplingRoutine = [&](const vk::Device::SamplingRoutineCache::Key &key) {
+ const vk::Identifier::State imageViewState = vk::Identifier(imageDescriptor->imageViewId).getState();
auto type = imageViewState.imageViewType;
auto samplerMethod = static_cast<SamplerMethod>(instruction.samplerMethod);
@@ -52,34 +51,34 @@
samplerState.textureType = type;
samplerState.textureFormat = imageViewState.format;
- samplerState.addressingModeU = convertAddressingMode(0, vkSamplerState, type);
- samplerState.addressingModeV = convertAddressingMode(1, vkSamplerState, type);
- samplerState.addressingModeW = convertAddressingMode(2, vkSamplerState, type);
+ samplerState.addressingModeU = convertAddressingMode(0, sampler, type);
+ samplerState.addressingModeV = convertAddressingMode(1, sampler, type);
+ samplerState.addressingModeW = convertAddressingMode(2, sampler, type);
- samplerState.mipmapFilter = convertMipmapMode(vkSamplerState);
+ samplerState.mipmapFilter = convertMipmapMode(sampler);
samplerState.swizzle = imageViewState.mapping;
samplerState.gatherComponent = instruction.gatherComponent;
- if(vkSamplerState)
+ if(sampler)
{
- samplerState.textureFilter = convertFilterMode(vkSamplerState, type, samplerMethod);
- samplerState.border = vkSamplerState->borderColor;
+ samplerState.textureFilter = convertFilterMode(sampler, type, samplerMethod);
+ samplerState.border = sampler->borderColor;
- samplerState.mipmapFilter = convertMipmapMode(vkSamplerState);
- samplerState.highPrecisionFiltering = (vkSamplerState->filteringPrecision == VK_SAMPLER_FILTERING_PRECISION_MODE_HIGH_GOOGLE);
+ samplerState.mipmapFilter = convertMipmapMode(sampler);
+ samplerState.highPrecisionFiltering = (sampler->filteringPrecision == VK_SAMPLER_FILTERING_PRECISION_MODE_HIGH_GOOGLE);
- samplerState.compareEnable = (vkSamplerState->compareEnable != VK_FALSE);
- samplerState.compareOp = vkSamplerState->compareOp;
- samplerState.unnormalizedCoordinates = (vkSamplerState->unnormalizedCoordinates != VK_FALSE);
+ samplerState.compareEnable = (sampler->compareEnable != VK_FALSE);
+ samplerState.compareOp = sampler->compareOp;
+ samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates != VK_FALSE);
- samplerState.ycbcrModel = vkSamplerState->ycbcrModel;
- samplerState.studioSwing = vkSamplerState->studioSwing;
- samplerState.swappedChroma = vkSamplerState->swappedChroma;
+ samplerState.ycbcrModel = sampler->ycbcrModel;
+ samplerState.studioSwing = sampler->studioSwing;
+ samplerState.swappedChroma = sampler->swappedChroma;
- samplerState.mipLodBias = vkSamplerState->mipLodBias;
- samplerState.maxAnisotropy = vkSamplerState->maxAnisotropy;
- samplerState.minLod = vkSamplerState->minLod;
- samplerState.maxLod = vkSamplerState->maxLod;
+ samplerState.mipLodBias = sampler->mipLodBias;
+ samplerState.maxAnisotropy = sampler->maxAnisotropy;
+ samplerState.minLod = sampler->minLod;
+ samplerState.maxLod = sampler->maxLod;
}
else
{
@@ -201,7 +200,7 @@
return function("sampler");
}
-sw::FilterType SpirvShader::convertFilterMode(const vk::SamplerState *samplerState, VkImageViewType imageViewType, SamplerMethod samplerMethod)
+sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod)
{
if(samplerMethod == Gather)
{
@@ -213,7 +212,7 @@
return FILTER_POINT;
}
- if(samplerState->anisotropyEnable != VK_FALSE)
+ if(sampler->anisotropyEnable != VK_FALSE)
{
if(imageViewType == VK_IMAGE_VIEW_TYPE_2D || imageViewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY)
{
@@ -224,25 +223,25 @@
}
}
- switch(samplerState->magFilter)
+ switch(sampler->magFilter)
{
case VK_FILTER_NEAREST:
- switch(samplerState->minFilter)
+ switch(sampler->minFilter)
{
case VK_FILTER_NEAREST: return FILTER_POINT;
case VK_FILTER_LINEAR: return FILTER_MIN_LINEAR_MAG_POINT;
default:
- UNSUPPORTED("minFilter %d", samplerState->minFilter);
+ UNSUPPORTED("minFilter %d", sampler->minFilter);
return FILTER_POINT;
}
break;
case VK_FILTER_LINEAR:
- switch(samplerState->minFilter)
+ switch(sampler->minFilter)
{
case VK_FILTER_NEAREST: return FILTER_MIN_POINT_MAG_LINEAR;
case VK_FILTER_LINEAR: return FILTER_LINEAR;
default:
- UNSUPPORTED("minFilter %d", samplerState->minFilter);
+ UNSUPPORTED("minFilter %d", sampler->minFilter);
return FILTER_POINT;
}
break;
@@ -250,34 +249,34 @@
break;
}
- UNSUPPORTED("magFilter %d", samplerState->magFilter);
+ UNSUPPORTED("magFilter %d", sampler->magFilter);
return FILTER_POINT;
}
-sw::MipmapType SpirvShader::convertMipmapMode(const vk::SamplerState *samplerState)
+sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler)
{
- if(!samplerState)
+ if(!sampler)
{
return MIPMAP_POINT; // Samplerless operations (OpImageFetch) can take an integer Lod operand.
}
- if(samplerState->ycbcrModel != VK_SAMPLER_YCBCR_MODEL_CONVERSION_RGB_IDENTITY)
+ if(sampler->ycbcrModel != VK_SAMPLER_YCBCR_MODEL_CONVERSION_RGB_IDENTITY)
{
// TODO(b/151263485): Check image view level count instead.
return MIPMAP_NONE;
}
- switch(samplerState->mipmapMode)
+ switch(sampler->mipmapMode)
{
case VK_SAMPLER_MIPMAP_MODE_NEAREST: return MIPMAP_POINT;
case VK_SAMPLER_MIPMAP_MODE_LINEAR: return MIPMAP_LINEAR;
default:
- UNSUPPORTED("mipmapMode %d", samplerState->mipmapMode);
+ UNSUPPORTED("mipmapMode %d", sampler->mipmapMode);
return MIPMAP_POINT;
}
}
-sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const vk::SamplerState *samplerState, VkImageViewType imageViewType)
+sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType)
{
switch(imageViewType)
{
@@ -322,7 +321,7 @@
return ADDRESSING_WRAP;
}
- if(!samplerState)
+ if(!sampler)
{
// OpImageFetch does not take a sampler descriptor, but still needs a valid
// addressing mode that prevents out-of-bounds accesses:
@@ -340,9 +339,9 @@
VkSamplerAddressMode addressMode = VK_SAMPLER_ADDRESS_MODE_REPEAT;
switch(coordinateIndex)
{
- case 0: addressMode = samplerState->addressModeU; break;
- case 1: addressMode = samplerState->addressModeV; break;
- case 2: addressMode = samplerState->addressModeW; break;
+ case 0: addressMode = sampler->addressModeU; break;
+ case 1: addressMode = sampler->addressModeV; break;
+ case 2: addressMode = sampler->addressModeW; break;
default: UNSUPPORTED("coordinateIndex: %d", coordinateIndex);
}
diff --git a/src/Vulkan/VkDevice.cpp b/src/Vulkan/VkDevice.cpp
index 4a742cf..e420921 100644
--- a/src/Vulkan/VkDevice.cpp
+++ b/src/Vulkan/VkDevice.cpp
@@ -105,16 +105,6 @@
}
}
-const SamplerState *Device::SamplerIndexer::find(uint32_t id)
-{
- marl::lock lock(mutex);
-
- auto it = std::find_if(std::begin(map), std::end(map),
- [&id](auto &&p) { return p.second.id == id; });
-
- return (it != std::end(map)) ? &(it->first) : nullptr;
-}
-
Device::Device(const VkDeviceCreateInfo *pCreateInfo, void *mem, PhysicalDevice *physicalDevice, const VkPhysicalDeviceFeatures *enabledFeatures, const std::shared_ptr<marl::Scheduler> &scheduler)
: physicalDevice(physicalDevice)
, queues(reinterpret_cast<Queue *>(mem))
@@ -405,11 +395,6 @@
samplerIndexer->remove(samplerState);
}
-const SamplerState *Device::findSampler(uint32_t samplerId) const
-{
- return samplerIndexer->find(samplerId);
-}
-
VkResult Device::setDebugUtilsObjectName(const VkDebugUtilsObjectNameInfoEXT *pNameInfo)
{
// Optionally maps user-friendly name to an object
diff --git a/src/Vulkan/VkDevice.hpp b/src/Vulkan/VkDevice.hpp
index 70330ef..fdcce63 100644
--- a/src/Vulkan/VkDevice.hpp
+++ b/src/Vulkan/VkDevice.hpp
@@ -141,7 +141,6 @@
uint32_t index(const SamplerState &samplerState);
void remove(const SamplerState &samplerState);
- const SamplerState *find(uint32_t id);
private:
struct Identifier
@@ -158,7 +157,6 @@
uint32_t indexSampler(const SamplerState &samplerState);
void removeSampler(const SamplerState &samplerState);
- const SamplerState *findSampler(uint32_t samplerId) const;
std::shared_ptr<vk::dbg::Context> getDebuggerContext() const
{
