Commit e260190e by Alexis Hetu Committed by Alexis Hétu

Obtain all sampler parameters through SamplingRoutineCache::Key

This change ensures that the descriptor state identifiers used to perform lookups in the sampling routine cache are all that is used to obtain the state itself which is used for specializing sampling routine generation. The createSamplingRoutine lambda function is made to capture only the 'device' variable, instead of allowing access to all local variables. The device is required to obtain the sampler state from the sampler identifier. Compared to the original CL: https://swiftshader-review.googlesource.com/c/SwiftShader/+/53068 This reorders the arguments to SpirvShader::getImageSampler(). Locally, at least, this makes the msan error go away. Bug: b/152227757 Bug: b/187467599 Change-Id: Id8488f3718a29bb53c9eb409e404a49ce2995987 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/54108 Presubmit-Ready: Alexis Hétu <sugoi@google.com> Tested-by: 's avatarAlexis Hétu <sugoi@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 8b09c107
...@@ -46,11 +46,13 @@ ...@@ -46,11 +46,13 @@
namespace vk { namespace vk {
class Device;
class PipelineLayout; class PipelineLayout;
class ImageView; class ImageView;
class Sampler; class Sampler;
class RenderPass; class RenderPass;
struct SampledImageDescriptor; struct SampledImageDescriptor;
struct SamplerState;
namespace dbg { namespace dbg {
class Context; class Context;
...@@ -1291,13 +1293,13 @@ private: ...@@ -1291,13 +1293,13 @@ private:
// Returns the pair <significand, exponent> // Returns the pair <significand, exponent>
std::pair<SIMD::Float, SIMD::Int> Frexp(RValue<SIMD::Float> val) const; std::pair<SIMD::Float, SIMD::Int> Frexp(RValue<SIMD::Float> val) const;
static ImageSampler *getImageSampler(uint32_t instruction, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler); static ImageSampler *getImageSampler(const vk::Device *device, uint32_t instruction, uint32_t samplerId, uint32_t imageViewId);
static std::shared_ptr<rr::Routine> emitSamplerRoutine(ImageInstruction instruction, const Sampler &samplerState); static std::shared_ptr<rr::Routine> emitSamplerRoutine(ImageInstruction instruction, const Sampler &samplerState);
// TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly. // TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly.
static sw::FilterType convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod); static sw::FilterType convertFilterMode(const vk::SamplerState *samplerState, VkImageViewType imageViewType, SamplerMethod samplerMethod);
static sw::MipmapType convertMipmapMode(const vk::Sampler *sampler); static sw::MipmapType convertMipmapMode(const vk::SamplerState *samplerState);
static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType); static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::SamplerState *samplerState, VkImageViewType imageViewType);
// Returns 0 when invalid. // Returns 0 when invalid.
static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model); static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model);
...@@ -1366,7 +1368,7 @@ public: ...@@ -1366,7 +1368,7 @@ public:
struct SamplerCache struct SamplerCache
{ {
Pointer<Byte> imageDescriptor = nullptr; Pointer<Byte> imageDescriptor = nullptr;
Pointer<Byte> sampler; Int samplerId;
Pointer<Byte> function; Pointer<Byte> function;
}; };
......
...@@ -157,15 +157,15 @@ void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageIns ...@@ -157,15 +157,15 @@ void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageIns
auto coordinate = Operand(this, state, coordinateId); auto coordinate = Operand(this, state, coordinateId);
Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler* 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> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
// Above we assumed that if the SampledImage operand is not the result of an OpSampledImage, // 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 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. // it's just an Image operand, so there's no sampler descriptor data.
if(getType(sampledImage).opcode() != spv::OpTypeSampledImage) if(getType(sampledImage).opcode() != spv::OpTypeSampledImage)
{ {
sampler = Pointer<Byte>(nullptr); samplerId = Int(0);
} }
uint32_t imageOperands = spv::ImageOperandsMaskNone; uint32_t imageOperands = spv::ImageOperandsMaskNone;
...@@ -325,13 +325,15 @@ void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageIns ...@@ -325,13 +325,15 @@ void SpirvShader::EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageIns
auto cacheIt = state->routine->samplerCache.find(insn.resultId()); auto cacheIt = state->routine->samplerCache.find(insn.resultId());
ASSERT(cacheIt != state->routine->samplerCache.end()); ASSERT(cacheIt != state->routine->samplerCache.end());
auto &cache = cacheIt->second; auto &cache = cacheIt->second;
auto cacheHit = cache.imageDescriptor == imageDescriptor && cache.sampler == sampler; auto cacheHit = cache.imageDescriptor == imageDescriptor && cache.samplerId == samplerId;
If(!cacheHit) If(!cacheHit)
{ {
cache.function = Call(getImageSampler, instruction.parameters, imageDescriptor, sampler); 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, device, instruction.parameters, samplerId, imageViewId);
cache.imageDescriptor = imageDescriptor; cache.imageDescriptor = imageDescriptor;
cache.sampler = sampler; cache.samplerId = samplerId;
} }
Call<ImageSampler>(cache.function, texture, &in[0], &out[0], state->routine->constants); Call<ImageSampler>(cache.function, texture, &in[0], &out[0], state->routine->constants);
......
...@@ -30,19 +30,18 @@ ...@@ -30,19 +30,18 @@
namespace sw { namespace sw {
SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler) SpirvShader::ImageSampler *SpirvShader::getImageSampler(const vk::Device *device, uint32_t inst, uint32_t samplerId, uint32_t imageViewId)
{ {
ImageInstruction instruction(inst); ImageInstruction instruction(inst);
const auto samplerId = sampler ? sampler->id : 0; ASSERT(imageViewId != 0 && (samplerId != 0 || instruction.samplerMethod == Fetch));
ASSERT(imageDescriptor->imageViewId != 0 && (samplerId != 0 || instruction.samplerMethod == Fetch)); ASSERT(device);
ASSERT(imageDescriptor->device);
vk::Device::SamplingRoutineCache::Key key = { inst, imageDescriptor->imageViewId, samplerId }; vk::Device::SamplingRoutineCache::Key key = { inst, samplerId, imageViewId };
vk::Device::SamplingRoutineCache *cache = imageDescriptor->device->getSamplingRoutineCache(); auto createSamplingRoutine = [&device](const vk::Device::SamplingRoutineCache::Key &key) {
ImageInstruction instruction(key.instruction);
auto createSamplingRoutine = [&](const vk::Device::SamplingRoutineCache::Key &key) { const vk::Identifier::State imageViewState = vk::Identifier(key.imageView).getState();
const vk::Identifier::State imageViewState = vk::Identifier(imageDescriptor->imageViewId).getState(); const vk::SamplerState *vkSamplerState = (key.sampler != 0) ? device->findSampler(key.sampler) : nullptr;
auto type = imageViewState.imageViewType; auto type = imageViewState.imageViewType;
auto samplerMethod = static_cast<SamplerMethod>(instruction.samplerMethod); auto samplerMethod = static_cast<SamplerMethod>(instruction.samplerMethod);
...@@ -51,34 +50,34 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl ...@@ -51,34 +50,34 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl
samplerState.textureType = type; samplerState.textureType = type;
samplerState.textureFormat = imageViewState.format; samplerState.textureFormat = imageViewState.format;
samplerState.addressingModeU = convertAddressingMode(0, sampler, type); samplerState.addressingModeU = convertAddressingMode(0, vkSamplerState, type);
samplerState.addressingModeV = convertAddressingMode(1, sampler, type); samplerState.addressingModeV = convertAddressingMode(1, vkSamplerState, type);
samplerState.addressingModeW = convertAddressingMode(2, sampler, type); samplerState.addressingModeW = convertAddressingMode(2, vkSamplerState, type);
samplerState.mipmapFilter = convertMipmapMode(sampler); samplerState.mipmapFilter = convertMipmapMode(vkSamplerState);
samplerState.swizzle = imageViewState.mapping; samplerState.swizzle = imageViewState.mapping;
samplerState.gatherComponent = instruction.gatherComponent; samplerState.gatherComponent = instruction.gatherComponent;
if(sampler) if(vkSamplerState)
{ {
samplerState.textureFilter = convertFilterMode(sampler, type, samplerMethod); samplerState.textureFilter = convertFilterMode(vkSamplerState, type, samplerMethod);
samplerState.border = sampler->borderColor; samplerState.border = vkSamplerState->borderColor;
samplerState.mipmapFilter = convertMipmapMode(sampler); samplerState.mipmapFilter = convertMipmapMode(vkSamplerState);
samplerState.highPrecisionFiltering = (sampler->filteringPrecision == VK_SAMPLER_FILTERING_PRECISION_MODE_HIGH_GOOGLE); samplerState.highPrecisionFiltering = (vkSamplerState->filteringPrecision == VK_SAMPLER_FILTERING_PRECISION_MODE_HIGH_GOOGLE);
samplerState.compareEnable = (sampler->compareEnable != VK_FALSE); samplerState.compareEnable = (vkSamplerState->compareEnable != VK_FALSE);
samplerState.compareOp = sampler->compareOp; samplerState.compareOp = vkSamplerState->compareOp;
samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates != VK_FALSE); samplerState.unnormalizedCoordinates = (vkSamplerState->unnormalizedCoordinates != VK_FALSE);
samplerState.ycbcrModel = sampler->ycbcrModel; samplerState.ycbcrModel = vkSamplerState->ycbcrModel;
samplerState.studioSwing = sampler->studioSwing; samplerState.studioSwing = vkSamplerState->studioSwing;
samplerState.swappedChroma = sampler->swappedChroma; samplerState.swappedChroma = vkSamplerState->swappedChroma;
samplerState.mipLodBias = sampler->mipLodBias; samplerState.mipLodBias = vkSamplerState->mipLodBias;
samplerState.maxAnisotropy = sampler->maxAnisotropy; samplerState.maxAnisotropy = vkSamplerState->maxAnisotropy;
samplerState.minLod = sampler->minLod; samplerState.minLod = vkSamplerState->minLod;
samplerState.maxLod = sampler->maxLod; samplerState.maxLod = vkSamplerState->maxLod;
} }
else else
{ {
...@@ -91,6 +90,7 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl ...@@ -91,6 +90,7 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl
return emitSamplerRoutine(instruction, samplerState); return emitSamplerRoutine(instruction, samplerState);
}; };
vk::Device::SamplingRoutineCache *cache = device->getSamplingRoutineCache();
auto routine = cache->getOrCreate(key, createSamplingRoutine); auto routine = cache->getOrCreate(key, createSamplingRoutine);
return (ImageSampler *)(routine->getEntry()); return (ImageSampler *)(routine->getEntry());
...@@ -200,7 +200,7 @@ std::shared_ptr<rr::Routine> SpirvShader::emitSamplerRoutine(ImageInstruction in ...@@ -200,7 +200,7 @@ std::shared_ptr<rr::Routine> SpirvShader::emitSamplerRoutine(ImageInstruction in
return function("sampler"); return function("sampler");
} }
sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod) sw::FilterType SpirvShader::convertFilterMode(const vk::SamplerState *samplerState, VkImageViewType imageViewType, SamplerMethod samplerMethod)
{ {
if(samplerMethod == Gather) if(samplerMethod == Gather)
{ {
...@@ -212,7 +212,7 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag ...@@ -212,7 +212,7 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag
return FILTER_POINT; return FILTER_POINT;
} }
if(sampler->anisotropyEnable != VK_FALSE) if(samplerState->anisotropyEnable != VK_FALSE)
{ {
if(imageViewType == VK_IMAGE_VIEW_TYPE_2D || imageViewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY) if(imageViewType == VK_IMAGE_VIEW_TYPE_2D || imageViewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY)
{ {
...@@ -223,25 +223,25 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag ...@@ -223,25 +223,25 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag
} }
} }
switch(sampler->magFilter) switch(samplerState->magFilter)
{ {
case VK_FILTER_NEAREST: case VK_FILTER_NEAREST:
switch(sampler->minFilter) switch(samplerState->minFilter)
{ {
case VK_FILTER_NEAREST: return FILTER_POINT; case VK_FILTER_NEAREST: return FILTER_POINT;
case VK_FILTER_LINEAR: return FILTER_MIN_LINEAR_MAG_POINT; case VK_FILTER_LINEAR: return FILTER_MIN_LINEAR_MAG_POINT;
default: default:
UNSUPPORTED("minFilter %d", sampler->minFilter); UNSUPPORTED("minFilter %d", samplerState->minFilter);
return FILTER_POINT; return FILTER_POINT;
} }
break; break;
case VK_FILTER_LINEAR: case VK_FILTER_LINEAR:
switch(sampler->minFilter) switch(samplerState->minFilter)
{ {
case VK_FILTER_NEAREST: return FILTER_MIN_POINT_MAG_LINEAR; case VK_FILTER_NEAREST: return FILTER_MIN_POINT_MAG_LINEAR;
case VK_FILTER_LINEAR: return FILTER_LINEAR; case VK_FILTER_LINEAR: return FILTER_LINEAR;
default: default:
UNSUPPORTED("minFilter %d", sampler->minFilter); UNSUPPORTED("minFilter %d", samplerState->minFilter);
return FILTER_POINT; return FILTER_POINT;
} }
break; break;
...@@ -249,34 +249,34 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag ...@@ -249,34 +249,34 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImag
break; break;
} }
UNSUPPORTED("magFilter %d", sampler->magFilter); UNSUPPORTED("magFilter %d", samplerState->magFilter);
return FILTER_POINT; return FILTER_POINT;
} }
sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler) sw::MipmapType SpirvShader::convertMipmapMode(const vk::SamplerState *samplerState)
{ {
if(!sampler) if(!samplerState)
{ {
return MIPMAP_POINT; // Samplerless operations (OpImageFetch) can take an integer Lod operand. return MIPMAP_POINT; // Samplerless operations (OpImageFetch) can take an integer Lod operand.
} }
if(sampler->ycbcrModel != VK_SAMPLER_YCBCR_MODEL_CONVERSION_RGB_IDENTITY) if(samplerState->ycbcrModel != VK_SAMPLER_YCBCR_MODEL_CONVERSION_RGB_IDENTITY)
{ {
// TODO(b/151263485): Check image view level count instead. // TODO(b/151263485): Check image view level count instead.
return MIPMAP_NONE; return MIPMAP_NONE;
} }
switch(sampler->mipmapMode) switch(samplerState->mipmapMode)
{ {
case VK_SAMPLER_MIPMAP_MODE_NEAREST: return MIPMAP_POINT; case VK_SAMPLER_MIPMAP_MODE_NEAREST: return MIPMAP_POINT;
case VK_SAMPLER_MIPMAP_MODE_LINEAR: return MIPMAP_LINEAR; case VK_SAMPLER_MIPMAP_MODE_LINEAR: return MIPMAP_LINEAR;
default: default:
UNSUPPORTED("mipmapMode %d", sampler->mipmapMode); UNSUPPORTED("mipmapMode %d", samplerState->mipmapMode);
return MIPMAP_POINT; return MIPMAP_POINT;
} }
} }
sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType) sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const vk::SamplerState *samplerState, VkImageViewType imageViewType)
{ {
switch(imageViewType) switch(imageViewType)
{ {
...@@ -321,7 +321,7 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const ...@@ -321,7 +321,7 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const
return ADDRESSING_WRAP; return ADDRESSING_WRAP;
} }
if(!sampler) if(!samplerState)
{ {
// OpImageFetch does not take a sampler descriptor, but still needs a valid // OpImageFetch does not take a sampler descriptor, but still needs a valid
// addressing mode that prevents out-of-bounds accesses: // addressing mode that prevents out-of-bounds accesses:
...@@ -339,9 +339,9 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const ...@@ -339,9 +339,9 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const
VkSamplerAddressMode addressMode = VK_SAMPLER_ADDRESS_MODE_REPEAT; VkSamplerAddressMode addressMode = VK_SAMPLER_ADDRESS_MODE_REPEAT;
switch(coordinateIndex) switch(coordinateIndex)
{ {
case 0: addressMode = sampler->addressModeU; break; case 0: addressMode = samplerState->addressModeU; break;
case 1: addressMode = sampler->addressModeV; break; case 1: addressMode = samplerState->addressModeV; break;
case 2: addressMode = sampler->addressModeW; break; case 2: addressMode = samplerState->addressModeW; break;
default: UNSUPPORTED("coordinateIndex: %d", coordinateIndex); default: UNSUPPORTED("coordinateIndex: %d", coordinateIndex);
} }
......
...@@ -105,6 +105,16 @@ void Device::SamplerIndexer::remove(const SamplerState &samplerState) ...@@ -105,6 +105,16 @@ void Device::SamplerIndexer::remove(const SamplerState &samplerState)
} }
} }
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) Device::Device(const VkDeviceCreateInfo *pCreateInfo, void *mem, PhysicalDevice *physicalDevice, const VkPhysicalDeviceFeatures *enabledFeatures, const std::shared_ptr<marl::Scheduler> &scheduler)
: physicalDevice(physicalDevice) : physicalDevice(physicalDevice)
, queues(reinterpret_cast<Queue *>(mem)) , queues(reinterpret_cast<Queue *>(mem))
...@@ -395,6 +405,11 @@ void Device::removeSampler(const SamplerState &samplerState) ...@@ -395,6 +405,11 @@ void Device::removeSampler(const SamplerState &samplerState)
samplerIndexer->remove(samplerState); samplerIndexer->remove(samplerState);
} }
const SamplerState *Device::findSampler(uint32_t samplerId) const
{
return samplerIndexer->find(samplerId);
}
VkResult Device::setDebugUtilsObjectName(const VkDebugUtilsObjectNameInfoEXT *pNameInfo) VkResult Device::setDebugUtilsObjectName(const VkDebugUtilsObjectNameInfoEXT *pNameInfo)
{ {
// Optionally maps user-friendly name to an object // Optionally maps user-friendly name to an object
......
...@@ -141,6 +141,7 @@ public: ...@@ -141,6 +141,7 @@ public:
uint32_t index(const SamplerState &samplerState); uint32_t index(const SamplerState &samplerState);
void remove(const SamplerState &samplerState); void remove(const SamplerState &samplerState);
const SamplerState *find(uint32_t id);
private: private:
struct Identifier struct Identifier
...@@ -157,6 +158,7 @@ public: ...@@ -157,6 +158,7 @@ public:
uint32_t indexSampler(const SamplerState &samplerState); uint32_t indexSampler(const SamplerState &samplerState);
void removeSampler(const SamplerState &samplerState); void removeSampler(const SamplerState &samplerState);
const SamplerState *findSampler(uint32_t samplerId) const;
std::shared_ptr<vk::dbg::Context> getDebuggerContext() const std::shared_ptr<vk::dbg::Context> getDebuggerContext() const
{ {
......
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