Vulkan: noop glMemoryBarrier(CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT)

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -87,9 +87,18 @@ ANGLE_INLINE VkMemoryPropertyFlags GetStorageMemoryType(GLbitfield storageFlags,
         (VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT |
          VK_MEMORY_PROPERTY_HOST_COHERENT_BIT);
 
-    if (((storageFlags & GL_MAP_COHERENT_BIT_EXT) != 0) ||
-        ((storageFlags & GL_MAP_PERSISTENT_BIT_EXT) != 0) || externalBuffer)
-    {
+    const bool isCoherentMap   = (storageFlags & GL_MAP_COHERENT_BIT_EXT) != 0;
+    const bool isPersistentMap = (storageFlags & GL_MAP_PERSISTENT_BIT_EXT) != 0;
+
+    if (isCoherentMap || isPersistentMap || externalBuffer)
+    {
+        // We currently allocate coherent memory for persistently mapped buffers.
+        // GL_EXT_buffer_storage allows non-coherent memory, but currently the implementation of
+        // |glMemoryBarrier(CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT)| relies on the mapping being
+        // coherent.
+        //
+        // If persistently mapped buffers ever use non-coherent memory, then said |glMemoryBarrier|
+        // call must result in |vkInvalidateMappedMemoryRanges| for all persistently mapped buffers.
         return kDeviceLocalHostCoherentFlags;
     }
 

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -3728,6 +3728,30 @@ angle::Result ContextVk::memoryBarrierByRegion(const gl::Context *context, GLbit
 
 angle::Result ContextVk::memoryBarrierImpl(GLbitfield barriers, VkPipelineStageFlags stageMask)
 {
+    // First, turn GL_ALL_BARRIER_BITS into a mask that has only the valid barriers set.
+    constexpr GLbitfield kCoreBarrierBits =
+        GL_VERTEX_ATTRIB_ARRAY_BARRIER_BIT | GL_ELEMENT_ARRAY_BARRIER_BIT | GL_UNIFORM_BARRIER_BIT |
+        GL_TEXTURE_FETCH_BARRIER_BIT | GL_SHADER_IMAGE_ACCESS_BARRIER_BIT | GL_COMMAND_BARRIER_BIT |
+        GL_PIXEL_BUFFER_BARRIER_BIT | GL_TEXTURE_UPDATE_BARRIER_BIT | GL_BUFFER_UPDATE_BARRIER_BIT |
+        GL_FRAMEBUFFER_BARRIER_BIT | GL_TRANSFORM_FEEDBACK_BARRIER_BIT |
+        GL_ATOMIC_COUNTER_BARRIER_BIT | GL_SHADER_STORAGE_BARRIER_BIT;
+    constexpr GLbitfield kExtensionBarrierBits = GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT;
+
+    barriers &= kCoreBarrierBits | kExtensionBarrierBits;
+
+    // GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT specifies that a fence sync or glFinish must be used
+    // after the barrier for the CPU to to see the shader writes.  Since host-visible buffer writes
+    // always issue a barrier automatically for the sake of glMapBuffer() (see
+    // comment on |mIsAnyHostVisibleBufferWritten|), there's nothing to do for
+    // GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT.
+    barriers &= ~GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT;
+
+    // If no other barrier, early out.
+    if (barriers == 0)
+    {
+        return angle::Result::Continue;
+    }
+
     // Note: many of the barriers specified here are already covered automatically.
     //
     // The barriers that are necessary all have SHADER_WRITE as src access and the dst access is
@@ -3760,12 +3784,6 @@ angle::Result ContextVk::memoryBarrierImpl(GLbitfield barriers, VkPipelineStageF
     mOutsideRenderPassCommands->getCommandBuffer().memoryBarrier(stageMask, stageMask,
                                                                  &memoryBarrier);
 
-    if ((barriers & GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT) != 0)
-    {
-        // We need to make sure that all device-writes are host-visible, force a finish
-        ANGLE_TRY(finishImpl());
-    }
-
     return angle::Result::Continue;
 }
 

src/tests/gl_tests/ComputeShaderTest.cpp

@@ -4352,6 +4352,106 @@ void main()
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::yellow);
 }
 
+// Test glMemoryBarrier(CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT) by writing to persistenly mapped
+// buffer from a compute shader.
+TEST_P(ComputeShaderTest, WriteToPersistentBuffer)
+{
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_buffer_storage"));
+
+    constexpr char kCS[] = R"(#version 310 es
+layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
+layout(std140, binding = 0) buffer block {
+    uvec4 data;
+} outBlock;
+void main()
+{
+    outBlock.data += uvec4(1);
+})";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program, kCS);
+    glUseProgram(program);
+
+    constexpr std::array<uint32_t, 4> kInitData = {};
+
+    GLBuffer coherentBuffer;
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, coherentBuffer);
+    glBufferStorageEXT(
+        GL_SHADER_STORAGE_BUFFER, sizeof(kInitData), kInitData.data(),
+        GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT | GL_MAP_COHERENT_BIT_EXT);
+
+    GLBuffer nonCoherentBuffer;
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, nonCoherentBuffer);
+    glBufferStorageEXT(GL_SHADER_STORAGE_BUFFER, sizeof(kInitData), kInitData.data(),
+                       GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT);
+
+    // Map the buffers for read and write.
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, coherentBuffer);
+    uint32_t *coherentMapped = reinterpret_cast<uint32_t *>(glMapBufferRange(
+        GL_SHADER_STORAGE_BUFFER, 0, sizeof(kInitData),
+        GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT | GL_MAP_COHERENT_BIT_EXT));
+    ASSERT_GL_NO_ERROR();
+
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, nonCoherentBuffer);
+    uint32_t *nonCoherentMapped = reinterpret_cast<uint32_t *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, sizeof(kInitData),
+                         GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT_EXT));
+    ASSERT_GL_NO_ERROR();
+
+    constexpr std::array<uint32_t, 4> kCoherentExpectedData = {
+        0x12354678u,
+        0x2468ACE0u,
+        0x13579BDFu,
+        0x76543210u,
+    };
+
+    constexpr std::array<uint32_t, 4> kNonCoherentExpectedData = {
+        0x9ABCDEF0u,
+        0xFDB97531u,
+        0x1F2E3D4Bu,
+        0x5A697887u,
+    };
+
+    coherentMapped[0] = kCoherentExpectedData[0] - 1;
+    coherentMapped[1] = kCoherentExpectedData[1] - 1;
+    coherentMapped[2] = kCoherentExpectedData[2] - 1;
+    coherentMapped[3] = kCoherentExpectedData[3] - 1;
+
+    nonCoherentMapped[0] = kNonCoherentExpectedData[0] - 1;
+    nonCoherentMapped[1] = kNonCoherentExpectedData[1] - 1;
+    nonCoherentMapped[2] = kNonCoherentExpectedData[2] - 1;
+    nonCoherentMapped[3] = kNonCoherentExpectedData[3] - 1;
+
+    // Test coherent write
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, coherentBuffer);
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    glFinish();
+    EXPECT_EQ(coherentMapped[0], kCoherentExpectedData[0]);
+    EXPECT_EQ(coherentMapped[1], kCoherentExpectedData[1]);
+    EXPECT_EQ(coherentMapped[2], kCoherentExpectedData[2]);
+    EXPECT_EQ(coherentMapped[3], kCoherentExpectedData[3]);
+
+    // Test non-coherent write
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, nonCoherentBuffer);
+    glDispatchCompute(1, 1, 1);
+
+    glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT_EXT);
+    EXPECT_GL_NO_ERROR();
+
+    glFinish();
+    EXPECT_EQ(nonCoherentMapped[0], kNonCoherentExpectedData[0]);
+    EXPECT_EQ(nonCoherentMapped[1], kNonCoherentExpectedData[1]);
+    EXPECT_EQ(nonCoherentMapped[2], kNonCoherentExpectedData[2]);
+    EXPECT_EQ(nonCoherentMapped[3], kNonCoherentExpectedData[3]);
+
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, coherentBuffer);
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, nonCoherentBuffer);
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+    EXPECT_GL_NO_ERROR();
+}
+
 ANGLE_INSTANTIATE_TEST_ES31(ComputeShaderTest);
 ANGLE_INSTANTIATE_TEST_ES3(ComputeShaderTestES3);
 ANGLE_INSTANTIATE_TEST_ES31(WebGL2ComputeTest);