Vulkan: Implement memory barriers

src/libANGLE/renderer/vulkan/CommandGraph.cpp

@@ -112,6 +112,8 @@ const char *GetResourceTypeName(CommandGraphResourceType resourceType,
                     UNREACHABLE();
                     return "FenceSync";
             }
+        case CommandGraphResourceType::GraphBarrier:
+            return "GraphBarrier";
         case CommandGraphResourceType::DebugMarker:
             switch (function)
             {
@@ -355,6 +357,7 @@ CommandGraphNode::CommandGraphNode(CommandGraphNodeFunction function,
       mVisitedState(VisitedState::Unvisited),
       mGlobalMemoryBarrierSrcAccess(0),
       mGlobalMemoryBarrierDstAccess(0),
+      mGlobalMemoryBarrierStages(0),
       mRenderPassOwner(nullptr)
 {}
 
@@ -529,26 +532,24 @@ angle::Result CommandGraphNode::visitAndExecute(vk::Context *context,
                                                 RenderPassCache *renderPassCache,
                                                 PrimaryCommandBuffer *primaryCommandBuffer)
 {
+    // Record the deferred pipeline barrier if necessary.
+    ASSERT((mGlobalMemoryBarrierDstAccess == 0) == (mGlobalMemoryBarrierSrcAccess == 0));
+    if (mGlobalMemoryBarrierSrcAccess)
+    {
+        VkMemoryBarrier memoryBarrier = {};
+        memoryBarrier.sType           = VK_STRUCTURE_TYPE_MEMORY_BARRIER;
+        memoryBarrier.srcAccessMask   = mGlobalMemoryBarrierSrcAccess;
+        memoryBarrier.dstAccessMask   = mGlobalMemoryBarrierDstAccess;
+
+        primaryCommandBuffer->memoryBarrier(mGlobalMemoryBarrierStages, mGlobalMemoryBarrierStages,
+                                            &memoryBarrier);
+    }
+
     switch (mFunction)
     {
         case CommandGraphNodeFunction::Generic:
             ASSERT(mQueryPool == VK_NULL_HANDLE && mFenceSyncEvent == VK_NULL_HANDLE);
 
-            // Record the deferred pipeline barrier if necessary.
-            ASSERT((mGlobalMemoryBarrierDstAccess == 0) == (mGlobalMemoryBarrierSrcAccess == 0));
-            if (mGlobalMemoryBarrierSrcAccess)
-            {
-                VkMemoryBarrier memoryBarrier = {};
-                memoryBarrier.sType           = VK_STRUCTURE_TYPE_MEMORY_BARRIER;
-                memoryBarrier.srcAccessMask   = mGlobalMemoryBarrierSrcAccess;
-                memoryBarrier.dstAccessMask   = mGlobalMemoryBarrierDstAccess;
-
-                // Use the all pipe stage to keep the state management simple.
-                primaryCommandBuffer->memoryBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
-                                                    VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
-                                                    &memoryBarrier);
-            }
-
             if (mOutsideRenderPassCommands.valid())
             {
                 ANGLE_VK_TRY(context, mOutsideRenderPassCommands.end());
@@ -639,6 +640,11 @@ angle::Result CommandGraphNode::visitAndExecute(vk::Context *context,
 
             break;
 
+        case CommandGraphNodeFunction::GraphBarrier:
+            // Nothing to do.  The memory barrier, if any, is already handled above through global
+            // memory barrier flags.
+            break;
+
         case CommandGraphNodeFunction::InsertDebugMarker:
             ASSERT(!mOutsideRenderPassCommands.valid() && !mInsideRenderPassCommands.valid());
 
@@ -707,6 +713,13 @@ void CommandGraphNode::setDiagnosticInfo(CommandGraphResourceType resourceType,
     mResourceID   = resourceID;
 }
 
+bool CommandGraphNode::hasDiagnosticID() const
+{
+    // All nodes have diagnostic IDs to differentiate them except the following select few.
+    return mResourceType != CommandGraphResourceType::HostAvailabilityOperation &&
+           mResourceType != CommandGraphResourceType::GraphBarrier;
+}
+
 std::string CommandGraphNode::dumpCommandsForDiagnostics(const char *separator) const
 {
     std::string result;
@@ -989,6 +1002,13 @@ void CommandGraph::waitFenceSync(const vk::Event &event)
     newNode->setFenceSync(event);
 }
 
+void CommandGraph::memoryBarrier(VkFlags srcAccess, VkFlags dstAccess, VkPipelineStageFlags stages)
+{
+    CommandGraphNode *newNode = allocateBarrierNode(CommandGraphNodeFunction::GraphBarrier,
+                                                    CommandGraphResourceType::GraphBarrier, 0);
+    newNode->addGlobalMemoryBarrier(srcAccess, dstAccess, stages);
+}
+
 void CommandGraph::insertDebugMarker(GLenum source, std::string &&marker)
 {
     CommandGraphNode *newNode = allocateBarrierNode(CommandGraphNodeFunction::InsertDebugMarker,
@@ -1078,10 +1098,9 @@ void CommandGraph::dumpGraphDotFile(std::ostream &out) const
                 strstr << id;
             }
         }
-        else if (node->getResourceTypeForDiagnostics() ==
-                 CommandGraphResourceType::HostAvailabilityOperation)
+        else if (!node->hasDiagnosticID())
         {
-            // Nothing to append for this special node.  The name is sufficient.
+            // Nothing to append for these special nodes.  The name is sufficient.
         }
         else
         {

src/libANGLE/renderer/vulkan/CommandGraph.h

@@ -37,6 +37,7 @@ enum class CommandGraphResourceType
     // VK_EXT_transform_feedback), but still need to generate a command graph barrier node.
     EmulatedQuery,
     FenceSync,
+    GraphBarrier,
     DebugMarker,
     HostAvailabilityOperation,
 };
@@ -53,6 +54,7 @@ enum class CommandGraphNodeFunction
     EndTransformFeedbackQuery,
     SetFenceSync,
     WaitFenceSync,
+    GraphBarrier,
     InsertDebugMarker,
     PushDebugMarker,
     PopDebugMarker,
@@ -178,6 +180,7 @@ class CommandGraphNode final : angle::NonCopyable
 
     CommandGraphResourceType getResourceTypeForDiagnostics() const { return mResourceType; }
     uintptr_t getResourceIDForDiagnostics() const { return mResourceID; }
+    bool hasDiagnosticID() const;
     std::string dumpCommandsForDiagnostics(const char *separator) const;
 
     const gl::Rectangle &getRenderPassRenderArea() const { return mRenderPassRenderArea; }
@@ -191,10 +194,13 @@ class CommandGraphNode final : angle::NonCopyable
     void setDebugMarker(GLenum source, std::string &&marker);
     const std::string &getDebugMarker() const { return mDebugMarker; }
 
-    ANGLE_INLINE void addGlobalMemoryBarrier(VkFlags srcAccess, VkFlags dstAccess)
+    ANGLE_INLINE void addGlobalMemoryBarrier(VkFlags srcAccess,
+                                             VkFlags dstAccess,
+                                             VkPipelineStageFlags stages)
     {
         mGlobalMemoryBarrierSrcAccess |= srcAccess;
         mGlobalMemoryBarrierDstAccess |= dstAccess;
+        mGlobalMemoryBarrierStages |= stages;
     }
 
     // This can only be set for RenderPass nodes. Each RenderPass node can have at most one owner.
@@ -257,6 +263,7 @@ class CommandGraphNode final : angle::NonCopyable
     // For global memory barriers.
     VkFlags mGlobalMemoryBarrierSrcAccess;
     VkFlags mGlobalMemoryBarrierDstAccess;
+    VkPipelineStageFlags mGlobalMemoryBarrierStages;
 
     // Render pass command buffer notifications.
     RenderPassOwner *mRenderPassOwner;
@@ -401,10 +408,10 @@ class CommandGraphResource : angle::NonCopyable
     void finishCurrentCommands(ContextVk *contextVk);
 
     // Store a deferred memory barrier. Will be recorded into a primary command buffer at submit.
-    void addGlobalMemoryBarrier(VkFlags srcAccess, VkFlags dstAccess)
+    void addGlobalMemoryBarrier(VkFlags srcAccess, VkFlags dstAccess, VkPipelineStageFlags stages)
     {
         ASSERT(mCurrentWritingNode);
-        mCurrentWritingNode->addGlobalMemoryBarrier(srcAccess, dstAccess);
+        mCurrentWritingNode->addGlobalMemoryBarrier(srcAccess, dstAccess, stages);
     }
 
   protected:
@@ -490,6 +497,8 @@ class CommandGraph final : angle::NonCopyable
     // GLsync and EGLSync:
     void setFenceSync(const vk::Event &event);
     void waitFenceSync(const vk::Event &event);
+    // Memory barriers:
+    void memoryBarrier(VkFlags srcAccess, VkFlags dstAccess, VkPipelineStageFlags stages);
     // Debug markers:
     void insertDebugMarker(GLenum source, std::string &&marker);
     void pushDebugMarker(GLenum source, std::string &&marker);

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -2010,14 +2010,36 @@ angle::Result ContextVk::dispatchComputeIndirect(const gl::Context *context, GLi
 
 angle::Result ContextVk::memoryBarrier(const gl::Context *context, GLbitfield barriers)
 {
-    ANGLE_VK_UNREACHABLE(this);
-    return angle::Result::Stop;
+    // Note: most of the barriers specified here don't require us to issue a memory barrier, as the
+    // relevant resources already insert the appropriate barriers.  They do however require the
+    // resource writing nodes to finish so future buffer barriers are placed correctly, as well as
+    // resource dependencies not creating a graph loop.  This is done by inserting a command graph
+    // barrier that does nothing!
+
+    VkAccessFlags srcAccess = 0;
+    VkAccessFlags dstAccess = 0;
+
+    if ((barriers & GL_COMMAND_BARRIER_BIT) != 0)
+    {
+        srcAccess |= VK_ACCESS_SHADER_WRITE_BIT;
+        dstAccess |= VK_ACCESS_INDIRECT_COMMAND_READ_BIT;
+    }
+
+    mCommandGraph.memoryBarrier(srcAccess, dstAccess, VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT);
+    return angle::Result::Continue;
 }
 
 angle::Result ContextVk::memoryBarrierByRegion(const gl::Context *context, GLbitfield barriers)
 {
-    ANGLE_VK_UNREACHABLE(this);
-    return angle::Result::Stop;
+    // There aren't any barrier bits here that aren't otherwise automatically handled.  We only
+    // need to make sure writer resources (framebuffers and the dispatcher) start a new node.
+    //
+    // Note: memoryBarrierByRegion is expected to affect only the fragment pipeline.  Specifying
+    // that here is currently unnecessary, but is a reminder of this fact in case we do need to
+    // especially handle some future barrier bit.
+
+    mCommandGraph.memoryBarrier(0, 0, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT);
+    return angle::Result::Continue;
 }
 
 vk::DynamicQueryPool *ContextVk::getQueryPool(gl::QueryType queryType)

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -1322,7 +1322,7 @@ void BufferHelper::onWriteAccess(ContextVk *contextVk,
     VkAccessFlags barrierSrc, barrierDst;
     if (needsOnWriteBarrier(readAccessType, writeAccessType, &barrierSrc, &barrierDst))
     {
-        addGlobalMemoryBarrier(barrierSrc, barrierDst);
+        addGlobalMemoryBarrier(barrierSrc, barrierDst, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
     }
 
     bool hostVisible = mMemoryPropertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -555,7 +555,8 @@ class BufferHelper final : public CommandGraphResource
         VkAccessFlags barrierSrc, barrierDst;
         if (needsOnReadBarrier(readAccessType, &barrierSrc, &barrierDst))
         {
-            reader->addGlobalMemoryBarrier(barrierSrc, barrierDst);
+            reader->addGlobalMemoryBarrier(barrierSrc, barrierDst,
+                                           VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
         }
     }
     bool needsOnWriteBarrier(VkAccessFlags readAccessType,

src/tests/deqp_support/deqp_gles31_test_expectations.txt

@@ -632,10 +632,6 @@
 3520 VULKAN : dEQP-GLES31.functional.program_interface_query.program_*.resource_list.compute.empty = FAIL
 3520 VULKAN : dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.* = FAIL
 
-// glMemoryBarrier support:
-3574 VULKAN : dEQP-GLES31.functional.compute.*barrier* = SKIP
-3574 VULKAN : dEQP-GLES31.functional.synchronization.*memory_barrier* = SKIP
-
 // Indirect dispatch:
 3601 VULKAN : dEQP-GLES31.functional.compute.*indirect* = SKIP
 

src/tests/deqp_support/deqp_khr_gles31_test_expectations.txt

@@ -36,12 +36,11 @@
 // General Vulkan expectations
 
 // Limits:
-// GL_MIN/MAX_PROGRAM_TEXTURE_GATHER_OFFSET not set.  Also crashes on memory barrier support missing.
-3605 VULKAN : KHR-GLES31.core.texture_gather.* = SKIP
+// GL_MIN/MAX_PROGRAM_TEXTURE_GATHER_OFFSET not set.
+3605 VULKAN : KHR-GLES31.core.texture_gather.* = FAIL
 
-// Memory barriers
-3574 VULKAN : KHR-GLES31.core.compute_shader* = SKIP
-3574 VULKAN : KHR-GLES31.core.shader_atomic_counters.basic-glsl-built-in = SKIP
+// Dispatch indirect:
+3601 VULKAN : KHR-GLES31.core.compute_shader* = SKIP
 
 // Multisampled textures:
 3565 VULKAN : KHR-GLES31.core.texture_storage_multisample.* = SKIP

src/tests/gl_tests/ShaderStorageBufferTest.cpp

@@ -315,7 +315,7 @@ TEST_P(ShaderStorageBufferTest31, ShaderStorageBufferReadWrite)
 // Tests modifying an existing shader storage buffer
 TEST_P(ShaderStorageBufferTest31, ShaderStorageBufferReadWriteSame)
 {
-    // Vulkan doesn't support memory barriers yet. http://anglebug.com/3574
+    // Missing PBO support in Vulkan.  http://anglebug.com/3210
     ANGLE_SKIP_TEST_IF(IsVulkan());
 
     constexpr char kComputeShaderSource[] =
@@ -413,9 +413,6 @@ void main()
 // Tests reading and writing to a shader storage buffer bound at an offset.
 TEST_P(ShaderStorageBufferTest31, ShaderStorageBufferReadWriteOffset)
 {
-    // Vulkan doesn't support memory barriers yet. http://anglebug.com/3574
-    ANGLE_SKIP_TEST_IF(IsVulkan());
-
     constexpr char kCS[] = R"(#version 310 es
 layout(local_size_x=1, local_size_y=1, local_size_z=1) in;