Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix occasional GPU crash when a smaller descriptor set replaces a larger one. #2355

Merged
merged 1 commit into from
Sep 28, 2024
+136 −187
Merged

Fix occasional GPU crash when a smaller descriptor set replaces a larger one. #2355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ - (void)setDepthBoundsTestAMD:(BOOL)enable minDepth:(float)minDepth maxDepth:(fl
if (dsChanged) {
auto& usageDirty = _metalUsageDirtyDescriptors[descSetIndex];
usageDirty.resize(descSet->getDescriptorCount());
usageDirty.setAllBits();
usageDirty.enableAllBits();
}

// Update dynamic buffer offsets
Expand Down Expand Up @@ -717,7 +717,7 @@ - (void)setDepthBoundsTestAMD:(BOOL)enable minDepth:(float)minDepth maxDepth:(fl
MVKCommandEncoderState::markDirty();
if (_cmdEncoder->isUsingMetalArgumentBuffers()) {
for (uint32_t dsIdx = 0; dsIdx < kMVKMaxDescriptorSetCount; dsIdx++) {
_metalUsageDirtyDescriptors[dsIdx].setAllBits();
_metalUsageDirtyDescriptors[dsIdx].enableAllBits();
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,9 @@ class MVKDescriptorPool : public MVKVulkanAPIDeviceObject {

MVKSmallVector<MVKDescriptorSet> _descriptorSets;
MVKBitArray _descriptorSetAvailablility;
id<MTLBuffer> _metalArgumentBuffer;
NSUInteger _nextMetalArgumentBufferOffset;
MVKMTLBufferAllocator _mtlBufferAllocator;
id<MTLBuffer> _metalArgumentBuffer = nil;
NSUInteger _nextMetalArgumentBufferOffset = 0;

MVKDescriptorTypePool<MVKUniformBufferDescriptor> _uniformBufferDescriptors;
MVKDescriptorTypePool<MVKStorageBufferDescriptor> _storageBufferDescriptors;
Expand All @@ -322,7 +322,8 @@ class MVKDescriptorPool : public MVKVulkanAPIDeviceObject {
MVKDescriptorTypePool<MVKUniformTexelBufferDescriptor> _uniformTexelBufferDescriptors;
MVKDescriptorTypePool<MVKStorageTexelBufferDescriptor> _storageTexelBufferDescriptors;

VkDescriptorPoolCreateFlags _flags;
VkDescriptorPoolCreateFlags _flags = 0;
size_t _maxAllocDescSetCount = 0;
};


Expand Down
28 changes: 13 additions & 15 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
for (uint32_t bindIdx = 0; bindIdx < bindCnt; bindIdx++) {
auto& dslBind = _bindings[bindIdx];
if (context.isResourceUsed(spvExecModels[stage], descSetIndex, dslBind.getBinding())) {
bindingUse.setBit(bindIdx);
bindingUse.enableBit(bindIdx);
descSetIsUsed = true;
}
}
Expand Down Expand Up @@ -628,9 +628,9 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
bool& dynamicAllocation,
MVKDescriptorPool* pool) {
VkResult errRslt = VK_ERROR_OUT_OF_POOL_MEMORY;
size_t availDescIdx = _availability.getIndexOfFirstSetBit();
size_t availDescIdx = _availability.getIndexOfFirstEnabledBit();
if (availDescIdx < size()) {
_availability.clearBit(availDescIdx); // Mark the descriptor as taken
_availability.disableBit(availDescIdx); // Mark the descriptor as taken
*pMVKDesc = &_descriptors[availDescIdx];
(*pMVKDesc)->reset(); // Reset descriptor before reusing.
dynamicAllocation = false;
Expand All @@ -657,7 +657,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
DescriptorClass* pFirstDesc = _descriptors.data();
int64_t descIdx = pDesc >= pFirstDesc ? pDesc - pFirstDesc : pFirstDesc - pDesc;
if (descIdx >= 0 && descIdx < size()) {
_availability.setBit(descIdx);
_availability.enableBit(descIdx);
} else {
mvkDesc->destroy();
}
Expand All @@ -666,13 +666,13 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
// Preallocated descriptors will be reset when they are reused
template<typename DescriptorClass>
void MVKDescriptorTypePool<DescriptorClass>::reset() {
_availability.setAllBits();
_availability.enableAllBits();
}

template<typename DescriptorClass>
size_t MVKDescriptorTypePool<DescriptorClass>::getRemainingDescriptorCount() {
size_t enabledCount = 0;
_availability.enumerateEnabledBits(false, [&](size_t bitIdx) { enabledCount++; return true; });
_availability.enumerateEnabledBits([&](size_t bitIdx) { enabledCount++; return true; });
return enabledCount;
}

Expand Down Expand Up @@ -740,7 +740,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
uint64_t mtlArgBuffEncAlignedSize = mvkAlignByteCount(mtlArgBuffEncSize, getMetalFeatures().mtlBufferAlignment);

size_t dsCnt = _descriptorSetAvailablility.size();
_descriptorSetAvailablility.enumerateEnabledBits(true, [&](size_t dsIdx) {
_descriptorSetAvailablility.enumerateEnabledBits([&](size_t dsIdx) {
bool isSpaceAvail = true; // If not using Metal arg buffers, space will always be available.
MVKDescriptorSet* mvkDS = &_descriptorSets[dsIdx];
NSUInteger mtlArgBuffOffset = mvkDS->getMetalArgumentBuffer().getMetalArgumentBufferOffset();
Expand Down Expand Up @@ -772,11 +772,12 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
if (rslt) {
freeDescriptorSet(mvkDS, false);
} else {
_descriptorSetAvailablility.disableBit(dsIdx);
_maxAllocDescSetCount = std::max(_maxAllocDescSetCount, dsIdx + 1);
*pVKDS = (VkDescriptorSet)mvkDS;
}
return false;
} else {
_descriptorSetAvailablility.setBit(dsIdx); // We didn't consume this one after all, so it's still available
return true;
}
});
Expand All @@ -800,7 +801,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
mvkDS->free(isPoolReset);
if ( !isPoolReset ) {
size_t dsIdx = mvkDS - _descriptorSets.data();
_descriptorSetAvailablility.setBit(dsIdx);
_descriptorSetAvailablility.enableBit(dsIdx);
}
} else {
reportError(VK_ERROR_INITIALIZATION_FAILED, "A descriptor set is being returned to a descriptor pool that did not allocate it.");
Expand All @@ -810,11 +811,10 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
// Free allocated descriptor sets and reset descriptor pools.
// Don't waste time freeing desc sets that were never allocated.
VkResult MVKDescriptorPool::reset(VkDescriptorPoolResetFlags flags) {
size_t dsCnt = _descriptorSetAvailablility.getLowestNeverClearedBitIndex();
for (uint32_t dsIdx = 0; dsIdx < dsCnt; dsIdx++) {
for (uint32_t dsIdx = 0; dsIdx < _maxAllocDescSetCount; dsIdx++) {
freeDescriptorSet(&_descriptorSets[dsIdx], true);
}
_descriptorSetAvailablility.setAllBits();
_descriptorSetAvailablility.enableAllBits();

_uniformBufferDescriptors.reset();
_storageBufferDescriptors.reset();
Expand All @@ -830,6 +830,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
_storageTexelBufferDescriptors.reset();

_nextMetalArgumentBufferOffset = 0;
_maxAllocDescSetCount = 0;

return VK_SUCCESS;
}
Expand Down Expand Up @@ -1003,9 +1004,6 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
}

void MVKDescriptorPool::initMetalArgumentBuffer(const VkDescriptorPoolCreateInfo* pCreateInfo) {
_metalArgumentBuffer = nil;
_nextMetalArgumentBufferOffset = 0;

if ( !isUsingMetalArgumentBuffers() ) { return; }

auto& mtlFeats = getMetalFeatures();
Expand Down
Loading
Loading