Merge pull request #2243 from billhollings/fix-shdr-native-atomics

Fix shader conversion failure when using native texture atomics.
KhronosGroup · May 30, 2024 · 4ef99e5 · 4ef99e5
2 parents fc52da9 + f36e5a8
commit 4ef99e5
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 22 deletions.
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
@@ -18,6 +18,7 @@ MoltenVK 1.2.10
 
 Released TBD
 
+- Fix shader conversion failure when using native texture atomics.
 - MSL shader conversion, only pass resource bindings that apply to current shader stage.
 - Update documentation for minimum runtime OS requirements to indicate _macOS 10.15_, _iOS 13_, or _tvOS 13_.
 - Update to latest SPIRV-Cross:

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm b/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm
@@ -296,7 +296,7 @@
  if ( mvkIsAnyFlagEnabled(_buffer->getUsage(), VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT) ) {
  usage |= MTLTextureUsageShaderWrite;
 #if MVK_XCODE_15
- if (getPhysicalDevice()->useNativeTextureAtomics() && (_mtlPixelFormat == MTLPixelFormatR32Sint || _mtlPixelFormat == MTLPixelFormatR32Uint || _mtlPixelFormat == MTLPixelFormatRG32Uint))
+ if (getMetalFeatures().nativeTextureAtomics && (_mtlPixelFormat == MTLPixelFormatR32Sint || _mtlPixelFormat == MTLPixelFormatR32Uint || _mtlPixelFormat == MTLPixelFormatRG32Uint))
  usage |= MTLTextureUsageShaderAtomic;
 #endif
  }

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h
@@ -75,7 +75,8 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  uint32_t bindingIndex,
  uint32_t count,
  VkDescriptorType descType,
- MVKSampler* immutableSampler);
+ MVKSampler* immutableSampler,
+ bool usingNativeTextureAtomics);
 
 
 #pragma mark -

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm
@@ -112,7 +112,8 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  uint32_t bindingIndex,
  uint32_t count,
  VkDescriptorType descType,
- MVKSampler* immutableSampler) {
+ MVKSampler* immutableSampler,
+ bool usingNativeTextureAtomics) {
 
 #define addResourceBinding(spvRezType) \
  do { \
@@ -167,7 +168,9 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
  case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
  addResourceBinding(Image);
- addResourceBinding(Void);
+ if ( !usingNativeTextureAtomics ) {
+ addResourceBinding(Void);
+ }
  break;
 
  case VK_DESCRIPTOR_TYPE_SAMPLER:
@@ -323,7 +326,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  if (_applyToStage[i]) {
  tb.index = mtlIdxs.stages[i].textureIndex + rezIdx + planeIndex;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindTexture, pipelineBindPoint, i, tb);
- if (_info.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (_info.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !getMetalFeatures().nativeTextureAtomics) {
  bb.index = mtlIdxs.stages[i].bufferIndex + rezIdx;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindBuffer, pipelineBindPoint, i, bb);
  }
@@ -348,7 +351,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  if (_applyToStage[i]) {
  tb.index = mtlIdxs.stages[i].textureIndex + rezIdx;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindTexture, pipelineBindPoint, i, tb);
- if (_info.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (_info.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !getMetalFeatures().nativeTextureAtomics) {
  bb.index = mtlIdxs.stages[i].bufferIndex + rezIdx;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindBuffer, pipelineBindPoint, i, bb);
  }
@@ -440,7 +443,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
 
  case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
  addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
- if (!getPhysicalDevice()->useNativeTextureAtomics()) { // Needed for emulated atomic operations
+ if (!getMetalFeatures().nativeTextureAtomics) { // Needed for emulated atomic operations
  addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
  }
  break;
@@ -451,7 +454,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
 
  case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
  addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
- if (!getPhysicalDevice()->useNativeTextureAtomics()) { // Needed for emulated atomic operations
+ if (!getMetalFeatures().nativeTextureAtomics) { // Needed for emulated atomic operations
  addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
  }
  break;
@@ -507,7 +510,8 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  _info.binding,
  descCnt,
  getDescriptorType(),
- mvkSamp);
+ mvkSamp,
+ getMetalFeatures().nativeTextureAtomics);
  }
  }
 }
@@ -669,7 +673,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
  case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
  setResourceIndexOffset(textureIndex);
