Vulkan: Pack DescriptorSetLayoutDesc layout

Use angle::FastVector instead of arrays to further compact
DescriptorSetLayoutDesc layout

Bug: angleproject:8677
Tests: VulkanDescriptorSetLayoutDescTest*
Change-Id: I5bb7b2ebf0aa5aba3d7c47c45384788245dce3dc
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5470362
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: mohan maiya <m.maiya@samsung.com>

3 files changed, 197 insertions, 54 deletions

diff --git a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
index 46c259230c..e1d1b3cd23 100644
--- a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
@@ -4274,10 +4274,8 @@ bool operator==(const AttachmentOpsArray &lhs, const AttachmentOpsArray &rhs)
 
 // DescriptorSetLayoutDesc implementation.
 DescriptorSetLayoutDesc::DescriptorSetLayoutDesc()
-    : mPackedDescriptorSetLayout{}, mValidDescriptorSetLayoutIndexMask()
-{
-    mImmutableSamplers.fill(VK_NULL_HANDLE);
-}
+    : mDescriptorSetLayoutBindings{}, mImmutableSamplers{}
+{}
 
 DescriptorSetLayoutDesc::~DescriptorSetLayoutDesc() = default;
 
@@ -4288,20 +4286,32 @@ DescriptorSetLayoutDesc &DescriptorSetLayoutDesc::operator=(const DescriptorSetL
 
 size_t DescriptorSetLayoutDesc::hash() const
 {
-    size_t genericHash = angle::ComputeGenericHash(mValidDescriptorSetLayoutIndexMask);
-    for (size_t bindingIndex : mValidDescriptorSetLayoutIndexMask)
+    size_t validDescriptorSetLayoutBindingsCount = mDescriptorSetLayoutBindings.size();
+    size_t validImmutableSamplersCount           = mImmutableSamplers.size();
+
+    ASSERT(validDescriptorSetLayoutBindingsCount != 0 || validImmutableSamplersCount == 0);
+
+    size_t genericHash = 0;
+    if (validDescriptorSetLayoutBindingsCount > 0)
     {
-        genericHash ^= angle::ComputeGenericHash(mPackedDescriptorSetLayout[bindingIndex]) ^
-                       angle::ComputeGenericHash(mImmutableSamplers[bindingIndex]);
+        genericHash = angle::ComputeGenericHash(
+            mDescriptorSetLayoutBindings.data(),
+            validDescriptorSetLayoutBindingsCount * sizeof(PackedDescriptorSetBinding));
     }
+
+    if (validImmutableSamplersCount > 0)
+    {
+        genericHash ^= angle::ComputeGenericHash(mImmutableSamplers.data(),
+                                                 validImmutableSamplersCount * sizeof(VkSampler));
+    }
+
     return genericHash;
 }
 
 bool DescriptorSetLayoutDesc::operator==(const DescriptorSetLayoutDesc &other) const
 {
-    return memcmp(&mPackedDescriptorSetLayout, &other.mPackedDescriptorSetLayout,
-                  sizeof(mPackedDescriptorSetLayout)) == 0 &&
-           memcmp(&mImmutableSamplers, &other.mImmutableSamplers, sizeof(mImmutableSamplers)) == 0;
+    return mDescriptorSetLayoutBindings == other.mDescriptorSetLayoutBindings &&
+           mImmutableSamplers == other.mImmutableSamplers;
 }
 
 void DescriptorSetLayoutDesc::update(uint32_t bindingIndex,
@@ -4310,43 +4320,46 @@ void DescriptorSetLayoutDesc::update(uint32_t bindingIndex,
                                      VkShaderStageFlags stages,
                                      const Sampler *immutableSampler)
 {
-    ASSERT(static_cast<size_t>(descriptorType) < std::numeric_limits<uint16_t>::max());
+    ASSERT(static_cast<size_t>(descriptorType) < std::numeric_limits<uint8_t>::max());
     ASSERT(count < std::numeric_limits<uint16_t>::max());
+    ASSERT(bindingIndex < std::numeric_limits<uint16_t>::max());
 
-    PackedDescriptorSetBinding &packedBinding = mPackedDescriptorSetLayout[bindingIndex];
-
+    PackedDescriptorSetBinding packedBinding = {};
     SetBitField(packedBinding.type, descriptorType);
     SetBitField(packedBinding.count, count);
     SetBitField(packedBinding.stages, stages);
+    SetBitField(packedBinding.bindingIndex, bindingIndex);
+    SetBitField(packedBinding.hasImmutableSampler, 0);
 
     if (immutableSampler)
     {
         ASSERT(count == 1);
-        ASSERT(bindingIndex < gl::IMPLEMENTATION_MAX_ACTIVE_TEXTURES);
-
-        mImmutableSamplers[bindingIndex] = immutableSampler->getHandle();
+        SetBitField(packedBinding.hasImmutableSampler, 1);
+        mImmutableSamplers.push_back(immutableSampler->getHandle());
     }
 
-    mValidDescriptorSetLayoutIndexMask.set(bindingIndex, count > 0);
+    mDescriptorSetLayoutBindings.push_back(std::move(packedBinding));
 }
 
 void DescriptorSetLayoutDesc::unpackBindings(DescriptorSetLayoutBindingVector *bindings) const
 {
-    for (size_t bindingIndex : mValidDescriptorSetLayoutIndexMask)
+    size_t immutableSamplersIndex = 0;
+
+    // Unpack all valid descriptor set layout bindings
+    for (const PackedDescriptorSetBinding &packedBinding : mDescriptorSetLayoutBindings)
     {
-        const PackedDescriptorSetBinding &packedBinding = mPackedDescriptorSetLayout[bindingIndex];
         ASSERT(packedBinding.count != 0);
 
         VkDescriptorSetLayoutBinding binding = {};
-        binding.binding                      = static_cast<uint32_t>(bindingIndex);
+        binding.binding                      = static_cast<uint32_t>(packedBinding.bindingIndex);
         binding.descriptorCount              = packedBinding.count;
         binding.descriptorType               = static_cast<VkDescriptorType>(packedBinding.type);
         binding.stageFlags = static_cast<VkShaderStageFlags>(packedBinding.stages);
-        if (mImmutableSamplers[bindingIndex] != VK_NULL_HANDLE)
+
+        if (packedBinding.hasImmutableSampler)
         {
             ASSERT(packedBinding.count == 1);
-            ASSERT(bindingIndex < gl::IMPLEMENTATION_MAX_ACTIVE_TEXTURES);
-            binding.pImmutableSamplers = &mImmutableSamplers[bindingIndex];
+            binding.pImmutableSamplers = &mImmutableSamplers[immutableSamplersIndex++];
         }
 
         bindings->push_back(binding);
@@ -4371,12 +4384,18 @@ PipelineLayoutDesc &PipelineLayoutDesc::operator=(const PipelineLayoutDesc &rhs)
 
 size_t PipelineLayoutDesc::hash() const
 {
-    return angle::ComputeGenericHash(*this);
+    size_t genericHash = angle::ComputeGenericHash(mPushConstantRange);
+    for (const DescriptorSetLayoutDesc &descriptorSetLayoutDesc : mDescriptorSetLayouts)
+    {
+        genericHash ^= descriptorSetLayoutDesc.hash();
+    }
+    return genericHash;
 }
 
 bool PipelineLayoutDesc::operator==(const PipelineLayoutDesc &other) const
 {
-    return memcmp(this, &other, sizeof(PipelineLayoutDesc)) == 0;
+    return mPushConstantRange == other.mPushConstantRange &&
+           mDescriptorSetLayouts == other.mDescriptorSetLayouts;
 }
 
 void PipelineLayoutDesc::updateDescriptorSetLayout(DescriptorSetIndex setIndex,
diff --git a/src/libANGLE/renderer/vulkan/vk_cache_utils.h b/src/libANGLE/renderer/vulkan/vk_cache_utils.h
index 4db163d9de..b04b604403 100644
--- a/src/libANGLE/renderer/vulkan/vk_cache_utils.h
+++ b/src/libANGLE/renderer/vulkan/vk_cache_utils.h
@@ -984,17 +984,11 @@ class GraphicsPipelineDesc final
 constexpr size_t kGraphicsPipelineDescSize = sizeof(GraphicsPipelineDesc);
 static_assert(kGraphicsPipelineDescSize == kGraphicsPipelineDescSumOfSizes, "Size mismatch");
 
-constexpr uint32_t kMaxDescriptorSetLayoutBindings =
-    std::max(gl::IMPLEMENTATION_MAX_ACTIVE_TEXTURES,
-             gl::IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS);
-
+// Values are based on data recorded here -> https://anglebug.com/8677#c4
+constexpr size_t kDefaultDescriptorSetLayoutBindingsCount = 8;
+constexpr size_t kDefaultImmutableSamplerBindingsCount    = 1;
 using DescriptorSetLayoutBindingVector =
-    angle::FixedVector<VkDescriptorSetLayoutBinding, kMaxDescriptorSetLayoutBindings>;
-
-// Technically this needs to only be kMaxDescriptorSetLayoutBindings but due to struct padding
-// issues round up size to 64.
-constexpr uint32_t kMaxDescriptorSetLayoutCount = roundUpPow2(kMaxDescriptorSetLayoutBindings, 64u);
-using DescriptorSetLayoutIndexMask              = angle::BitSet<kMaxDescriptorSetLayoutCount>;
+    angle::FastVector<VkDescriptorSetLayoutBinding, kDefaultDescriptorSetLayoutBindingsCount>;
 
 // A packed description of a descriptor set layout. Use similarly to RenderPassDesc and
 // GraphicsPipelineDesc. Currently we only need to differentiate layouts based on sampler and ubo
@@ -1018,39 +1012,53 @@ class DescriptorSetLayoutDesc final
 
     void unpackBindings(DescriptorSetLayoutBindingVector *bindings) const;
 
-    bool empty() const { return !mValidDescriptorSetLayoutIndexMask.any(); }
+    bool empty() const { return mDescriptorSetLayoutBindings.empty(); }
 
   private:
     // There is a small risk of an issue if the sampler cache is evicted but not the descriptor
     // cache we would have an invalid handle here. Thus propose follow-up work:
     // TODO: https://issuetracker.google.com/issues/159156775: Have immutable sampler use serial
-    struct PackedDescriptorSetBinding
+    union PackedDescriptorSetBinding
     {
-        uint8_t type;    // Stores a packed VkDescriptorType descriptorType.
-        uint8_t stages;  // Stores a packed VkShaderStageFlags.
-        uint16_t count;  // Stores a packed uint32_t descriptorCount.
+        struct
+        {
+            uint8_t type;                  // Stores a packed VkDescriptorType descriptorType.
+            uint8_t stages;                // Stores a packed VkShaderStageFlags.
+            uint16_t count;                // Stores a packed uint32_t descriptorCount
+            uint16_t bindingIndex;         // Stores the binding index
+            uint16_t hasImmutableSampler;  // Whether this binding has an immutable sampler
+        };
+        uint64_t value;
+
+        bool operator==(const PackedDescriptorSetBinding &other) const
+        {
+            return value == other.value;
+        }
     };
 
-    // 1x 32bit
-    static_assert(sizeof(PackedDescriptorSetBinding) == 4, "Unexpected size");
-
-    // This is a compact representation of a descriptor set layout.
-    std::array<PackedDescriptorSetBinding, kMaxDescriptorSetLayoutBindings>
-        mPackedDescriptorSetLayout;
-    gl::ActiveTextureArray<VkSampler> mImmutableSamplers;
+    // 1x 64bit
+    static_assert(sizeof(PackedDescriptorSetBinding) == 8, "Unexpected size");
 
-    DescriptorSetLayoutIndexMask mValidDescriptorSetLayoutIndexMask;
+    angle::FastVector<PackedDescriptorSetBinding, kDefaultDescriptorSetLayoutBindingsCount>
+        mDescriptorSetLayoutBindings;
+    angle::FastVector<VkSampler, kDefaultImmutableSamplerBindingsCount> mImmutableSamplers;
 };
 
 // The following are for caching descriptor set layouts. Limited to max three descriptor set
 // layouts. This can be extended in the future.
-constexpr size_t kMaxDescriptorSetLayouts = 3;
+constexpr size_t kMaxDescriptorSetLayouts = ToUnderlying(DescriptorSetIndex::EnumCount);
 
-struct PackedPushConstantRange
+union PackedPushConstantRange
 {
-    uint8_t offset;
-    uint8_t size;
-    uint16_t stageMask;
+    struct
+    {
+        uint8_t offset;
+        uint8_t size;
+        uint16_t stageMask;
+    };
+    uint32_t value;
+
+    bool operator==(const PackedPushConstantRange &other) const { return value == other.value; }
 };
 
 static_assert(sizeof(PackedPushConstantRange) == sizeof(uint32_t), "Unexpected Size");
@@ -2559,6 +2567,10 @@ class DescriptorSetLayoutCache final : angle::NonCopyable
         const vk::DescriptorSetLayoutDesc &desc,
         vk::AtomicBindingPointer<vk::DescriptorSetLayout> *descriptorSetLayoutOut);
 
+    // Helpers for white box tests
+    size_t getCacheHitCount() const { return mCacheStats.getHitCount(); }
+    size_t getCacheMissCount() const { return mCacheStats.getMissCount(); }
+
   private:
     mutable std::mutex mMutex;
     std::unordered_map<vk::DescriptorSetLayoutDesc, vk::RefCountedDescriptorSetLayout> mPayload;
diff --git a/src/tests/gl_tests/VulkanDescriptorSetTest.cpp b/src/tests/gl_tests/VulkanDescriptorSetTest.cpp
index 068f4113f6..7ec9b4af43 100644
--- a/src/tests/gl_tests/VulkanDescriptorSetTest.cpp
+++ b/src/tests/gl_tests/VulkanDescriptorSetTest.cpp
@@ -11,6 +11,7 @@
 #include "test_utils/gl_raii.h"
 
 #include "libANGLE/Context.h"
+#include "libANGLE/Display.h"
 #include "libANGLE/angletypes.h"
 #include "libANGLE/renderer/vulkan/ContextVk.h"
 #include "libANGLE/renderer/vulkan/ProgramVk.h"
@@ -97,6 +98,117 @@ TEST_P(VulkanDescriptorSetTest, AtomicCounterReadLimitedDescriptorPool)
     }
 }
 
+class VulkanDescriptorSetLayoutDescTest : public ANGLETest<>
+{
+  protected:
+    VulkanDescriptorSetLayoutDescTest() {}
+
+    void testSetUp() override { ANGLETest::testSetUp(); }
+
+    void testTearDown() override { ANGLETest::testTearDown(); }
+
+    gl::Context *hackContext() const
+    {
+        egl::Display *display   = static_cast<egl::Display *>(getEGLWindow()->getDisplay());
+        gl::ContextID contextID = {
+            static_cast<GLuint>(reinterpret_cast<uintptr_t>(getEGLWindow()->getContext()))};
+        return display->getContext(contextID);
+    }
+
+    rx::ContextVk *hackANGLE() const
+    {
+        // Hack the angle!
+        return rx::GetImplAs<rx::ContextVk>(hackContext());
+    }
+
+    struct DescriptorSetBinding
+    {
+        uint32_t bindingIndex;
+        VkDescriptorType type;
+        uint32_t bindingCount;
+        VkShaderStageFlagBits shaderStage;
+    };
+
+    const std::array<DescriptorSetBinding, 12> mBindings = {{
+        {0, VK_DESCRIPTOR_TYPE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {1, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+        {2, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {3, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+        {4, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {5, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+        {6, VK_DESCRIPTOR_TYPE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {7, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+        {8, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {9, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+        {10, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
+        {11, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT},
+    }};
+
+    void updateBindings(const std::vector<uint32_t> &bindingIndices,
+                        rx::vk::DescriptorSetLayoutDesc *desc)
+    {
+        for (uint32_t index : bindingIndices)
+        {
+            ASSERT(index < mBindings.size());
+            const DescriptorSetBinding &binding = mBindings[index];
+            desc->update(binding.bindingIndex, binding.type, binding.bindingCount,
+                         binding.shaderStage, nullptr);
+        }
+    }
+
+    rx::vk::DescriptorSetLayoutDesc mDescriptorSetLayoutDesc;
+    rx::DescriptorSetLayoutCache mDescriptorSetLayoutCache;
+};
+
+// Test basic interaction between DescriptorSetLayoutDesc and DescriptorSetLayoutCache
+TEST_P(VulkanDescriptorSetLayoutDescTest, Basic)
+{
+    const std::vector<uint32_t> bindingsPattern1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11};
+    const std::vector<uint32_t> bindingsPattern2 = {0, 1};
+    const std::vector<uint32_t> bindingsPattern3 = {0, 1, 5, 9};
+
+    angle::Result result;
+    rx::ContextVk *contextVk = hackANGLE();
+    rx::vk::AtomicBindingPointer<rx::vk::DescriptorSetLayout> descriptorSetLayout;
+
+    mDescriptorSetLayoutDesc = {};
+    updateBindings(bindingsPattern1, &mDescriptorSetLayoutDesc);
+    result = mDescriptorSetLayoutCache.getDescriptorSetLayout(contextVk, mDescriptorSetLayoutDesc,
+                                                              &descriptorSetLayout);
+    EXPECT_EQ(result, angle::Result::Continue);
+    EXPECT_EQ(mDescriptorSetLayoutCache.getCacheMissCount(), 1u);
+
+    mDescriptorSetLayoutDesc = {};
+    updateBindings(bindingsPattern2, &mDescriptorSetLayoutDesc);
+    result = mDescriptorSetLayoutCache.getDescriptorSetLayout(contextVk, mDescriptorSetLayoutDesc,
+                                                              &descriptorSetLayout);
+    EXPECT_EQ(result, angle::Result::Continue);
+    EXPECT_EQ(mDescriptorSetLayoutCache.getCacheMissCount(), 2u);
+
+    mDescriptorSetLayoutDesc = {};
+    updateBindings(bindingsPattern3, &mDescriptorSetLayoutDesc);
+    size_t reusedDescHash = mDescriptorSetLayoutDesc.hash();
+    result = mDescriptorSetLayoutCache.getDescriptorSetLayout(contextVk, mDescriptorSetLayoutDesc,
+                                                              &descriptorSetLayout);
+    EXPECT_EQ(result, angle::Result::Continue);
+    EXPECT_EQ(mDescriptorSetLayoutCache.getCacheMissCount(), 3u);
+
+    rx::vk::DescriptorSetLayoutDesc desc;
+    updateBindings(bindingsPattern3, &desc);
+    size_t newDescHash = desc.hash();
+    EXPECT_EQ(reusedDescHash, newDescHash);
+
+    result =
+        mDescriptorSetLayoutCache.getDescriptorSetLayout(contextVk, desc, &descriptorSetLayout);
+    EXPECT_EQ(result, angle::Result::Continue);
+    EXPECT_EQ(mDescriptorSetLayoutCache.getCacheHitCount(), 1u);
+    EXPECT_EQ(mDescriptorSetLayoutCache.getCacheMissCount(), 3u);
+
+    descriptorSetLayout.reset();
+    mDescriptorSetLayoutCache.destroy(contextVk->getRenderer());
+}
+
 ANGLE_INSTANTIATE_TEST(VulkanDescriptorSetTest, ES31_VULKAN(), ES31_VULKAN_SWIFTSHADER());
+ANGLE_INSTANTIATE_TEST(VulkanDescriptorSetLayoutDescTest, ES31_VULKAN());
 
 }  // namespace