Skip to content

Commit

Permalink
Merge pull request #1715 from cdavis5e/tess-unwritten-builtin-read
Browse files Browse the repository at this point in the history 
MVKPipeline: Add builtins that are read but not written to tessellation pipelines.
  • Loading branch information
@billhollings
billhollings authored Sep 14, 2022
2 parents 5bae97a + fafcc4b commit 250e1f9
Show file tree
Hide file tree
Showing 8 changed files with 283 additions and 94 deletions.
2 changes: 1 addition & 1 deletion ExternalRevisions/SPIRV-Cross_repo_revision
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
61c603f3baa5270e04bcfb6acf83c654e3c57679
f6ca6178251c3c886d99781c5437df919fc21734
8 changes: 5 additions & 3 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ class MVKGraphicsPipeline : public MVKPipeline {
~MVKGraphicsPipeline() override;

protected:
typedef MVKSmallVector<SPIRVShaderOutput, 32> SPIRVShaderOutputs;
typedef MVKSmallVector<SPIRVShaderInterfaceVariable, 32> SPIRVShaderOutputs;
typedef MVKSmallVector<SPIRVShaderInterfaceVariable, 32> SPIRVShaderInputs;

id<MTLRenderPipelineState> getOrCompilePipeline(MTLRenderPipelineDescriptor* plDesc, id<MTLRenderPipelineState>& plState);
id<MTLComputePipelineState> getOrCompilePipeline(MTLComputePipelineDescriptor* plDesc, id<MTLComputePipelineState>& plState, const char* compilerType);
Expand All @@ -302,14 +303,15 @@ class MVKGraphicsPipeline : public MVKPipeline {
void initShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig, const VkGraphicsPipelineCreateInfo* pCreateInfo, const SPIRVTessReflectionData& reflectData);
void initReservedVertexAttributeBufferCount(const VkGraphicsPipelineCreateInfo* pCreateInfo);
void addVertexInputToShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig, const VkGraphicsPipelineCreateInfo* pCreateInfo);
void addNextStageInputToShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderInputs& inputs);
void addPrevStageOutputToShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderOutputs& outputs);
MTLRenderPipelineDescriptor* newMTLRenderPipelineDescriptor(const VkGraphicsPipelineCreateInfo* pCreateInfo, const SPIRVTessReflectionData& reflectData);
MTLComputePipelineDescriptor* newMTLTessVertexStageDescriptor(const VkGraphicsPipelineCreateInfo* pCreateInfo, const SPIRVTessReflectionData& reflectData, SPIRVToMSLConversionConfiguration& shaderConfig);
MTLComputePipelineDescriptor* newMTLTessControlStageDescriptor(const VkGraphicsPipelineCreateInfo* pCreateInfo, const SPIRVTessReflectionData& reflectData, SPIRVToMSLConversionConfiguration& shaderConfig);
MTLRenderPipelineDescriptor* newMTLTessRasterStageDescriptor(const VkGraphicsPipelineCreateInfo* pCreateInfo, const SPIRVTessReflectionData& reflectData, SPIRVToMSLConversionConfiguration& shaderConfig);
bool addVertexShaderToPipeline(MTLRenderPipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig);
bool addVertexShaderToPipeline(MTLComputePipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig);
bool addTessCtlShaderToPipeline(MTLComputePipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderOutputs& prevOutput);
bool addVertexShaderToPipeline(MTLComputePipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderInputs& nextInputs);
bool addTessCtlShaderToPipeline(MTLComputePipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderOutputs& prevOutput, SPIRVShaderInputs& nextInputs);
bool addTessEvalShaderToPipeline(MTLRenderPipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderOutputs& prevOutput);
bool addFragmentShaderToPipeline(MTLRenderPipelineDescriptor* plDesc, const VkGraphicsPipelineCreateInfo* pCreateInfo, SPIRVToMSLConversionConfiguration& shaderConfig, SPIRVShaderOutputs& prevOutput);
template<class T>
Expand Down
142 changes: 122 additions & 20 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,8 +669,21 @@
SPIRVToMSLConversionConfiguration& shaderConfig) {
MTLComputePipelineDescriptor* plDesc = [MTLComputePipelineDescriptor new]; // retained

SPIRVShaderInputs tcInputs;
std::string errorLog;
if (!getShaderInputs(((MVKShaderModule*)_pTessCtlSS->module)->getSPIRV(), spv::ExecutionModelTessellationControl, _pTessCtlSS->pName, tcInputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get tessellation control inputs: %s", errorLog.c_str()));
return nil;
}

// Filter out anything but builtins. We couldn't do this before because we needed to make sure
// locations were assigned correctly.
tcInputs.erase(std::remove_if(tcInputs.begin(), tcInputs.end(), [](const SPIRVShaderInterfaceVariable& var) {
return var.builtin != spv::BuiltInPosition && var.builtin != spv::BuiltInPointSize && var.builtin != spv::BuiltInClipDistance && var.builtin != spv::BuiltInCullDistance;
}), tcInputs.end());

// Add shader stages.
if (!addVertexShaderToPipeline(plDesc, pCreateInfo, shaderConfig)) { return nil; }
if (!addVertexShaderToPipeline(plDesc, pCreateInfo, shaderConfig, tcInputs)) { return nil; }

// Vertex input
plDesc.stageInputDescriptor = [MTLStageInputOutputDescriptor stageInputOutputDescriptor];
Expand Down Expand Up @@ -794,14 +807,25 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
MTLComputePipelineDescriptor* plDesc = [MTLComputePipelineDescriptor new]; // retained

SPIRVShaderOutputs vtxOutputs;
SPIRVShaderInputs teInputs;
std::string errorLog;
if (!getShaderOutputs(((MVKShaderModule*)_pVertexSS->module)->getSPIRV(), spv::ExecutionModelVertex, _pVertexSS->pName, vtxOutputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get vertex outputs: %s", errorLog.c_str()));
return nil;
}
if (!getShaderInputs(((MVKShaderModule*)_pTessEvalSS->module)->getSPIRV(), spv::ExecutionModelTessellationEvaluation, _pTessEvalSS->pName, teInputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get tessellation evaluation inputs: %s", errorLog.c_str()));
return nil;
}

// Filter out anything but builtins. We couldn't do this before because we needed to make sure
// locations were assigned correctly.
teInputs.erase(std::remove_if(teInputs.begin(), teInputs.end(), [](const SPIRVShaderInterfaceVariable& var) {
return var.builtin != spv::BuiltInPosition && var.builtin != spv::BuiltInPointSize && var.builtin != spv::BuiltInClipDistance && var.builtin != spv::BuiltInCullDistance;
}), teInputs.end());

// Add shader stages.
if (!addTessCtlShaderToPipeline(plDesc, pCreateInfo, shaderConfig, vtxOutputs)) {
if (!addTessCtlShaderToPipeline(plDesc, pCreateInfo, shaderConfig, vtxOutputs, teInputs)) {
[plDesc release];
return nil;
}
Expand All @@ -822,11 +846,16 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
MTLRenderPipelineDescriptor* plDesc = [MTLRenderPipelineDescriptor new]; // retained

SPIRVShaderOutputs tcOutputs, teOutputs;
SPIRVShaderInputs teInputs;
std::string errorLog;
if (!getShaderOutputs(((MVKShaderModule*)_pTessCtlSS->module)->getSPIRV(), spv::ExecutionModelTessellationControl, _pTessCtlSS->pName, tcOutputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get tessellation control outputs: %s", errorLog.c_str()));
return nil;
}
if (!getShaderInputs(((MVKShaderModule*)_pTessEvalSS->module)->getSPIRV(), spv::ExecutionModelTessellationEvaluation, _pTessEvalSS->pName, teInputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get tessellation evaluation inputs: %s", errorLog.c_str()));
return nil;
}
if (!getShaderOutputs(((MVKShaderModule*)_pTessEvalSS->module)->getSPIRV(), spv::ExecutionModelTessellationEvaluation, _pTessEvalSS->pName, teOutputs, errorLog) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Failed to get tessellation evaluation outputs: %s", errorLog.c_str()));
return nil;
Expand All @@ -840,13 +869,38 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3

// Tessellation evaluation stage input
// This needs to happen before compiling the fragment shader, or we'll lose information on shader inputs.
// First, add extra builtins that are in teInputs but not tcOutputs. They can be read
// even if not written.
teInputs.erase(std::remove_if(teInputs.begin(), teInputs.end(), [&tcOutputs](const SPIRVShaderInterfaceVariable& var) {
return var.builtin != spv::BuiltInPosition && var.builtin != spv::BuiltInPointSize && var.builtin != spv::BuiltInClipDistance && var.builtin != spv::BuiltInCullDistance;
}), teInputs.end());
std::remove_copy_if(teInputs.begin(), teInputs.end(), std::back_inserter(tcOutputs), [&tcOutputs](const SPIRVShaderInterfaceVariable& input) {
auto iter = std::find_if(tcOutputs.begin(), tcOutputs.end(), [input](const SPIRVShaderInterfaceVariable& oldVar) {
return oldVar.builtin == input.builtin;
});
if (iter != tcOutputs.end()) {
iter->isUsed = input.isUsed;
}
return iter != tcOutputs.end();
});

auto isBuiltInRead = [&teInputs](spv::BuiltIn builtin) {
for (const auto& input : teInputs) {
if (input.builtin == builtin) {
return input.isUsed;
}
}
return false;
};

plDesc.vertexDescriptor = [MTLVertexDescriptor vertexDescriptor];
uint32_t offset = 0, patchOffset = 0, outerLoc = -1, innerLoc = -1;
bool usedPerVertex = false, usedPerPatch = false;
const SPIRVShaderOutput* firstVertex = nullptr, * firstPatch = nullptr;
for (const SPIRVShaderOutput& output : tcOutputs) {
if (output.builtin == spv::BuiltInPointSize && !reflectData.pointMode) { continue; }
if (!shaderConfig.isShaderInputLocationUsed(output.location)) {
if ((output.builtin != spv::BuiltInMax && !isBuiltInRead(output.builtin)) &&
!shaderConfig.isShaderInputLocationUsed(output.location)) {
if (output.perPatch && !(output.builtin == spv::BuiltInTessLevelOuter || output.builtin == spv::BuiltInTessLevelInner) ) {
if (!firstPatch) { firstPatch = &output; }
patchOffset += getShaderOutputSize(output);
Expand Down Expand Up @@ -1014,7 +1068,8 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
// Adds a vertex shader compiled as a compute kernel to the pipeline description.
bool MVKGraphicsPipeline::addVertexShaderToPipeline(MTLComputePipelineDescriptor* plDesc,
const VkGraphicsPipelineCreateInfo* pCreateInfo,
SPIRVToMSLConversionConfiguration& shaderConfig) {
SPIRVToMSLConversionConfiguration& shaderConfig,
SPIRVShaderInputs& tcInputs) {
shaderConfig.options.entryPointStage = spv::ExecutionModelVertex;
shaderConfig.options.entryPointName = _pVertexSS->pName;
shaderConfig.options.mslOptions.swizzle_buffer_index = _swizzleBufferIndex.stages[kMVKShaderStageVertex];
Expand All @@ -1026,6 +1081,7 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
shaderConfig.options.mslOptions.vertex_for_tessellation = true;
shaderConfig.options.mslOptions.disable_rasterization = true;
addVertexInputToShaderConversionConfig(shaderConfig, pCreateInfo);
addNextStageInputToShaderConversionConfig(shaderConfig, tcInputs);

static const CompilerMSL::Options::IndexType indexTypes[] = {
CompilerMSL::Options::IndexType::None,
Expand Down Expand Up @@ -1078,7 +1134,8 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
bool MVKGraphicsPipeline::addTessCtlShaderToPipeline(MTLComputePipelineDescriptor* plDesc,
const VkGraphicsPipelineCreateInfo* pCreateInfo,
SPIRVToMSLConversionConfiguration& shaderConfig,
SPIRVShaderOutputs& vtxOutputs) {
SPIRVShaderOutputs& vtxOutputs,
SPIRVShaderInputs& teInputs) {
shaderConfig.options.entryPointStage = spv::ExecutionModelTessellationControl;
shaderConfig.options.entryPointName = _pTessCtlSS->pName;
shaderConfig.options.mslOptions.swizzle_buffer_index = _swizzleBufferIndex.stages[kMVKShaderStageTessCtl];
Expand All @@ -1093,6 +1150,7 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
shaderConfig.options.mslOptions.multi_patch_workgroup = true;
shaderConfig.options.mslOptions.fixed_subgroup_size = mvkIsAnyFlagEnabled(_pTessCtlSS->flags, VK_PIPELINE_SHADER_STAGE_CREATE_ALLOW_VARYING_SUBGROUP_SIZE_BIT_EXT) ? 0 : _device->_pMetalFeatures->maxSubgroupSize;
addPrevStageOutputToShaderConversionConfig(shaderConfig, vtxOutputs);
addNextStageInputToShaderConversionConfig(shaderConfig, teInputs);

MVKMTLFunction func = ((MVKShaderModule*)_pTessCtlSS->module)->getMTLFunction(&shaderConfig, _pTessCtlSS->pSpecializationInfo, _pipelineCache);
id<MTLFunction> mtlFunc = func.getMTLFunction();
Expand Down Expand Up @@ -1695,7 +1753,7 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3

// Set binding and offset from Vulkan vertex attribute
mvk::MSLShaderInput si;
si.shaderInput.location = pVKVA->location;
si.shaderVar.location = pVKVA->location;
si.binding = pVKVA->binding;

// Metal can't do signedness conversions on vertex buffers (rdar://45922847). If the shader
Expand All @@ -1705,11 +1763,11 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
// declared type. Programs that try to invoke undefined behavior are on their own.
switch (getPixelFormats()->getFormatType(pVKVA->format) ) {
case kMVKFormatColorUInt8:
si.shaderInput.format = MSL_VERTEX_FORMAT_UINT8;
si.shaderVar.format = MSL_VERTEX_FORMAT_UINT8;
break;

case kMVKFormatColorUInt16:
si.shaderInput.format = MSL_VERTEX_FORMAT_UINT16;
si.shaderVar.format = MSL_VERTEX_FORMAT_UINT16;
break;

case kMVKFormatDepthStencil:
Expand All @@ -1719,7 +1777,7 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
case VK_FORMAT_D16_UNORM_S8_UINT:
case VK_FORMAT_D24_UNORM_S8_UINT:
case VK_FORMAT_D32_SFLOAT_S8_UINT:
si.shaderInput.format = MSL_VERTEX_FORMAT_UINT8;
si.shaderVar.format = MSL_VERTEX_FORMAT_UINT8;
break;

default:
Expand All @@ -1736,6 +1794,49 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
}
}

// Initializes the shader outputs in a shader conversion config from the next stage input.
void MVKGraphicsPipeline::addNextStageInputToShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig,
SPIRVShaderInputs& shaderInputs) {
// Set the shader conversion configuration output variable information
shaderConfig.shaderOutputs.clear();
uint32_t soCnt = (uint32_t)shaderInputs.size();
for (uint32_t soIdx = 0; soIdx < soCnt; soIdx++) {
if (!shaderInputs[soIdx].isUsed) { continue; }

mvk::MSLShaderInterfaceVariable so;
so.shaderVar.location = shaderInputs[soIdx].location;
so.shaderVar.component = shaderInputs[soIdx].component;
so.shaderVar.builtin = shaderInputs[soIdx].builtin;
so.shaderVar.vecsize = shaderInputs[soIdx].vecWidth;

switch (getPixelFormats()->getFormatType(mvkFormatFromOutput(shaderInputs[soIdx]) ) ) {
case kMVKFormatColorUInt8:
so.shaderVar.format = MSL_SHADER_INPUT_FORMAT_UINT8;
break;

case kMVKFormatColorUInt16:
so.shaderVar.format = MSL_SHADER_INPUT_FORMAT_UINT16;
break;

case kMVKFormatColorHalf:
case kMVKFormatColorInt16:
so.shaderVar.format = MSL_SHADER_INPUT_FORMAT_ANY16;
break;

case kMVKFormatColorFloat:
case kMVKFormatColorInt32:
case kMVKFormatColorUInt32:
so.shaderVar.format = MSL_SHADER_INPUT_FORMAT_ANY32;
break;

default:
break;
}

shaderConfig.shaderOutputs.push_back(so);
}
}

// Initializes the shader inputs in a shader conversion config from the previous stage output.
void MVKGraphicsPipeline::addPrevStageOutputToShaderConversionConfig(SPIRVToMSLConversionConfiguration& shaderConfig,
SPIRVShaderOutputs& shaderOutputs) {
Expand All @@ -1746,29 +1847,29 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
if (!shaderOutputs[siIdx].isUsed) { continue; }

mvk::MSLShaderInput si;
si.shaderInput.location = shaderOutputs[siIdx].location;
si.shaderInput.component = shaderOutputs[siIdx].component;
si.shaderInput.builtin = shaderOutputs[siIdx].builtin;
si.shaderInput.vecsize = shaderOutputs[siIdx].vecWidth;
si.shaderVar.location = shaderOutputs[siIdx].location;
si.shaderVar.component = shaderOutputs[siIdx].component;
si.shaderVar.builtin = shaderOutputs[siIdx].builtin;
si.shaderVar.vecsize = shaderOutputs[siIdx].vecWidth;

switch (getPixelFormats()->getFormatType(mvkFormatFromOutput(shaderOutputs[siIdx]) ) ) {
case kMVKFormatColorUInt8:
si.shaderInput.format = MSL_SHADER_INPUT_FORMAT_UINT8;
si.shaderVar.format = MSL_SHADER_INPUT_FORMAT_UINT8;
break;

case kMVKFormatColorUInt16:
si.shaderInput.format = MSL_SHADER_INPUT_FORMAT_UINT16;
si.shaderVar.format = MSL_SHADER_INPUT_FORMAT_UINT16;
break;

case kMVKFormatColorHalf:
case kMVKFormatColorInt16:
si.shaderInput.format = MSL_SHADER_INPUT_FORMAT_ANY16;
si.shaderVar.format = MSL_SHADER_INPUT_FORMAT_ANY16;
break;

case kMVKFormatColorFloat:
case kMVKFormatColorInt32:
case kMVKFormatColorUInt32:
si.shaderInput.format = MSL_SHADER_INPUT_FORMAT_ANY32;
si.shaderVar.format = MSL_SHADER_INPUT_FORMAT_ANY32;
break;

default:
Expand Down Expand Up @@ -2251,7 +2352,7 @@ void serialize(Archive & archive, CompilerMSL::Options& opt) {
}

template<class Archive>
void serialize(Archive & archive, MSLShaderInput& si) {
void serialize(Archive & archive, MSLShaderInterfaceVariable& si) {
archive(si.location,
si.component,
si.format,
Expand Down Expand Up @@ -2331,8 +2432,8 @@ void serialize(Archive & archive, SPIRVToMSLConversionOptions& opt) {
}

template<class Archive>
void serialize(Archive & archive, MSLShaderInput& si) {
archive(si.shaderInput,
void serialize(Archive & archive, MSLShaderInterfaceVariable& si) {
archive(si.shaderVar,
si.binding,
si.outIsUsedByShader);
}
Expand All @@ -2357,6 +2458,7 @@ void serialize(Archive & archive, DescriptorBinding& db) {
void serialize(Archive & archive, SPIRVToMSLConversionConfiguration& ctx) {
archive(ctx.options,
ctx.shaderInputs,
ctx.shaderOutputs,
ctx.resourceBindings,
ctx.discreteDescriptorSets);
}
Expand Down
2 changes: 1 addition & 1 deletion MoltenVK/MoltenVK/GPUObjects/MVKShaderModule.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ static uint32_t getWorkgroupDimensionSize(const SPIRVWorkgroupSizeDimension& wgD
_device->addActivityPerformance(_device->_performanceStatistics.shaderCompilation.shaderLibraryFromCache, startTime);
} else {
mvkLib->setEntryPointName(pShaderConfig->options.entryPointName);
pShaderConfig->markAllInputsAndResourcesUsed();
pShaderConfig->markAllInterfaceVarsAndResourcesUsed();
}

return mvkLib ? mvkLib->getMTLFunction(pSpecializationInfo, this) : MVKMTLFunctionNull;
Expand Down
Loading

0 comments on commit 250e1f9

Please sign in to comment.