Split Vulkan command buffers on compute pass
This is a further generalization of the workaround added for
dawn:1564. It splits the command buffer on Qualcomm GPUs any time
a compute pass follows a render pass in a single command buffer.
This feels like a farily heavy-handed workaround, but where
previously it seemed like the crash only happened if the compute
pass used attachements from the render pass, I'm now seeing some
scenarios where the compute pass shares no resources with the
render pass and still fails unless the command buffer is split.
Fortunately the broadening of scope allows the code to be simpler
and removes the need to track and check render pass attachements.
Bug: dawn:1897
Change-Id: I2c511a22e745ee5804d46337cb52483c009a200e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/144780
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index a7091b4..597a187 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -319,12 +319,11 @@
"This toggle is enabled by default on Metal backend where GPU counters cannot be stored to"
"sampleBufferAttachments on empty blit encoder.",
"https://crbug.com/dawn/1473", ToggleStage::Device}},
- {Toggle::VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass,
- {"vulkan_split_command_buffer_on_depth_stencil_compute_sample_after_render_pass",
- "Splits any command buffer that samples a depth/stencil texture in a compute pass after that "
- "texture was used as an attachment for a prior render pass. This toggle is enabled by "
- "default on Qualcomm GPUs, which have been observed experiencing a driver crash in this "
- "situation.",
+ {Toggle::VulkanSplitCommandBufferOnComputePassAfterRenderPass,
+ {"vulkan_split_command_buffer_on_compute_pass_after_render_pass",
+ "Splits any command buffer where a compute pass is recorded after a render pass. This "
+ "toggle is enabled by default on Qualcomm GPUs, which have been observed experiencing a "
+ "driver crash in this situation.",
"https://crbug.com/dawn/1564", ToggleStage::Device}},
{Toggle::DisableSubAllocationFor2DTextureWithCopyDstOrRenderAttachment,
{"disable_sub_allocation_for_2d_texture_with_copy_dst_or_render_attachment",
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h
index f7d88dc..0dd44b3 100644
--- a/src/dawn/native/Toggles.h
+++ b/src/dawn/native/Toggles.h
@@ -81,7 +81,7 @@
D3D12UseTempBufferInTextureToTextureCopyBetweenDifferentDimensions,
ApplyClearBigIntegerColorValueWithDraw,
MetalUseMockBlitEncoderForWriteTimestamp,
- VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass,
+ VulkanSplitCommandBufferOnComputePassAfterRenderPass,
DisableSubAllocationFor2DTextureWithCopyDstOrRenderAttachment,
MetalUseCombinedDepthStencilFormatForStencil8,
MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats,
diff --git a/src/dawn/native/vulkan/CommandBufferVk.cpp b/src/dawn/native/vulkan/CommandBufferVk.cpp
index 3b79742..a822772 100644
--- a/src/dawn/native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn/native/vulkan/CommandBufferVk.cpp
@@ -525,6 +525,10 @@
size_t nextComputePassNumber = 0;
size_t nextRenderPassNumber = 0;
+ // Need to track if a render pass has already been recorded for the
+ // VulkanSplitCommandBufferOnComputePassAfterRenderPass workaround.
+ bool hasRecordedRenderPassInCurrentCommandBuffer = false;
+
Command type;
while (mCommands.NextCommandId(&type)) {
switch (type) {
@@ -736,6 +740,7 @@
LazyClearRenderPassAttachments(cmd);
DAWN_TRY(RecordRenderPass(recordingContext, cmd));
+ hasRecordedRenderPassInCurrentCommandBuffer = true;
nextRenderPassNumber++;
break;
}
@@ -743,6 +748,17 @@
case Command::BeginComputePass: {
BeginComputePassCmd* cmd = mCommands.NextCommand<BeginComputePassCmd>();
+ // If required, split the command buffer any time a compute pass follows a render
+ // pass to work around a Qualcomm bug.
+ if (hasRecordedRenderPassInCurrentCommandBuffer &&
+ device->IsToggleEnabled(
+ Toggle::VulkanSplitCommandBufferOnComputePassAfterRenderPass)) {
+ // Identified a potential crash case, split the command buffer.
+ DAWN_TRY(device->SplitRecordingContext(recordingContext));
+ hasRecordedRenderPassInCurrentCommandBuffer = false;
+ commands = recordingContext->commandBuffer;
+ }
+
DAWN_TRY(
RecordComputePass(recordingContext, cmd,
GetResourceUsages().computePasses[nextComputePassNumber]));
@@ -889,24 +905,6 @@
const ComputePassResourceUsage& resourceUsages) {
Device* device = ToBackend(GetDevice());
- // If required, split the command buffer any time we detect a dpeth/stencil attachment is
- // used in a compute pass after being used as a render pass attachment in the same command
- // buffer.
- if (device->IsToggleEnabled(
- Toggle::VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass) &&
- !mRenderPassDepthStencilAttachments.empty()) {
- for (auto texture : resourceUsages.referencedTextures) {
- if (texture->GetFormat().HasDepthOrStencil() &&
- mRenderPassDepthStencilAttachments.find(texture) !=
- mRenderPassDepthStencilAttachments.end()) {
- // Identified a potential crash case, split the command buffer.
- DAWN_TRY(device->SplitRecordingContext(recordingContext));
- mRenderPassDepthStencilAttachments.clear();
- break;
- }
- }
- }
-
// Write timestamp at the beginning of compute pass if it's set
if (computePassCmd->beginTimestamp.querySet.Get() != nullptr) {
RecordWriteTimestampCmd(recordingContext, device,
@@ -1058,14 +1056,6 @@
DAWN_TRY(RecordBeginRenderPass(recordingContext, device, renderPassCmd));
- // If required, track depth/stencil textures used as render pass attachments.
- if (device->IsToggleEnabled(
- Toggle::VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass) &&
- renderPassCmd->attachmentState->HasDepthStencilAttachment()) {
- mRenderPassDepthStencilAttachments.insert(
- renderPassCmd->depthStencilAttachment.view->GetTexture());
- }
-
// Write timestamp at the beginning of render pass if it's set.
if (renderPassCmd->beginTimestamp.querySet.Get() != nullptr) {
RecordWriteTimestampCmd(recordingContext, device,
diff --git a/src/dawn/native/vulkan/CommandBufferVk.h b/src/dawn/native/vulkan/CommandBufferVk.h
index 322d17f..71e6af4 100644
--- a/src/dawn/native/vulkan/CommandBufferVk.h
+++ b/src/dawn/native/vulkan/CommandBufferVk.h
@@ -52,10 +52,6 @@
const TextureCopy& srcCopy,
const TextureCopy& dstCopy,
const Extent3D& copySize);
-
- // Need to track depth/stencil textures used by render passes if the
- // VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass toggle is enabled.
- std::set<TextureBase*> mRenderPassDepthStencilAttachments;
};
} // namespace dawn::native::vulkan
diff --git a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
index 0045790..ddcb641 100644
--- a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
+++ b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
@@ -470,11 +470,11 @@
deviceToggles->Default(Toggle::UseTemporaryBufferInCompressedTextureToTextureCopy, true);
if (IsAndroidQualcomm()) {
- // dawn:1564: Clearing a depth/stencil buffer in a render pass and then sampling it in a
- // compute pass in the same command buffer causes a crash on Qualcomm GPUs. To work around
- // that bug, split the command buffer any time we can detect that situation.
- deviceToggles->Default(
- Toggle::VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass, true);
+ // dawn:1564, dawn:1897: Recording a compute pass after a render pass in the same command
+ // buffer frequently causes a crash on Qualcomm GPUs. To work around that bug, split the
+ // command buffer any time we are about to record a compute pass when a render pass has
+ // already been recorded.
+ deviceToggles->Default(Toggle::VulkanSplitCommandBufferOnComputePassAfterRenderPass, true);
// dawn:1569: Qualcomm devices have a bug resolving into a non-zero level of an array
// texture. Work around it by resolving into a single level texture and then copying into
diff --git a/src/dawn/tests/end2end/GpuMemorySynchronizationTests.cpp b/src/dawn/tests/end2end/GpuMemorySynchronizationTests.cpp
index 86cc4fa..7299198 100644
--- a/src/dawn/tests/end2end/GpuMemorySynchronizationTests.cpp
+++ b/src/dawn/tests/end2end/GpuMemorySynchronizationTests.cpp
@@ -216,13 +216,15 @@
EXPECT_PIXEL_RGBA8_EQ(utils::RGBA8(2, 0, 0, 255), renderPass.color, 0, 0);
}
-DAWN_INSTANTIATE_TEST(GpuMemorySyncTests,
- D3D11Backend(),
- D3D12Backend(),
- MetalBackend(),
- OpenGLBackend(),
- OpenGLESBackend(),
- VulkanBackend());
+DAWN_INSTANTIATE_TEST(
+ GpuMemorySyncTests,
+ D3D11Backend(),
+ D3D12Backend(),
+ MetalBackend(),
+ OpenGLBackend(),
+ OpenGLESBackend(),
+ VulkanBackend(),
+ VulkanBackend({"vulkan_split_command_buffer_on_compute_pass_after_render_pass"}));
class StorageToUniformSyncTests : public DawnTest {
protected:
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index 1e17cd5..8bf5058 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -550,12 +550,6 @@
crbug.com/dawn/1896 [ android-r ] webgpu:api,operation,memory_sync,texture,same_subresource:ww:boundary="command-buffer";first={"op":"attachment-store","in":"command-encoder"};second={"op":"storage","in":"compute-pass-encoder"} [ Failure ]
################################################################################
-# Dispatch/draw indirect causes device lost on Android Qualcomm (Pixel 4)
-################################################################################
-crbug.com/dawn/1897 [ android-r ] webgpu:api,operation,resource_init,buffer:indirect_buffer_for_dispatch_indirect: [ Failure ]
-crbug.com/dawn/1897 [ android-r ] webgpu:api,operation,resource_init,buffer:indirect_buffer_for_draw_indirect:* [ Failure ]
-
-################################################################################
# Primitive restart with indexFormat=uint32 causes device lost on Android Arm (Pixel 6)
################################################################################
crbug.com/dawn/1898 [ android-t ] webgpu:api,operation,vertex_state,index_format:primitive_restart:indexFormat="uint32";primitiveTopology="line-list" [ Failure ]
