From cca2dbb6889271568617c309a3c7f3a336f98cff Mon Sep 17 00:00:00 2001 From: Jeremy Gebben Date: Wed, 9 Dec 2020 13:38:13 -0700 Subject: [PATCH] Fix end of Renderpass synchronization hazards The implicit subpass dependency at the end of a renderpass has dstStageMask set to VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, which prevents forming a dependency chain with specific pipeline stages. To synchronize commands with vkCmdEndRenderPass, use either an explict external subpass dependency or a pipeline barrier with srcStageMask set to VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT. VK-GL-CTS Issue: 2695 Affected tests: dEQP-VK.pipeline.multisample.sample_locations_ext.* dEQP-VK.renderpass.suballocation.attachment_sparse_filling.* dEQP-VK.renderpass.suballocation.subpass_dependencies.* dEQP-VK.renderpass2.suballocation.attachment_sparse_filling.* dEQP-VK.renderpass2.suballocation.subpass_dependencies.* Components: Vulkan Change-Id: I1b33c128dfdc54705f953f044db9acf1ff6e63e9 --- ...tPipelineMultisampleSampleLocationsExtTests.cpp | 58 ++++------------------ .../vktRenderPassSubpassDependencyTests.cpp | 24 ++++++--- ...enderPassUnusedAttachmentSparseFillingTests.cpp | 18 ++++++- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/external/vulkancts/modules/vulkan/pipeline/vktPipelineMultisampleSampleLocationsExtTests.cpp b/external/vulkancts/modules/vulkan/pipeline/vktPipelineMultisampleSampleLocationsExtTests.cpp index 421f367..12687fe 100644 --- a/external/vulkancts/modules/vulkan/pipeline/vktPipelineMultisampleSampleLocationsExtTests.cpp +++ b/external/vulkancts/modules/vulkan/pipeline/vktPipelineMultisampleSampleLocationsExtTests.cpp @@ -814,6 +814,17 @@ public: subpassDependencies.push_back(dependency); } } + // add a final dependency to synchronize results for the copy commands that will follow the renderpass + const VkSubpassDependency finalDependency = { + numSubpasses - 1, // uint32_t srcSubpass; + VK_SUBPASS_EXTERNAL, // uint32_t dstSubpass; + VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, // VkPipelineStageFlags srcStageMask; + VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask; + VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask; + VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask; + (VkDependencyFlags)0, // VkDependencyFlags dependencyFlags; + }; + subpassDependencies.push_back(finalDependency); const VkRenderPassCreateInfo renderPassInfo = { @@ -1535,16 +1546,6 @@ protected: vk.cmdDraw(*cmdBuffer, m_numVertices, 1u, 0u, 0u); endRenderPass(vk, *cmdBuffer); - // Resolve image -> host buffer - recordImageBarrier(vk, *cmdBuffer, *m_resolveImage, - VK_IMAGE_ASPECT_COLOR_BIT, // VkImageAspectFlags aspect, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout oldLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); // VkImageLayout newLayout) - recordCopyImageToBuffer(vk, *cmdBuffer, m_renderSize, *m_resolveImage, *m_colorBuffer); endCommandBuffer(vk, *cmdBuffer); @@ -2222,16 +2223,6 @@ protected: endRenderPass(vk, *cmdBuffer); - // Resolve image -> host buffer - recordImageBarrier(vk, *cmdBuffer, *m_resolveImage, - VK_IMAGE_ASPECT_COLOR_BIT, // VkImageAspectFlags aspect, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout oldLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); // VkImageLayout newLayout) - recordCopyImageToBuffer(vk, *cmdBuffer, m_renderSize, *m_resolveImage, *m_colorBuffer); endCommandBuffer(vk, *cmdBuffer); @@ -2567,15 +2558,6 @@ protected: } // Resolve image -> host buffer - recordImageBarrier(vk, currentCmdBuffer, *m_resolveImage, - VK_IMAGE_ASPECT_COLOR_BIT, // VkImageAspectFlags aspect, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout oldLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); // VkImageLayout newLayout) - recordCopyImageToBuffer(vk, currentCmdBuffer, m_renderSize, *m_resolveImage, *m_colorBuffer); endCommandBuffer(vk, currentCmdBuffer); @@ -2808,15 +2790,6 @@ protected: endRenderPass(vk, *cmdBuffer); // Resolve image -> host buffer - recordImageBarrier(vk, *cmdBuffer, *m_resolveImage, - VK_IMAGE_ASPECT_COLOR_BIT, // VkImageAspectFlags aspect, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout oldLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); // VkImageLayout newLayout) - recordCopyImageToBuffer(vk, *cmdBuffer, m_renderSize, *m_resolveImage, *m_colorBuffer); endCommandBuffer(vk, *cmdBuffer); @@ -2963,15 +2936,6 @@ protected: endRenderPass(vk, *cmdBuffer); // Resolve image -> host buffer - recordImageBarrier(vk, *cmdBuffer, *m_resolveImage, - VK_IMAGE_ASPECT_COLOR_BIT, // VkImageAspectFlags aspect, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT, // VkPipelineStageFlags dstStageMask, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout oldLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); // VkImageLayout newLayout) - recordCopyImageToBuffer(vk, *cmdBuffer, m_renderSize, *m_resolveImage, *m_colorBuffer); endCommandBuffer(vk, *cmdBuffer); diff --git a/external/vulkancts/modules/vulkan/renderpass/vktRenderPassSubpassDependencyTests.cpp b/external/vulkancts/modules/vulkan/renderpass/vktRenderPassSubpassDependencyTests.cpp index e4e58e2..774156d 100644 --- a/external/vulkancts/modules/vulkan/renderpass/vktRenderPassSubpassDependencyTests.cpp +++ b/external/vulkancts/modules/vulkan/renderpass/vktRenderPassSubpassDependencyTests.cpp @@ -851,7 +851,7 @@ tcu::TestStatus ExternalDependencyTestInstance::iterateInternal (void) { VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, // VkStructureType sType DE_NULL, // const void* pNext - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask + 0, // VkAccessFlags srcAccessMask VK_ACCESS_TRANSFER_READ_BIT, // VkAccessFlags dstAccessMask VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, // VkImageLayout oldLayout VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, // VkImageLayout newLayout @@ -860,8 +860,11 @@ tcu::TestStatus ExternalDependencyTestInstance::iterateInternal (void) **m_images[m_renderPasses.size() - 1], // VkImage image imageSubresourceRange // VkImageSubresourceRange subresourceRange }; - - vkd.cmdPipelineBarrier(*commandBuffer, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, 0u, 0u, DE_NULL, 0u, DE_NULL, 1u, &barrier); + // Since the implicit 'end' subpass dependency has VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT in its dstStageMask, + // we can't form an execution dependency chain with a specific pipeline stage. The cases that provide an explict + // 'end' subpass dependency could use a specific pipline stage, but there isn't a way to distinguish between the + // implicit and explicit cases here. + vkd.cmdPipelineBarrier(*commandBuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, 0u, 0u, DE_NULL, 0u, DE_NULL, 1u, &barrier); } // Copy image memory to buffer @@ -3989,10 +3992,10 @@ void initTests (tcu::TestCaseGroup* group, const RenderPassType renderPassType) { deps.push_back(SubpassDependency(VK_SUBPASS_EXTERNAL, // deUint32 srcPass 0, // deUint32 dstPass - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, // VkPipelineStageFlags srcStageMask - VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, // VkPipelineStageFlags dstStageMask - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask - VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT, // VkAccessFlags dstAccessMask + VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, // VkPipelineStageFlags srcStageMask + VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, // VkPipelineStageFlags dstStageMask + 0, // VkAccessFlags srcAccessMask + VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT, // VkAccessFlags dstAccessMask 0)); // VkDependencyFlags flags } @@ -4098,6 +4101,13 @@ void initTests (tcu::TestCaseGroup* group, const RenderPassType renderPassType) VK_DEPENDENCY_BY_REGION_BIT)); // VkDependencyFlags flags } } + deps.push_back(SubpassDependency((deUint32)subpassCount - 1, // deUint32 srcPass + VK_SUBPASS_EXTERNAL, // deUint32 dstPass + VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, // VkPipelineStageFlags srcStageMask + VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, // VkPipelineStageFlags dstStageMask + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT, // VkAccessFlags srcAccessMask + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT, // VkAccessFlags dstAccessMask + VK_DEPENDENCY_BY_REGION_BIT)); // VkDependencyFlags flags const RenderPass renderPass (attachments, subpasses, deps); const SubpassTestConfig testConfig (formats[formatNdx], renderSizes[renderSizeNdx], renderPass, renderPassType); diff --git a/external/vulkancts/modules/vulkan/renderpass/vktRenderPassUnusedAttachmentSparseFillingTests.cpp b/external/vulkancts/modules/vulkan/renderpass/vktRenderPassUnusedAttachmentSparseFillingTests.cpp index 24a4227..abef672 100644 --- a/external/vulkancts/modules/vulkan/renderpass/vktRenderPassUnusedAttachmentSparseFillingTests.cpp +++ b/external/vulkancts/modules/vulkan/renderpass/vktRenderPassUnusedAttachmentSparseFillingTests.cpp @@ -717,6 +717,20 @@ Move InputAttachmentSparseFillingTestInstance::createRenderPass (c DE_NULL // const deUint32* pPreserveAttachments ), }; + std::vector subpassDependencies = + { + SubpassDep ( + DE_NULL, + 0u, // deUint32 srcPass + VK_SUBPASS_EXTERNAL, // deUint32 dstPass + VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, // VkPipelineStageFlags srcStageMask + VK_PIPELINE_STAGE_DRAW_INDIRECT_BIT, // VkPipelineStageFlags dstStageMask + VK_ACCESS_SHADER_WRITE_BIT, // VkAccessFlags srcAccessMask + VK_ACCESS_INDIRECT_COMMAND_READ_BIT, // VkAccessFlags dstAccessMask + 0, // VkDependencyFlags flags + 0 // deInt32 viewOffset + ), + }; const RenderPassCreateInfo renderPassInfo ( DE_NULL, // const void* pNext @@ -725,8 +739,8 @@ Move InputAttachmentSparseFillingTestInstance::createRenderPass (c attachmentDescriptions.data(), // const VkAttachmentDescription* pAttachments static_cast(subpassDescriptions.size()), // deUint32 subpassCount subpassDescriptions.data(), // const VkSubpassDescription* pSubpasses - 0u, // deUint32 dependencyCount - DE_NULL, // const VkSubpassDependency* pDependencies + static_cast(subpassDependencies.size()), // deUint32 dependencyCount + subpassDependencies.data(), // const VkSubpassDependency* pDependencies 0u, // deUint32 correlatedViewMaskCount DE_NULL // const deUint32* pCorrelatedViewMasks ); -- 2.7.4