- if (!getPhysicalDevice()->useNativeTextureAtomics()) setResourceIndexOffset(bufferIndex);
+ if (!getMetalFeatures().nativeTextureAtomics) setResourceIndexOffset(bufferIndex);
 
  if (pBinding->descriptorCount > 1 && !mtlFeats.arrayOfTextures) {
  _layout->setConfigurationResult(reportError(VK_ERROR_FEATURE_NOT_PRESENT, "Device %s does not support arrays of textures.", _device->getName()));
@@ -932,7 +936,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  if (stages[i]) {
  tb.index = mtlIndexes.stages[i].textureIndex + elementIndex + planeIndex;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindTexture, pipelineBindPoint, i, tb);
- if (descType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !cmdEncoder->getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (descType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !cmdEncoder->getMetalFeatures().nativeTextureAtomics) {
  bb.index = mtlIndexes.stages[i].bufferIndex + elementIndex + planeIndex;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindBuffer, pipelineBindPoint, i, bb);
  }
@@ -963,7 +967,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  if (encodeUsage) {
  rezEncState->encodeResourceUsage(stage, mtlTexture, getMTLResourceUsage(), mvkDSLBind->getMTLRenderStages());
  }
- if (descType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !mvkDSLBind->getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (descType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE && !mvkDSLBind->getMetalFeatures().nativeTextureAtomics) {
  id<MTLTexture> mtlTex = mtlTexture.parentTexture ? mtlTexture.parentTexture : mtlTexture;
  id<MTLBuffer> mtlBuff = mtlTex.buffer;
  if (mtlBuff) {
@@ -1231,7 +1235,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  if (stages[i]) {
  tb.index = mtlIndexes.stages[i].textureIndex + elementIndex;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindTexture, pipelineBindPoint, i, tb);
- if (descType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !cmdEncoder->getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (descType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !cmdEncoder->getMetalFeatures().nativeTextureAtomics) {
  bb.index = mtlIndexes.stages[i].bufferIndex + elementIndex;
  BIND_GRAPHICS_OR_COMPUTE(cmdEncoder, bindBuffer, pipelineBindPoint, i, bb);
  }
@@ -1256,7 +1260,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
  rezEncState->encodeResourceUsage(stage, mtlTexture, getMTLResourceUsage(), mvkDSLBind->getMTLRenderStages());
  }
 
- if (descType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !mvkDSLBind->getPhysicalDevice()->useNativeTextureAtomics()) {
+ if (descType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER && !mvkDSLBind->getMetalFeatures().nativeTextureAtomics) {
  id<MTLBuffer> mtlBuff = mtlTexture.buffer;
  if (mtlBuff) {
  if (encodeToArgBuffer) {

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
@@ -764,7 +764,7 @@
  case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
  case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
  mtlTexCnt += poolSize.descriptorCount;
- if (!getPhysicalDevice()->useNativeTextureAtomics())
+ if (!getMetalFeatures().nativeTextureAtomics)
  mtlBuffCnt += poolSize.descriptorCount;
  break;
 

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h
@@ -357,9 +357,6 @@ class MVKPhysicalDevice : public MVKDispatchableVulkanAPIObject {
  return _metalFeatures.argumentBuffers && getMVKConfig().useMetalArgumentBuffers != MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS_NEVER;
  };
 
- /** Returns whether native texture atomics are supported and should be used. */
- bool useNativeTextureAtomics() { return _metalFeatures.nativeTextureAtomics; }
-
  /** Returns the MTLStorageMode that matches the Vulkan memory property flags. */
  MTLStorageMode getMTLStorageModeFromVkMemoryPropertyFlags(VkMemoryPropertyFlags vkFlags);
 

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm
@@ -3764,7 +3764,7 @@ static uint32_t mvkGetEntryProperty(io_registry_entry_t entry, CFStringRef prope
  case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
  case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
  mtlTexCnt += pBind->descriptorCount;
- if (_physicalDevice->useNativeTextureAtomics()) {
+ if (_physicalDevice->_metalFeatures.nativeTextureAtomics) {
  mtlBuffCnt += pBind->descriptorCount;
  }
  maxVarDescCount = min(mtlFeats.maxPerStageTextureCount - mtlTexCnt,

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
@@ -1083,7 +1083,7 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) {
 
  MTLTextureUsage mtlUsage = pixFmts->getMTLTextureUsage(getCombinedUsage(), mtlPixFmt, _samples,
  _isLinear || _isLinearForAtomics, needsReinterpretation, _hasExtendedUsage,
- _shouldSupportAtomics && getPhysicalDevice()->useNativeTextureAtomics());
+ _shouldSupportAtomics && getMetalFeatures().nativeTextureAtomics);
 
  // Metal before 3.0 doesn't support 3D compressed textures, so we'll
  // decompress the texture ourselves, and we need to be able to write to it.
@@ -1162,7 +1162,7 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) {
  ((_vkFormat == VK_FORMAT_R32_UINT || _vkFormat == VK_FORMAT_R32_SINT) ||
  (_hasMutableFormat && pixFmts->getViewClass(_vkFormat) == MVKMTLViewClass::Color32 && (getIsValidViewFormat(VK_FORMAT_R32_UINT) || getIsValidViewFormat(VK_FORMAT_R32_SINT))));
 
- _isLinearForAtomics = _shouldSupportAtomics && !getPhysicalDevice()->useNativeTextureAtomics() && _arrayLayers == 1 && getImageType() == VK_IMAGE_TYPE_2D;
+ _isLinearForAtomics = _shouldSupportAtomics && !getMetalFeatures().nativeTextureAtomics && _arrayLayers == 1 && getImageType() == VK_IMAGE_TYPE_2D;
 
  _is3DCompressed = (getImageType() == VK_IMAGE_TYPE_3D) && (pixFmts->getFormatType(pCreateInfo->format) == kMVKFormatCompressed) && !mtlFeats.native3DCompressedTextures;
  _isDepthStencilAttachment = (mvkAreAllFlagsEnabled(pCreateInfo->usage, VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) ||

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
@@ -104,7 +104,8 @@
  kPushConstBinding,
  1,
  VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT,
- nullptr);
+ nullptr,
+ getMetalFeatures().nativeTextureAtomics);
  }
  }