Skip to content

Commit

Permalink
Merge pull request #2273 from billhollings/alloc-var-count-descs-from…
Browse files Browse the repository at this point in the history 
…-pool

Descriptor set only consumes the variable number of descriptors from the pool.
  • Loading branch information
@billhollings
billhollings authored Jul 14, 2024
2 parents 8bfa85b + 3262113 commit 96f9d89
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 176 deletions.
2 changes: 2 additions & 0 deletions Docs/Whats_New.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Released TBD
- Use Metal argument buffers by default when they are available.
- Revert `MVKConfiguration::useMetalArgumentBuffers` and env var
`MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS` to a boolean value, and enable it by default.
- Support a descriptor pool with less descriptors than the descriptor set layout,
as long as the pool has enough descriptors for the variable descriptor count,
- Update max number of bindless buffers and textures per stage to 1M, per Apple Docs.
- Add option to generate a GPU capture via a temporary named pipe from an external process.
- Fix shader conversion failure when using native texture atomics.
Expand Down
2 changes: 1 addition & 1 deletion MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ - (void)setDepthBoundsTestAMD:(BOOL)enable minDepth:(float)minDepth maxDepth:(fl
auto* dslBind = dsLayout->getBindingAt(dslBindIdx);
if (dslBind->getApplyToStage(stage) && shaderBindingUsage.getBit(dslBindIdx)) {
shouldBindArgBuffToStage = true;
uint32_t elemCnt = dslBind->getDescriptorCount(descSet);
uint32_t elemCnt = dslBind->getDescriptorCount(descSet->getVariableDescriptorCount());
for (uint32_t elemIdx = 0; elemIdx < elemCnt; elemIdx++) {
uint32_t descIdx = dslBind->getDescriptorIndex(elemIdx);
if (resourceUsageDirtyDescs.getBit(descIdx, true)) {
Expand Down
33 changes: 24 additions & 9 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ class MVKCommandEncoder;
class MVKResourcesCommandEncoderState;


/** Magic number to indicate the variable descriptor count is currently unknown. */
static uint32_t kMVKVariableDescriptorCountUnknown = std::numeric_limits<uint32_t>::max();


#pragma mark MVKShaderStageResourceBinding

/** Indicates the Metal resource indexes used by a single shader stage in a descriptor. */
Expand Down Expand Up @@ -100,11 +104,10 @@ class MVKDescriptorSetLayoutBinding : public MVKBaseDeviceObject {
* Returns the number of descriptors in this layout.
*
* If this is an inline block data descriptor, always returns 1. If this descriptor
* has a variable descriptor count, and descSet is not null, the variable descriptor
* count provided to that descriptor set is returned. Otherwise returns the value
* defined in VkDescriptorSetLayoutBinding::descriptorCount.
* has a variable descriptor count, and it is provided here, it is returned.
* Otherwise returns the value defined in VkDescriptorSetLayoutBinding::descriptorCount.
*/
uint32_t getDescriptorCount(MVKDescriptorSet* descSet = nullptr) const;
uint32_t getDescriptorCount(uint32_t variableDescriptorCount = kMVKVariableDescriptorCountUnknown) const;

/** Returns the descriptor type of this layout. */
VkDescriptorType getDescriptorType() { return _info.descriptorType; }
Expand Down Expand Up @@ -157,7 +160,9 @@ class MVKDescriptorSetLayoutBinding : public MVKBaseDeviceObject {
MVKDescriptorSetLayoutBinding(MVKDevice* device,
MVKDescriptorSetLayout* layout,
const VkDescriptorSetLayoutBinding* pBinding,
VkDescriptorBindingFlagsEXT bindingFlags);
VkDescriptorBindingFlagsEXT bindingFlags,
uint32_t& dslDescCnt,
uint32_t& dslMTLRezCnt);

MVKDescriptorSetLayoutBinding(const MVKDescriptorSetLayoutBinding& binding);

Expand All @@ -168,9 +173,13 @@ class MVKDescriptorSetLayoutBinding : public MVKBaseDeviceObject {
friend class MVKDescriptorSet;
friend class MVKInlineUniformBlockDescriptor;

void initMetalResourceIndexOffsets(const VkDescriptorSetLayoutBinding* pBinding, uint32_t stage);
void addMTLArgumentDescriptors(NSMutableArray<MTLArgumentDescriptor*>* args);
void initMetalResourceIndexOffsets(const VkDescriptorSetLayoutBinding* pBinding,
uint32_t stage,
uint32_t dslMTLRezCnt);
void addMTLArgumentDescriptors(NSMutableArray<MTLArgumentDescriptor*>* args,
uint32_t variableDescriptorCount);
void addMTLArgumentDescriptor(NSMutableArray<MTLArgumentDescriptor*>* args,
uint32_t variableDescriptorCount,
uint32_t argIndex,
MTLDataType dataType,
MTLArgumentAccess access);
Expand All @@ -180,8 +189,7 @@ class MVKDescriptorSetLayoutBinding : public MVKBaseDeviceObject {
bool validate(MVKSampler* mvkSampler);
void encodeImmutableSamplersToMetalArgumentBuffer(MVKDescriptorSet* mvkDescSet);
uint8_t getMaxPlaneCount();
uint32_t getMTLResourceCount();
bool needsBuffSizeAuxBuffer();
uint32_t getMTLResourceCount(uint32_t variableDescriptorCount = kMVKVariableDescriptorCountUnknown);
std::string getLogDescription();

MVKDescriptorSetLayout* _layout;
Expand Down Expand Up @@ -674,5 +682,12 @@ class MVKStorageTexelBufferDescriptor : public MVKTexelBufferDescriptor {
#pragma mark -
#pragma mark Support functions

/**
* If the binding defines a buffer type, returns whether there are buffers, and
* therefore an auxilliary buffer is required to hold the lengths of those buffers.
* Returns false if the binding does not define a buffer type.
*/
bool mvkNeedsBuffSizeAuxBuffer(const VkDescriptorSetLayoutBinding* pBinding);

/** Returns the name of the descriptor type. */
const char* mvkVkDescriptorTypeName(VkDescriptorType vkDescType);
120 changes: 62 additions & 58 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,13 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s

MVKVulkanAPIObject* MVKDescriptorSetLayoutBinding::getVulkanAPIObject() { return _layout; };

uint32_t MVKDescriptorSetLayoutBinding::getDescriptorCount(MVKDescriptorSet* descSet) const {

uint32_t MVKDescriptorSetLayoutBinding::getDescriptorCount(uint32_t variableDescriptorCount) const {
if (_info.descriptorType == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {
return 1;
}

if (descSet && hasVariableDescriptorCount()) {
return descSet->_variableDescriptorCount;
if (hasVariableDescriptorCount()) {
return std::min(variableDescriptorCount, _info.descriptorCount);
}

return _info.descriptorCount;
}

Expand All @@ -215,7 +212,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
MVKShaderResourceBinding mtlIdxs = _mtlResourceIndexOffsets + dslMTLRezIdxOffsets;

VkDescriptorType descType = getDescriptorType();
uint32_t descCnt = getDescriptorCount(descSet);
uint32_t descCnt = getDescriptorCount(descSet->_variableDescriptorCount);
for (uint32_t descIdx = 0; descIdx < descCnt; descIdx++) {
MVKDescriptor* mvkDesc = descSet->getDescriptor(getBinding(), descIdx);
if (mvkDesc->getDescriptorType() == descType) {
Expand Down Expand Up @@ -417,53 +414,54 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
}

// Adds MTLArgumentDescriptors to the array, and updates resource indexes consumed.
void MVKDescriptorSetLayoutBinding::addMTLArgumentDescriptors(NSMutableArray<MTLArgumentDescriptor*>* args) {
void MVKDescriptorSetLayoutBinding::addMTLArgumentDescriptors(NSMutableArray<MTLArgumentDescriptor*>* args,
uint32_t variableDescriptorCount) {
switch (getDescriptorType()) {

case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadOnly);
break;

case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
break;

case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE:
case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
break;

case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
if (!getMetalFeatures().nativeTextureAtomics) { // Needed for emulated atomic operations
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
}
break;

case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
break;

case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().textureIndex, MTLDataTypeTexture, MTLArgumentAccessReadWrite);
if (!getMetalFeatures().nativeTextureAtomics) { // Needed for emulated atomic operations
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().bufferIndex, MTLDataTypePointer, MTLArgumentAccessReadWrite);
}
break;

case VK_DESCRIPTOR_TYPE_SAMPLER:
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().samplerIndex, MTLDataTypeSampler, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().samplerIndex, MTLDataTypeSampler, MTLArgumentAccessReadOnly);
break;

case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: {
uint8_t maxPlaneCnt = getMaxPlaneCount();
for (uint8_t planeIdx = 0; planeIdx < maxPlaneCnt; planeIdx++) {
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().textureIndex + planeIdx, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().textureIndex + planeIdx, MTLDataTypeTexture, MTLArgumentAccessReadOnly);
}
addMTLArgumentDescriptor(args, getMetalResourceIndexOffsets().samplerIndex, MTLDataTypeSampler, MTLArgumentAccessReadOnly);
addMTLArgumentDescriptor(args, variableDescriptorCount, getMetalResourceIndexOffsets().samplerIndex, MTLDataTypeSampler, MTLArgumentAccessReadOnly);
break;
}

Expand All @@ -473,10 +471,11 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
}

void MVKDescriptorSetLayoutBinding::addMTLArgumentDescriptor(NSMutableArray<MTLArgumentDescriptor*>* args,
uint32_t variableDescriptorCount,
uint32_t argIndex,
MTLDataType dataType,
MTLArgumentAccess access) {
uint32_t descCnt = getDescriptorCount();
uint32_t descCnt = getDescriptorCount(variableDescriptorCount);
if (descCnt == 0) { return; }

auto* argDesc = [MTLArgumentDescriptor argumentDescriptor];
Expand All @@ -497,7 +496,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
return maxPlaneCnt;
}

uint32_t MVKDescriptorSetLayoutBinding::getMTLResourceCount() {
uint32_t MVKDescriptorSetLayoutBinding::getMTLResourceCount(uint32_t variableDescriptorCount) {
uint32_t rezCntPerElem = 1;
switch (_info.descriptorType) {
case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
Expand All @@ -510,7 +509,7 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
default:
break;
}
return rezCntPerElem * getDescriptorCount();
return rezCntPerElem * getDescriptorCount(variableDescriptorCount);
}

// Encodes an immutable sampler to the Metal argument buffer.
Expand All @@ -532,13 +531,15 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
void MVKDescriptorSetLayoutBinding::populateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& shaderConfig,
MVKShaderResourceBinding& dslMTLRezIdxOffsets,
uint32_t dslIndex) {
uint32_t descCnt = getDescriptorCount();
// For argument buffers, set variable length arrays to length 1 in shader.
bool isUsingMtlArgBuff = _layout->isUsingMetalArgumentBuffers();
MVKSampler* mvkSamp = !_immutableSamplers.empty() ? _immutableSamplers.front() : nullptr;
uint32_t descCnt = isUsingMtlArgBuff ? getDescriptorCount(1) : getDescriptorCount();

// Establish the resource indices to use, by combining the offsets of the DSL and this DSL binding.
MVKShaderResourceBinding mtlIdxs = _mtlResourceIndexOffsets + dslMTLRezIdxOffsets;

MVKSampler* mvkSamp = !_immutableSamplers.empty() ? _immutableSamplers.front() : nullptr;

for (uint32_t stage = kMVKShaderStageVertex; stage < kMVKShaderStageCount; stage++) {
if (_applyToStage[stage] || isUsingMtlArgBuff) {
mvkPopulateShaderConversionConfig(shaderConfig,
Expand Down Expand Up @@ -600,12 +601,14 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
MVKDescriptorSetLayoutBinding::MVKDescriptorSetLayoutBinding(MVKDevice* device,
MVKDescriptorSetLayout* layout,
const VkDescriptorSetLayoutBinding* pBinding,
VkDescriptorBindingFlagsEXT bindingFlags) :
VkDescriptorBindingFlagsEXT bindingFlags,
uint32_t& dslDescCnt,
uint32_t& dslMTLRezCnt) :
MVKBaseDeviceObject(device),
_layout(layout),
_info(*pBinding),
_flags(bindingFlags),
_descriptorIndex(layout->_descriptorCount) {
_descriptorIndex(dslDescCnt) {

// If immutable samplers are defined, copy them in.
// Do this before anything else, because they are referenced in getMaxPlaneCount().
Expand All @@ -624,14 +627,14 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
// Determine if this binding is used by this shader stage, and initialize resource indexes.
for (uint32_t stage = kMVKShaderStageVertex; stage < kMVKShaderStageCount; stage++) {
_applyToStage[stage] = mvkAreAllFlagsEnabled(pBinding->stageFlags, mvkVkShaderStageFlagBitsFromMVKShaderStage(MVKShaderStage(stage)));
initMetalResourceIndexOffsets(pBinding, stage);
initMetalResourceIndexOffsets(pBinding, stage, dslMTLRezCnt);
}

// Update descriptor set layout counts
uint32_t descCnt = getDescriptorCount();
_layout->_descriptorCount += descCnt;
_layout->_mtlResourceTotalCount += getMTLResourceCount();
if (needsBuffSizeAuxBuffer()) {
dslDescCnt += descCnt;
dslMTLRezCnt += getMTLResourceCount();
if (mvkNeedsBuffSizeAuxBuffer(pBinding)) {
_layout->_maxBufferIndex = std::max(_layout->_maxBufferIndex, int32_t(_mtlResourceIndexOffsets.getMaxBufferIndex() + descCnt) - 1);
}
}
Expand Down Expand Up @@ -661,19 +664,20 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s

// Sets the appropriate Metal resource indexes within this binding from the
// specified descriptor set binding counts, and updates those counts accordingly.
void MVKDescriptorSetLayoutBinding::initMetalResourceIndexOffsets(const VkDescriptorSetLayoutBinding* pBinding, uint32_t stage) {

void MVKDescriptorSetLayoutBinding::initMetalResourceIndexOffsets(const VkDescriptorSetLayoutBinding* pBinding,
uint32_t stage,
uint32_t dslMTLRezCnt) {
// Sets an index offset and updates both that index and the general resource index.
// Can be used more than once for combined multi-resource descriptor types.
// When using Metal argument buffers, we accumulate the resource indexes cummulatively, across all resource types.
#define setResourceIndexOffset(rezIdx, mtlRezCntPerElem) \
if (isUsingMtlArgBuff) { \
bindIdxs.rezIdx = _layout->_mtlResourceTotalCount + descIdxOfst; \
descIdxOfst += descCnt * mtlRezCntPerElem; \
} else if (_applyToStage[stage]) { \
bindIdxs.rezIdx = dslCnts.rezIdx; \
dslCnts.rezIdx += descCnt * mtlRezCntPerElem; \
} \
#define setResourceIndexOffset(rezIdx, mtlRezCntPerElem) \
if (isUsingMtlArgBuff) { \
bindIdxs.rezIdx = dslMTLRezCnt + descIdxOfst; \
descIdxOfst += descCnt * mtlRezCntPerElem; \
} else if (_applyToStage[stage]) { \
bindIdxs.rezIdx = dslCnts.rezIdx; \
dslCnts.rezIdx += descCnt * mtlRezCntPerElem; \
} \

bool isUsingMtlArgBuff = _layout->isUsingMetalArgumentBuffers();
auto& mtlFeats = getMetalFeatures();
Expand Down Expand Up @@ -744,24 +748,6 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
}
}

bool MVKDescriptorSetLayoutBinding::needsBuffSizeAuxBuffer() {

if ( !_layout->isUsingMetalArgumentBuffers() ) { return false; }
if ( getDescriptorCount() == 0 ) { return false; }

switch (getDescriptorType()) {
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
return true;

default:
return false;
}
}


#pragma mark -
#pragma mark MVKDescriptor
Expand Down Expand Up @@ -1355,6 +1341,24 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s
#pragma mark -
#pragma mark Support functions


bool mvkNeedsBuffSizeAuxBuffer(const VkDescriptorSetLayoutBinding* pBinding) {
switch (pBinding->descriptorType) {
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
return pBinding->descriptorCount > 0;

case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
return true;

default:
return false;
}
}


#define CASE_STRINGIFY(V) case V: return #V

const char* mvkVkDescriptorTypeName(VkDescriptorType vkDescType) {
Expand Down
Loading

0 comments on commit 96f9d89

Please sign in to comment.