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.

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

Apply SPIRV-Cross workarounds for Metal bugs #2248

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ class MVKGraphicsPipeline : public MVKPipeline {
bool _isRasterizingColor = false;
bool _sampleLocationsEnable = false;
bool _isTessellationPipeline = false;
bool _inputAttachmentIsDSAttachment = false;
};


Expand Down
14 changes: 14 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,16 @@
_outputControlPointCount = reflectData.numControlPoints;
mvkSetOrClear(&_tessInfo, _isTessellationPipeline ? pCreateInfo->pTessellationState : nullptr);

// Handles depth attachment being used as input attachment. However, it does not solve the issue when
// the pipeline is created without render pass (dynamic rendering) since we won't be able to know
// which resources will be used when rendering. Needs to be done before we do shaders
// Potential solution would be to generate 2 pipelines, one with the workaround for the Metal issue
// and one without it, and decide at bind time once we know the resources which one to use.
billhollings marked this conversation as resolved.
Show resolved Hide resolved
if (pCreateInfo->renderPass) {
MVKRenderSubpass* subpass = ((MVKRenderPass*)pCreateInfo->renderPass)->getSubpass(pCreateInfo->subpass);
_inputAttachmentIsDSAttachment = subpass->isInputAttachmentDepthStencilAttachment();
}

// Render pipeline state. Do this as early as possible, to fail fast if pipeline requires a fail on cache-miss.
initMTLRenderPipelineState(pCreateInfo, reflectData, pPipelineFB, pVertexSS, pVertexFB, pTessCtlSS, pTessCtlFB, pTessEvalSS, pTessEvalFB, pFragmentSS, pFragmentFB);
if ( !_hasValidMTLPipelineStates ) { return; }
Expand Down Expand Up @@ -1332,6 +1342,8 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
shaderConfig.options.mslOptions.fixed_subgroup_size = mvkIsAnyFlagEnabled(pFragmentSS->flags, VK_PIPELINE_SHADER_STAGE_CREATE_ALLOW_VARYING_SUBGROUP_SIZE_BIT_EXT) ? 0 : mtlFeats.maxSubgroupSize;
shaderConfig.options.mslOptions.check_discarded_frag_stores = true;
shaderConfig.options.mslOptions.force_fragment_with_side_effects_execution = true;
shaderConfig.options.mslOptions.input_attachment_is_ds_attachment = _inputAttachmentIsDSAttachment;
if (mtlFeats.needsSampleDrefLodArrayWorkaround) {
shaderConfig.options.mslOptions.sample_dref_lod_array_as_grad = true;
}
Expand Down Expand Up @@ -2580,6 +2592,8 @@ void serialize(Archive & archive, CompilerMSL::Options& opt) {
opt.force_sample_rate_shading,
opt.manual_helper_invocation_updates,
opt.check_discarded_frag_stores,
opt.force_fragment_with_side_effects_execution,
opt.input_attachment_is_ds_attachment,
opt.sample_dref_lod_array_as_grad,
opt.replace_recursive_inputs,
opt.agx_manual_cube_grad_fixup);
Expand Down
8 changes: 6 additions & 2 deletions MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ class MVKRenderSubpass : public MVKBaseObject {
bool isColorAttachmentAlsoInputAttachment(uint32_t colorAttIdx);

/** Returns whether or not the depth attachment is being used. */
bool isDepthAttachmentUsed() { return _depthAttachment.attachment != VK_ATTACHMENT_UNUSED; }
bool isDepthAttachmentUsed() const { return _depthAttachment.attachment != VK_ATTACHMENT_UNUSED; }

/** Returns whether or not the stencil attachment is being used. */
bool isStencilAttachmentUsed() { return _stencilAttachment.attachment != VK_ATTACHMENT_UNUSED; }
bool isStencilAttachmentUsed() const { return _stencilAttachment.attachment != VK_ATTACHMENT_UNUSED; }

/** Return the depth attachment format. */
VkFormat getDepthFormat();
Expand All @@ -90,6 +90,9 @@ class MVKRenderSubpass : public MVKBaseObject {
/** Returns whether or not this is a multiview subpass. */
bool isMultiview() const { return _pipelineRenderingCreateInfo.viewMask != 0; }

/** Returns whether or not the depth stencil is being used as input attachment */
bool isInputAttachmentDepthStencilAttachment() const { return _isInputAttachmentDepthStencilAttachment; }

/** Returns the multiview view mask. */
uint32_t getViewMask() const { return _pipelineRenderingCreateInfo.viewMask; }

Expand Down Expand Up @@ -177,6 +180,7 @@ class MVKRenderSubpass : public MVKBaseObject {
VkResolveModeFlagBits _stencilResolveMode = VK_RESOLVE_MODE_NONE;
VkSampleCountFlagBits _defaultSampleCount = VK_SAMPLE_COUNT_1_BIT;
uint32_t _subpassIndex;
bool _isInputAttachmentDepthStencilAttachment = false;
};


Expand Down
15 changes: 15 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,21 @@
_pipelineRenderingCreateInfo.pColorAttachmentFormats = _colorAttachmentFormats.data();
_pipelineRenderingCreateInfo.depthAttachmentFormat = getDepthFormat();
_pipelineRenderingCreateInfo.stencilAttachmentFormat = getStencilFormat();

// Needed to understand if we need to force the depth/stencil write to post fragment execution
// since Metal may try to do the write pre fragment exeuction which is against Vulkan
bool depthAttachmentUsed = isDepthAttachmentUsed();
bool stencilAttachmentUsed = isStencilAttachmentUsed();
for (uint32_t i = 0u; i < _inputAttachments.size(); ++i) {
bool isDepthInput = depthAttachmentUsed && (_inputAttachments[i].attachment == _depthAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _depthAttachment.aspectMask);
bool isStencilInput = stencilAttachmentUsed && (_inputAttachments[i].attachment == _stencilAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _stencilAttachment.aspectMask);
if (isDepthInput || isStencilInput) {
_isInputAttachmentDepthStencilAttachment = true;
break;
}
}
}

static const VkAttachmentReference2 _unusedAttachment = {VK_STRUCTURE_TYPE_ATTACHMENT_REFERENCE_2, nullptr, VK_ATTACHMENT_UNUSED, VK_IMAGE_LAYOUT_UNDEFINED, 0};
Expand Down
Loading