Vulkan: Accumulate Buffer barriers.

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -252,31 +252,40 @@ angle::Result BufferVk::copySubData(const gl::Context *context,
 {
     ASSERT(mBuffer && mBuffer->valid());
 
-    ContextVk *contextVk = vk::GetImpl(context);
-    auto *sourceBuffer   = GetAs<BufferVk>(source);
-    ASSERT(sourceBuffer->getBuffer().valid());
+    ContextVk *contextVk           = vk::GetImpl(context);
+    BufferVk *sourceVk             = GetAs<BufferVk>(source);
+    vk::BufferHelper &sourceBuffer = sourceVk->getBuffer();
+    ASSERT(sourceBuffer.valid());
 
     // If the shadow buffer is enabled for the destination buffer then
     // we need to update that as well. This will require us to complete
     // all recorded and in-flight commands involving the source buffer.
     if (mShadowBuffer.valid())
     {
-        ANGLE_TRY(sourceBuffer->getBuffer().waitForIdle(contextVk));
+        ANGLE_TRY(sourceBuffer.waitForIdle(contextVk));
 
         // Update the shadow buffer
         uint8_t *srcPtr;
-        ANGLE_TRY(sourceBuffer->getBuffer().mapWithOffset(contextVk, &srcPtr, sourceOffset));
+        ANGLE_TRY(sourceBuffer.mapWithOffset(contextVk, &srcPtr, sourceOffset));
 
         updateShadowBuffer(srcPtr, size, destOffset);
 
         // Unmap the source buffer
-        sourceBuffer->getBuffer().unmap(contextVk->getRenderer());
+        sourceBuffer.unmap(contextVk->getRenderer());
     }
 
     vk::CommandBuffer *commandBuffer = nullptr;
 
-    ANGLE_TRY(contextVk->onBufferTransferRead(&sourceBuffer->getBuffer()));
-    ANGLE_TRY(contextVk->onBufferTransferWrite(mBuffer));
+    // Check for self-dependency.
+    if (sourceBuffer.getBufferSerial() == mBuffer->getBufferSerial())
+    {
+        ANGLE_TRY(contextVk->onBufferSelfCopy(mBuffer));
+    }
+    else
+    {
+        ANGLE_TRY(contextVk->onBufferTransferRead(&sourceBuffer));
+        ANGLE_TRY(contextVk->onBufferTransferWrite(mBuffer));
+    }
     ANGLE_TRY(contextVk->endRenderPassAndGetCommandBuffer(&commandBuffer));
 
     // Enqueue a copy command on the GPU.
@@ -284,8 +293,7 @@ angle::Result BufferVk::copySubData(const gl::Context *context,
                                      static_cast<VkDeviceSize>(destOffset),
                                      static_cast<VkDeviceSize>(size)};
 
-    commandBuffer->copyBuffer(sourceBuffer->getBuffer().getBuffer(), mBuffer->getBuffer(), 1,
-                              &copyRegion);
+    commandBuffer->copyBuffer(sourceBuffer.getBuffer(), mBuffer->getBuffer(), 1, &copyRegion);
 
     // The new destination buffer data may require a conversion for the next draw, so mark it dirty.
     onDataChanged();

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -1521,7 +1521,8 @@ angle::Result ContextVk::handleDirtyGraphicsTransformFeedbackBuffersEmulation(
         vk::BufferHelper *bufferHelper = bufferHelpers[bufferIndex];
         ASSERT(bufferHelper);
         mRenderPassCommands->bufferWrite(&mResourceUseList, VK_ACCESS_SHADER_WRITE_BIT,
-                                         vk::PipelineStage::VertexShader, bufferHelper);
+                                         vk::PipelineStage::VertexShader,
+                                         vk::BufferAliasingMode::Disallowed, bufferHelper);
     }
 
     // TODO(http://anglebug.com/3570): Need to update to handle Program Pipelines
@@ -1556,9 +1557,9 @@ angle::Result ContextVk::handleDirtyGraphicsTransformFeedbackBuffersExtension(
     {
         vk::BufferHelper *bufferHelper = bufferHelpers[bufferIndex];
         ASSERT(bufferHelper);
-        mRenderPassCommands->bufferWrite(&mResourceUseList,
-                                         VK_ACCESS_TRANSFORM_FEEDBACK_WRITE_BIT_EXT,
-                                         vk::PipelineStage::TransformFeedback, bufferHelper);
+        mRenderPassCommands->bufferWrite(
+            &mResourceUseList, VK_ACCESS_TRANSFORM_FEEDBACK_WRITE_BIT_EXT,
+            vk::PipelineStage::TransformFeedback, vk::BufferAliasingMode::Disallowed, bufferHelper);
     }
 
     const gl::TransformFeedbackBuffersArray<VkBuffer> &bufferHandles =
@@ -3385,7 +3386,9 @@ angle::Result ContextVk::onBeginTransformFeedback(
 
     for (size_t bufferIndex = 0; bufferIndex < bufferCount; ++bufferIndex)
     {
-        if (mCurrentTransformFeedbackBuffers.count(buffers[bufferIndex]) != 0)
+        const vk::BufferHelper *buffer = buffers[bufferIndex];
+        if (mCurrentTransformFeedbackBuffers.count(buffer) != 0 ||
+            mRenderPassCommands->usesBuffer(*buffer))
         {
             ANGLE_TRY(endRenderPass());
             break;
@@ -4322,7 +4325,8 @@ angle::Result ContextVk::onBufferRead(VkAccessFlags readAccessType,
 
     ANGLE_TRY(endRenderPass());
 
-    if (!buffer->canAccumulateRead(this, readAccessType))
+    // A current write access means we need to start a new command buffer.
+    if (mOutsideRenderPassCommands->usesBufferForWrite(*buffer))
     {
         ANGLE_TRY(flushOutsideRenderPassCommands());
     }
@@ -4340,12 +4344,14 @@ angle::Result ContextVk::onBufferWrite(VkAccessFlags writeAccessType,
 
     ANGLE_TRY(endRenderPass());
 
-    if (!buffer->canAccumulateWrite(this, writeAccessType))
+    // Any current access means we need to start a new command buffer.
+    if (mOutsideRenderPassCommands->usesBuffer(*buffer))
     {
         ANGLE_TRY(flushOutsideRenderPassCommands());
     }
 
-    mOutsideRenderPassCommands->bufferWrite(&mResourceUseList, writeAccessType, writeStage, buffer);
+    mOutsideRenderPassCommands->bufferWrite(&mResourceUseList, writeAccessType, writeStage,
+                                            vk::BufferAliasingMode::Disallowed, buffer);
 
     return angle::Result::Continue;
 }

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -494,6 +494,11 @@ class ContextVk : public ContextImpl, public vk::Context
     {
         return onBufferWrite(VK_ACCESS_TRANSFER_WRITE_BIT, vk::PipelineStage::Transfer, buffer);
     }
+    angle::Result onBufferSelfCopy(vk::BufferHelper *buffer)
+    {
+        return onBufferWrite(VK_ACCESS_TRANSFER_READ_BIT | VK_ACCESS_TRANSFER_WRITE_BIT,
+                             vk::PipelineStage::Transfer, buffer);
+    }
     angle::Result onBufferComputeShaderRead(vk::BufferHelper *buffer)
     {
         return onBufferRead(VK_ACCESS_SHADER_READ_BIT, vk::PipelineStage::ComputeShader, buffer);

src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp

@@ -985,7 +985,8 @@ void ProgramExecutableVk::updateBuffersDescriptorSet(ContextVk *contextVk,
             // We set the SHADER_READ_BIT to be conservative.
             VkAccessFlags accessFlags = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT;
             commandBufferHelper->bufferWrite(resourceUseList, accessFlags,
-                                             kPipelineStageShaderMap[shaderType], &bufferHelper);
+                                             kPipelineStageShaderMap[shaderType],
+                                             vk::BufferAliasingMode::Allowed, &bufferHelper);
         }
         else
         {
@@ -1053,9 +1054,9 @@ void ProgramExecutableVk::updateAtomicCounterBuffersDescriptorSet(
                                         &writeInfo);
 
         // We set SHADER_READ_BIT to be conservative.
-        commandBufferHelper->bufferWrite(resourceUseList,
-                                         VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
-                                         kPipelineStageShaderMap[shaderType], &bufferHelper);
+        commandBufferHelper->bufferWrite(
+            resourceUseList, VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
+            kPipelineStageShaderMap[shaderType], vk::BufferAliasingMode::Allowed, &bufferHelper);
 
         writtenBindings.set(binding);
     }

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -557,6 +557,8 @@ CommandBufferHelper::~CommandBufferHelper()
 
 void CommandBufferHelper::initialize(bool isRenderPassCommandBuffer, bool mergeBarriers)
 {
+    ASSERT(mUsedBuffers.empty());
+
     mAllocator.initialize(kDefaultPoolAllocatorPageSize, 1);
     // Push a scope into the pool allocator so we can easily free and re-init on reset()
     mAllocator.push();
@@ -565,6 +567,17 @@ void CommandBufferHelper::initialize(bool isRenderPassCommandBuffer, bool mergeB
     mMergeBarriers             = mergeBarriers;
 }
 
+bool CommandBufferHelper::usesBuffer(const BufferHelper &buffer) const
+{
+    return mUsedBuffers.count(buffer.getBufferSerial()) > 0;
+}
+
+bool CommandBufferHelper::usesBufferForWrite(const BufferHelper &buffer) const
+{
+    auto iter = mUsedBuffers.find(buffer.getBufferSerial());
+    return iter != mUsedBuffers.end() && iter->second == BufferAccess::Write;
+}
+
 void CommandBufferHelper::bufferRead(vk::ResourceUseList *resourceUseList,
                                      VkAccessFlags readAccessType,
                                      vk::PipelineStage readStage,
@@ -576,11 +589,15 @@ void CommandBufferHelper::bufferRead(vk::ResourceUseList *resourceUseList,
     {
         mPipelineBarrierMask.set(readStage);
     }
+
+    ASSERT(!usesBufferForWrite(*buffer));
+    mUsedBuffers[buffer->getBufferSerial()] = BufferAccess::Read;
 }
 
 void CommandBufferHelper::bufferWrite(vk::ResourceUseList *resourceUseList,
                                       VkAccessFlags writeAccessType,
                                       vk::PipelineStage writeStage,
+                                      BufferAliasingMode aliasingMode,
                                       vk::BufferHelper *buffer)
 {
     buffer->retain(resourceUseList);
@@ -589,6 +606,16 @@ void CommandBufferHelper::bufferWrite(vk::ResourceUseList *resourceUseList,
     {
         mPipelineBarrierMask.set(writeStage);
     }
+
+    // Storage buffers are special. They can alias one another in a shader.
+    // We support aliasing by not tracking storage buffers. This works well with the GL API
+    // because storage buffers are required to be externally synchronized.
+    // Compute / XFB emulation buffers are not allowed to alias.
+    if (aliasingMode == BufferAliasingMode::Disallowed)
+    {
+        ASSERT(!usesBuffer(*buffer));
+        mUsedBuffers[buffer->getBufferSerial()] = BufferAccess::Write;
+    }
 }
 
 void CommandBufferHelper::imageRead(vk::ResourceUseList *resourceUseList,
@@ -884,6 +911,8 @@ void CommandBufferHelper::reset()
     mAllocator.pop();
     mAllocator.push();
     mCommandBuffer.reset();
+    mUsedBuffers.clear();
+
     if (mIsRenderPassCommandBuffer)
     {
         mRenderPassStarted                 = false;
@@ -2490,22 +2519,6 @@ bool BufferHelper::isReleasedToExternal() const
 #endif
 }
 
-bool BufferHelper::canAccumulateRead(ContextVk *contextVk, VkAccessFlags readAccessType)
-{
-    // We only need to start a new command buffer when we need a new barrier.
-    // For simplicity's sake for now we always start a new command buffer.
-    // TODO(jmadill): Re-use the command buffer. http://anglebug.com/4429
-    return false;
-}
-
-bool BufferHelper::canAccumulateWrite(ContextVk *contextVk, VkAccessFlags writeAccessType)
-{
-    // We only need to start a new command buffer when we need a new barrier.
-    // For simplicity's sake for now we always start a new command buffer.
-    // TODO(jmadill): Re-use the command buffer. http://anglebug.com/4429
-    return false;
-}
-
 bool BufferHelper::updateReadBarrier(VkAccessFlags readAccessType,
                                      VkPipelineStageFlags readStage,
                                      PipelineBarrier *barrier)

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -798,10 +798,6 @@ class BufferHelper final : public Resource
     // Returns true if the image is owned by an external API or instance.
     bool isReleasedToExternal() const;
 
-    // Currently always returns false. Should be smarter about accumulation.
-    bool canAccumulateRead(ContextVk *contextVk, VkAccessFlags readAccessType);
-    bool canAccumulateWrite(ContextVk *contextVk, VkAccessFlags writeAccessType);
-
     bool updateReadBarrier(VkAccessFlags readAccessType,
                            VkPipelineStageFlags readStage,
                            PipelineBarrier *barrier);
@@ -835,6 +831,18 @@ class BufferHelper final : public Resource
     BufferSerial mSerial;
 };
 
+enum class BufferAccess
+{
+    Read,
+    Write,
+};
+
+enum class BufferAliasingMode
+{
+    Allowed,
+    Disallowed,
+};
+
 // CommandBufferHelper (CBH) class wraps ANGLE's custom command buffer
 //  class, SecondaryCommandBuffer. This provides a way to temporarily
 //  store Vulkan commands that be can submitted in-line to a primary
@@ -859,6 +867,7 @@ struct CommandBufferHelper : angle::NonCopyable
     void bufferWrite(vk::ResourceUseList *resourceUseList,
                      VkAccessFlags writeAccessType,
                      vk::PipelineStage writeStage,
+                     BufferAliasingMode aliasingMode,
                      vk::BufferHelper *buffer);
 
     void imageRead(vk::ResourceUseList *resourceUseList,
@@ -960,6 +969,9 @@ struct CommandBufferHelper : angle::NonCopyable
         return mFramebuffer.getHandle();
     }
 
+    bool usesBuffer(const BufferHelper &buffer) const;
+    bool usesBufferForWrite(const BufferHelper &buffer) const;
+
     // Dumping the command stream is disabled by default.
     static constexpr bool kEnableCommandStreamDiagnostics = false;
 
@@ -997,7 +1009,11 @@ struct CommandBufferHelper : angle::NonCopyable
     bool mDepthTestEverEnabled;
     bool mStencilTestEverEnabled;
     uint32_t mDepthStencilAttachmentIndex;
+
+    // Tracks resources used in the command buffer.
+    std::unordered_map<BufferSerial, BufferAccess> mUsedBuffers;
 };
+
 static constexpr uint32_t kInvalidAttachmentIndex = -1;
 
 // Imagine an image going through a few layout transitions:

src/tests/gl_tests/VulkanPerformanceCounterTest.cpp

@@ -140,6 +140,50 @@ TEST_P(VulkanPerformanceCounterTest, ChangingMaxLevelHitsDescriptorCache)
     EXPECT_EQ(expectedWriteDescriptorSetCount, actualWriteDescriptorSetCount);
 }
 
+// Tests that two glCopyBufferSubData commands can share a barrier.
+TEST_P(VulkanPerformanceCounterTest, IndependentBufferCopiesShareSingleBarrier)
+{
+    constexpr GLint srcDataA[] = {1, 2, 3, 4};
+    constexpr GLint srcDataB[] = {5, 6, 7, 8};
+
+    // Step 1: Set up four buffers for two copies.
+    GLBuffer srcA;
+    glBindBuffer(GL_COPY_READ_BUFFER, srcA);
+    glBufferData(GL_COPY_READ_BUFFER, sizeof(srcDataA), srcDataA, GL_STATIC_COPY);
+
+    GLBuffer dstA;
+    glBindBuffer(GL_COPY_WRITE_BUFFER, dstA);
+    glBufferData(GL_COPY_WRITE_BUFFER, sizeof(srcDataA[0]) * 2, nullptr, GL_STATIC_COPY);
+
+    GLBuffer srcB;
+    glBindBuffer(GL_COPY_READ_BUFFER, srcB);
+    glBufferData(GL_COPY_READ_BUFFER, sizeof(srcDataB), srcDataB, GL_STATIC_COPY);
+
+    GLBuffer dstB;
+    glBindBuffer(GL_COPY_WRITE_BUFFER, dstB);
+    glBufferData(GL_COPY_WRITE_BUFFER, sizeof(srcDataB[0]) * 2, nullptr, GL_STATIC_COPY);
+
+    // We expect that ANGLE generate zero additional command buffers.
+    const rx::vk::PerfCounters &counters = hackANGLE();
+    uint32_t expectedFlushCount          = counters.flushedOutsideRenderPassCommandBuffers;
+
+    // Step 2: Do the two copies.
+    glBindBuffer(GL_COPY_READ_BUFFER, srcA);
+    glBindBuffer(GL_COPY_WRITE_BUFFER, dstA);
+    glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, sizeof(srcDataB[0]), 0,
+                        sizeof(srcDataA[0]) * 2);
+
+    glBindBuffer(GL_COPY_READ_BUFFER, srcB);
+    glBindBuffer(GL_COPY_WRITE_BUFFER, dstB);
+    glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, sizeof(srcDataB[0]), 0,
+                        sizeof(srcDataB[0]) * 2);
+
+    ASSERT_GL_NO_ERROR();
+
+    uint32_t actualFlushCount = counters.flushedOutsideRenderPassCommandBuffers;
+    EXPECT_EQ(expectedFlushCount, actualFlushCount);
+}
+
 ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest, ES3_VULKAN());
 
 }  // anonymous namespace