Fix crash on pipeline cache merge after VkShaderModule destroyed.

Set MTLFunction label immediately after creation from MVKShaderModule.
Remove _shaderModule reference from MVKShaderLibrary & MVKShaderLibraryCache.
No-op propogateDebugName() function in MVKShaderModule & MVKPipelineCache.
This commit is contained in:
Bill Hollings 2019-05-30 17:53:11 -04:00
parent 97a1f68069
commit 70b78ac7fa
6 changed files with 11 additions and 70 deletions

View File

@ -177,7 +177,7 @@ void MVKCommandBuffer::clearPrefilledMTLCommandBuffer() {
#pragma mark Construction
// Initializes this instance after it has been created retrieved from a pool.
// Initializes this instance after it has been created or retrieved from a pool.
void MVKCommandBuffer::init(const VkCommandBufferAllocateInfo* pAllocateInfo) {
_commandPool = (MVKCommandPool*)pAllocateInfo->commandPool;
_isSecondary = (pAllocateInfo->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY);

View File

@ -1839,7 +1839,7 @@ VkResult MVKDevice::createPipelines(VkPipelineCache pipelineCache,
}
// Create concrete implementations of the two variations of the mvkCreatePipelines() function
// that we will be using. This is required since the template definition is location in this
// that we will be using. This is required since the template definition is located in this
// implementation file instead of in the header file. This is a realistic approach if the
// universe of possible template implementation variations is small and known in advance.
template VkResult MVKDevice::createPipelines<MVKGraphicsPipeline, VkGraphicsPipelineCreateInfo>(VkPipelineCache pipelineCache,

View File

@ -365,7 +365,7 @@ public:
~MVKPipelineCache() override;
protected:
void propogateDebugName() override;
void propogateDebugName() override {}
MVKShaderLibraryCache* getShaderLibraryCache(MVKShaderModuleKey smKey);
void readData(const VkPipelineCacheCreateInfo* pCreateInfo);
void writeData(std::ostream& outstream, bool isCounting = false);

View File

@ -1268,20 +1268,12 @@ MVKComputePipeline::~MVKComputePipeline() {
#pragma mark -
#pragma mark MVKPipelineCache
void MVKPipelineCache::propogateDebugName() {
lock_guard<mutex> lock(_shaderCacheLock);
for (auto& slPair : _shaderCache) { slPair.second->propogateDebugName(); }
}
// Return a shader library from the specified shader context sourced from the specified shader module.
MVKShaderLibrary* MVKPipelineCache::getShaderLibrary(SPIRVToMSLConverterContext* pContext, MVKShaderModule* shaderModule) {
lock_guard<mutex> lock(_shaderCacheLock);
bool wasAdded = false;
MVKShaderLibraryCache* slCache = getShaderLibraryCache(shaderModule->getKey());
slCache->setShaderModule(shaderModule);
MVKShaderLibrary* shLib = slCache->getShaderLibrary(pContext, shaderModule, &wasAdded);
if (wasAdded) { markDirty(); }
return shLib;

View File

@ -75,15 +75,11 @@ protected:
friend MVKShaderLibraryCache;
friend MVKShaderModule;
void propogateDebugName();
NSString* getDebugName();
void setShaderModule(MVKShaderModule* shaderModule);
MVKMTLFunction getMTLFunction(const VkSpecializationInfo* pSpecializationInfo);
MVKMTLFunction getMTLFunction(const VkSpecializationInfo* pSpecializationInfo, MVKShaderModule* shaderModule);
void handleCompilationError(NSError* err, const char* opDesc);
MTLFunctionConstant* getFunctionConstant(NSArray<MTLFunctionConstant*>* mtlFCs, NSUInteger mtlFCID);
MVKVulkanAPIDeviceObject* _owner;
MVKShaderModule* _shaderModule = nullptr;
id<MTLLibrary> _mtlLibrary;
SPIRVEntryPoint _entryPoint;
std::string _msl;
@ -111,11 +107,6 @@ public:
MVKShaderLibrary* getShaderLibrary(SPIRVToMSLConverterContext* pContext,
MVKShaderModule* shaderModule,
bool* pWasAdded = nullptr);
/**
* Sets the shader module associated with this library cache.
* This is set after creation because libraries can be loaded from a pipeline cache.
*/
void setShaderModule(MVKShaderModule* shaderModule);
MVKShaderLibraryCache(MVKVulkanAPIDeviceObject* owner) : _owner(owner) {};
@ -126,7 +117,6 @@ protected:
friend MVKPipelineCache;
friend MVKShaderModule;
void propogateDebugName();
MVKShaderLibrary* findShaderLibrary(SPIRVToMSLConverterContext* pContext);
MVKShaderLibrary* addShaderLibrary(SPIRVToMSLConverterContext* pContext,
const std::string& mslSourceCode,
@ -134,7 +124,6 @@ protected:
void merge(MVKShaderLibraryCache* other);
MVKVulkanAPIDeviceObject* _owner;
MVKShaderModule* _shaderModule = nullptr;
std::vector<std::pair<SPIRVToMSLConverterContext, MVKShaderLibrary*>> _shaderLibraries;
};
@ -208,7 +197,7 @@ public:
protected:
friend MVKShaderCacheIterator;
void propogateDebugName() override;
void propogateDebugName() override {}
MVKGLSLConversionShaderStage getMVKGLSLConversionShaderStage(SPIRVToMSLConverterContext* pContext);
MVKShaderLibraryCache _shaderLibraryCache;

View File

@ -31,16 +31,6 @@ const MVKMTLFunction MVKMTLFunctionNull = { nil, MTLSizeMake(1, 1, 1) };
#pragma mark -
#pragma mark MVKShaderLibrary
void MVKShaderLibrary::propogateDebugName() { setLabelIfNotNil(_mtlLibrary, getDebugName()); }
// First choice is to name after shader module if it exists, otherwise name after owner.
NSString* MVKShaderLibrary::getDebugName() {
NSString* dbName = _shaderModule ? _shaderModule-> getDebugName() : nil;
if (dbName) { return dbName; }
return _owner ? _owner-> getDebugName() : nil;
}
// If the size of the workgroup dimension is specialized, extract it from the
// specialization info, otherwise use the value specified in the SPIR-V shader code.
static uint32_t getWorkgroupDimensionSize(const SPIRVWorkgroupSizeDimension& wgDim, const VkSpecializationInfo* pSpecInfo) {
@ -56,7 +46,7 @@ static uint32_t getWorkgroupDimensionSize(const SPIRVWorkgroupSizeDimension& wgD
return wgDim.size;
}
MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpecializationInfo) {
MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpecializationInfo, MVKShaderModule* shaderModule) {
if ( !_mtlLibrary ) { return MVKMTLFunctionNull; }
@ -101,7 +91,10 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
reportError(VK_ERROR_INVALID_SHADER_NV, "Shader module does not contain an entry point named '%s'.", mtlFuncName.UTF8String);
}
setLabelIfNotNil(mtlFunc, getDebugName());
// Set the debug name. First try name of shader module, otherwise try name of owner.
NSString* dbName = shaderModule-> getDebugName();
if ( !dbName ) { dbName = _owner-> getDebugName(); }
setLabelIfNotNil(mtlFunc, dbName);
return { mtlFunc, MTLSizeMake(getWorkgroupDimensionSize(_entryPoint.workgroupSize.width, pSpecializationInfo),
getWorkgroupDimensionSize(_entryPoint.workgroupSize.height, pSpecializationInfo),
@ -116,18 +109,10 @@ MTLFunctionConstant* MVKShaderLibrary::getFunctionConstant(NSArray<MTLFunctionCo
return nil;
}
void MVKShaderLibrary::setShaderModule(MVKShaderModule* shaderModule) {
if (shaderModule == _shaderModule) { return; }
_shaderModule = shaderModule;
propogateDebugName();
}
MVKShaderLibrary::MVKShaderLibrary(MVKVulkanAPIDeviceObject* owner, const string& mslSourceCode, const SPIRVEntryPoint& entryPoint) : _owner(owner) {
MVKShaderLibraryCompiler* slc = new MVKShaderLibraryCompiler(_owner);
_mtlLibrary = slc->newMTLLibrary(@(mslSourceCode.c_str())); // retained
slc->destroy();
propogateDebugName();
_entryPoint = entryPoint;
_msl = mslSourceCode;
@ -148,7 +133,6 @@ MVKShaderLibrary::MVKShaderLibrary(MVKVulkanAPIDeviceObject* owner,
handleCompilationError(err, "Compiled shader module creation");
[shdrData release];
}
propogateDebugName();
mvkDev->addActivityPerformance(mvkDev->_performanceStatistics.shaderCompilation.mslLoad, startTime);
}
@ -156,7 +140,6 @@ MVKShaderLibrary::MVKShaderLibrary(MVKShaderLibrary& other) : _owner(other._owne
_mtlLibrary = [other._mtlLibrary retain];
_entryPoint = other._entryPoint;
_msl = other._msl;
setShaderModule(other._shaderModule);
}
// If err object is nil, the compilation succeeded without any warnings.
@ -183,10 +166,6 @@ MVKShaderLibrary::~MVKShaderLibrary() {
#pragma mark -
#pragma mark MVKShaderLibraryCache
void MVKShaderLibraryCache::propogateDebugName() {
for (auto& slPair : _shaderLibraries) { slPair.second->propogateDebugName(); }
}
MVKShaderLibrary* MVKShaderLibraryCache::getShaderLibrary(SPIRVToMSLConverterContext* pContext,
MVKShaderModule* shaderModule,
bool* pWasAdded) {
@ -221,7 +200,6 @@ MVKShaderLibrary* MVKShaderLibraryCache::addShaderLibrary(SPIRVToMSLConverterCon
const string& mslSourceCode,
const SPIRVEntryPoint& entryPoint) {
MVKShaderLibrary* shLib = new MVKShaderLibrary(_owner, mslSourceCode, entryPoint);
shLib->setShaderModule(_shaderModule);
_shaderLibraries.emplace_back(*pContext, shLib);
return shLib;
}
@ -236,13 +214,6 @@ void MVKShaderLibraryCache::merge(MVKShaderLibraryCache* other) {
}
}
void MVKShaderLibraryCache::setShaderModule(MVKShaderModule* shaderModule) {
if (shaderModule == _shaderModule) { return; }
_shaderModule = shaderModule;
for (auto& slPair : _shaderLibraries) { slPair.second->setShaderModule(_shaderModule); }
}
MVKShaderLibraryCache::~MVKShaderLibraryCache() {
for (auto& slPair : _shaderLibraries) { slPair.second->destroy(); }
}
@ -251,13 +222,6 @@ MVKShaderLibraryCache::~MVKShaderLibraryCache() {
#pragma mark -
#pragma mark MVKShaderModule
void MVKShaderModule::propogateDebugName() {
lock_guard<mutex> lock(_accessLock);
_shaderLibraryCache.propogateDebugName();
if (_defaultLibrary) { _defaultLibrary->propogateDebugName(); }
}
MVKMTLFunction MVKShaderModule::getMTLFunction(SPIRVToMSLConverterContext* pContext,
const VkSpecializationInfo* pSpecializationInfo,
MVKPipelineCache* pipelineCache) {
@ -276,7 +240,7 @@ MVKMTLFunction MVKShaderModule::getMTLFunction(SPIRVToMSLConverterContext* pCont
pContext->markAllAttributesAndResourcesUsed();
}
return mvkLib ? mvkLib->getMTLFunction(pSpecializationInfo) : MVKMTLFunctionNull;
return mvkLib ? mvkLib->getMTLFunction(pSpecializationInfo, this) : MVKMTLFunctionNull;
}
bool MVKShaderModule::convert(SPIRVToMSLConverterContext* pContext) {
@ -335,8 +299,6 @@ MVKShaderModule::MVKShaderModule(MVKDevice* device,
const VkShaderModuleCreateInfo* pCreateInfo) : MVKVulkanAPIDeviceObject(device), _shaderLibraryCache(this) {
_defaultLibrary = nullptr;
_shaderLibraryCache.setShaderModule(this);
size_t codeSize = pCreateInfo->codeSize;
@ -374,7 +336,6 @@ MVKShaderModule::MVKShaderModule(MVKDevice* device,
_spvConverter.setMSL(pMSLCode, nullptr);
_defaultLibrary = new MVKShaderLibrary(this, _spvConverter.getMSL().c_str(), _spvConverter.getEntryPoint());
_defaultLibrary->setShaderModule(this);
break;
}
@ -389,7 +350,6 @@ MVKShaderModule::MVKShaderModule(MVKDevice* device,
_device->addActivityPerformance(_device->_performanceStatistics.shaderCompilation.hashShaderCode, startTime);
_defaultLibrary = new MVKShaderLibrary(this, (void*)(pMSLCode), mslCodeLen);
_defaultLibrary->setShaderModule(this);
break;
}