[dawn][native] Allow directly iterating over bindings of one type. The BindGroupLayout reorders bindings and sorts them by type. This means that all buffers have a contiguous range, same for all textures, samplers, etc. Code looking at BGLs can use this to directly iterate over the bindings they are interested in instead of looping over bindings and calling MatchVariant or std::get_if. Introduce the scaffolding to support this kind of iteration and use it to make more robust the places in the code that already assumed that buffers are at the start. Bug: 439565641 Change-Id: I235f6df8ee2f0a3dc3a86ac715b47d0f9028b31c Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/257554 Reviewed-by: Loko Kung <lokokung@google.com> Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/BindGroup.cpp b/src/dawn/native/BindGroup.cpp index 9757bbf..be0dd95 100644 --- a/src/dawn/native/BindGroup.cpp +++ b/src/dawn/native/BindGroup.cpp
@@ -380,10 +380,10 @@ template <typename F> void ForEachUnverifiedBufferBindingIndexImpl(const BindGroupLayoutInternalBase* bgl, F&& f) { uint32_t packedIndex = 0; - for (BindingIndex bindingIndex{0}; bindingIndex < bgl->GetBufferCount(); ++bindingIndex) { - const auto* bufferLayout = - std::get_if<BufferBindingInfo>(&bgl->GetBindingInfo(bindingIndex).bindingLayout); - if (bufferLayout == nullptr || bufferLayout->minBindingSize == 0) { + for (BindingIndex bindingIndex : bgl->GetBufferIndices()) { + const auto& bufferLayout = + std::get<BufferBindingInfo>(bgl->GetBindingInfo(bindingIndex).bindingLayout); + if (bufferLayout.minBindingSize == 0) { f(bindingIndex, packedIndex++); } }
diff --git a/src/dawn/native/BindGroupLayoutInternal.cpp b/src/dawn/native/BindGroupLayoutInternal.cpp index c519d8c..523bca5 100644 --- a/src/dawn/native/BindGroupLayoutInternal.cpp +++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -38,7 +38,6 @@ #include "dawn/common/Enumerator.h" #include "dawn/common/MatchVariant.h" -#include "dawn/common/Range.h" #include "dawn/native/ChainUtils.h" #include "dawn/native/Device.h" #include "dawn/native/Error.h" @@ -524,35 +523,6 @@ return result; } -bool SortBindingsCompare(const BindingInfo& a, const BindingInfo& b) { - if (&a == &b) { - return false; - } - - // Buffers with dynamic offsets come first and then the rest of the buffers. Other bindings are - // only grouped by types. This is to make it easier and faster to handle them. - auto TypePriority = [](const BindingInfo& info) { - return MatchVariant( - info.bindingLayout, - [&](const BufferBindingInfo& layout) { return layout.hasDynamicOffset ? 0 : 1; }, - [&](const TextureBindingInfo&) { return 2; }, - [&](const StorageTextureBindingInfo&) { return 3; }, - [&](const SamplerBindingInfo&) { return 4; }, - [&](const StaticSamplerBindingInfo&) { return 5; }, - [&](const InputAttachmentBindingInfo&) { return 6; }); - }; - - auto aPriority = TypePriority(a); - auto bPriority = TypePriority(b); - if (aPriority != bPriority) { - return aPriority < bPriority; - } - - // Afterwards sort the bindings by binding number. This is necessary because dynamic buffers - // are applied in order of increasing binding number in SetBindGroup. - return a.binding < b.binding; -} - // This is a utility function to help DAWN_ASSERT that the BGL-binding comparator places buffers // first. bool CheckBufferBindingsFirst(ityp::span<BindingIndex, const BindingInfo> bindings) { @@ -596,16 +566,16 @@ DAWN_ASSERT(mBindingInfo.size() <= kMaxBindingsPerPipelineLayoutTyped); // Compute various counts of expanded bindings and other metadata. + std::array<BindingIndex, Order_Count + 1> counts{}; for (const auto& binding : mBindingInfo) { MatchVariant( binding.bindingLayout, [&](const BufferBindingInfo& layout) { - mBufferCount++; if (layout.minBindingSize == 0) { mUnverifiedBufferCount++; } if (layout.hasDynamicOffset) { - mDynamicBufferCount++; + counts[Order_DynamicBuffer]++; switch (layout.type) { case wgpu::BufferBindingType::Storage: case kInternalStorageBufferBinding: @@ -619,17 +589,27 @@ case wgpu::BufferBindingType::Undefined: break; } + } else { + counts[Order_RegularBuffer]++; } }, - [&](const TextureBindingInfo&) {}, [&](const StorageTextureBindingInfo&) {}, - [&](const SamplerBindingInfo&) {}, + [&](const TextureBindingInfo&) { counts[Order_SampledTexture]++; }, + [&](const StorageTextureBindingInfo&) { counts[Order_StorageTexture]++; }, + [&](const SamplerBindingInfo&) { counts[Order_RegularSampler]++; }, [&](const StaticSamplerBindingInfo& layout) { - mStaticSamplerCount++; + counts[Order_StaticSampler]++; if (layout.isUsedForSingleTextureBinding) { mNeedsCrossBindingValidation = true; } }, - [&](const InputAttachmentBindingInfo&) {}); + [&](const InputAttachmentBindingInfo&) { counts[Order_InputAttachment]++; }); + } + + // Do a prefix sum to store the start offset of each binding type. + BindingIndex sum{0}; + for (auto [type, count] : Enumerate(counts)) { + mBindingTypeStart[type] = sum; + sum += count; } // Recompute the number of bindings of each type from the descriptor since that is used for @@ -647,6 +627,38 @@ } } +// static +bool BindGroupLayoutInternalBase::SortBindingsCompare(const BindingInfo& a, const BindingInfo& b) { + if (&a == &b) { + return false; + } + + // Buffers with dynamic offsets come first and then the rest of the buffers. Other bindings are + // only grouped by types. This is to make it easier and faster to handle them. + auto TypeOrder = [](const BindingInfo& info) { + return MatchVariant( + info.bindingLayout, + [&](const BufferBindingInfo& layout) { + return layout.hasDynamicOffset ? Order_DynamicBuffer : Order_RegularBuffer; + }, + [&](const TextureBindingInfo&) { return Order_SampledTexture; }, + [&](const StorageTextureBindingInfo&) { return Order_StorageTexture; }, + [&](const SamplerBindingInfo&) { return Order_RegularSampler; }, + [&](const StaticSamplerBindingInfo&) { return Order_StaticSampler; }, + [&](const InputAttachmentBindingInfo&) { return Order_InputAttachment; }); + }; + + auto aOrder = TypeOrder(a); + auto bOrder = TypeOrder(b); + if (aOrder != bOrder) { + return aOrder < bOrder; + } + + // Afterwards sort the bindings by binding number. This is necessary because dynamic buffers + // are applied in order of increasing binding number in SetBindGroup. + return a.binding < b.binding; +} + BindGroupLayoutInternalBase::BindGroupLayoutInternalBase( DeviceBase* device, const UnpackedPtr<BindGroupLayoutDescriptor>& descriptor) @@ -782,14 +794,9 @@ return mBindingInfo.size(); } -BindingIndex BindGroupLayoutInternalBase::GetBufferCount() const { - DAWN_ASSERT(!IsError()); - return BindingIndex(mBufferCount); -} - BindingIndex BindGroupLayoutInternalBase::GetDynamicBufferCount() const { DAWN_ASSERT(!IsError()); - return BindingIndex(mDynamicBufferCount); + return GetBindingTypeEnd(Order_DynamicBuffer) - GetBindingTypeStart(Order_DynamicBuffer); } uint32_t BindGroupLayoutInternalBase::GetDynamicStorageBufferCount() const { @@ -804,7 +811,8 @@ uint32_t BindGroupLayoutInternalBase::GetStaticSamplerCount() const { DAWN_ASSERT(!IsError()); - return mStaticSamplerCount; + return uint32_t(GetBindingTypeEnd(Order_StaticSampler) - + GetBindingTypeStart(Order_StaticSampler)); } const BindingCounts& BindGroupLayoutInternalBase::GetValidationBindingCounts() const { @@ -812,6 +820,23 @@ return mValidationBindingCounts; } +BeginEndRange<BindingIndex> BindGroupLayoutInternalBase::GetDynamicBufferIndices() const { + return Range(GetBindingTypeStart(Order_DynamicBuffer), GetBindingTypeEnd(Order_DynamicBuffer)); +} + +BeginEndRange<BindingIndex> BindGroupLayoutInternalBase::GetBufferIndices() const { + return Range(GetBindingTypeStart(Order_DynamicBuffer), GetBindingTypeEnd(Order_RegularBuffer)); +} + +BeginEndRange<BindingIndex> BindGroupLayoutInternalBase::GetStorageTextureIndices() const { + return Range(GetBindingTypeStart(Order_StorageTexture), + GetBindingTypeEnd(Order_StorageTexture)); +} + +BeginEndRange<BindingIndex> BindGroupLayoutInternalBase::GetSamplerIndices() const { + return Range(GetBindingTypeStart(Order_StaticSampler), GetBindingTypeEnd(Order_RegularSampler)); +} + const ExternalTextureBindingExpansionMap& BindGroupLayoutInternalBase::GetExternalTextureBindingExpansionMap() const { DAWN_ASSERT(!IsError()); @@ -835,8 +860,10 @@ // Followed by: // |---------buffer size array--------| // |-uint64_t[mUnverifiedBufferCount]-| - const uint64_t bindingCount = uint64_t(uint32_t(mBindingInfo.size())); - size_t objectPointerStart = mBufferCount * sizeof(BufferBindingData); + const size_t bufferCount = size_t(GetBindingTypeEnd(Order_RegularBuffer)); + const size_t bindingCount = size_t(mBindingInfo.size()); + + size_t objectPointerStart = bufferCount * sizeof(BufferBindingData); DAWN_ASSERT(IsAligned(objectPointerStart, alignof(Ref<ObjectBase>))); size_t bufferSizeArrayStart = Align(objectPointerStart + bindingCount * sizeof(Ref<ObjectBase>), sizeof(uint64_t)); @@ -846,9 +873,11 @@ BindGroupLayoutInternalBase::BindingDataPointers BindGroupLayoutInternalBase::ComputeBindingDataPointers(void* dataStart) const { - const uint64_t bindingCount = uint64_t(uint32_t(mBindingInfo.size())); + const size_t bufferCount = size_t(GetBindingTypeEnd(Order_RegularBuffer)); + const size_t bindingCount = size_t(mBindingInfo.size()); + BufferBindingData* bufferData = reinterpret_cast<BufferBindingData*>(dataStart); - auto bindings = reinterpret_cast<Ref<ObjectBase>*>(bufferData + mBufferCount); + auto bindings = reinterpret_cast<Ref<ObjectBase>*>(bufferData + bufferCount); uint64_t* unverifiedBufferSizes = AlignPtr(reinterpret_cast<uint64_t*>(bindings + bindingCount), sizeof(uint64_t)); @@ -856,13 +885,12 @@ DAWN_ASSERT(IsPtrAligned(bindings, alignof(Ref<ObjectBase>))); DAWN_ASSERT(IsPtrAligned(unverifiedBufferSizes, alignof(uint64_t))); - return {{bufferData, GetBufferCount()}, + return {{bufferData, GetBindingTypeEnd(Order_RegularBuffer)}, {bindings, GetBindingCount()}, {unverifiedBufferSizes, mUnverifiedBufferCount}}; } bool BindGroupLayoutInternalBase::IsStorageBufferBinding(BindingIndex bindingIndex) const { - DAWN_ASSERT(bindingIndex < GetBufferCount()); switch (std::get<BufferBindingInfo>(GetBindingInfo(bindingIndex).bindingLayout).type) { case wgpu::BufferBindingType::Uniform: return false; @@ -891,4 +919,12 @@ return entries; } +BindingIndex BindGroupLayoutInternalBase::GetBindingTypeStart(BindingTypeOrder type) const { + return mBindingTypeStart[type]; +} + +BindingIndex BindGroupLayoutInternalBase::GetBindingTypeEnd(BindingTypeOrder type) const { + return mBindingTypeStart[BindingTypeOrder(static_cast<uint32_t>(type) + 1)]; +} + } // namespace dawn::native
diff --git a/src/dawn/native/BindGroupLayoutInternal.h b/src/dawn/native/BindGroupLayoutInternal.h index 780c2f0..8008807 100644 --- a/src/dawn/native/BindGroupLayoutInternal.h +++ b/src/dawn/native/BindGroupLayoutInternal.h
@@ -36,6 +36,7 @@ #include "absl/container/flat_hash_map.h" #include "dawn/common/Constants.h" #include "dawn/common/ContentLessObjectCacheable.h" +#include "dawn/common/Range.h" #include "dawn/common/SlabAllocator.h" #include "dawn/common/ityp_span.h" #include "dawn/common/ityp_vector.h" @@ -89,15 +90,25 @@ bool HasBinding(BindingNumber bindingNumber) const; BindingIndex GetBindingIndex(BindingNumber bindingNumber) const; + BindingIndex GetBindingCount() const; + // Returns |BindingIndex| because dynamic buffers are packed at the front. + BindingIndex GetDynamicBufferCount() const; + uint32_t GetDynamicStorageBufferCount() const; + uint32_t GetUnverifiedBufferCount() const; + uint32_t GetStaticSamplerCount() const; + bool IsStorageBufferBinding(BindingIndex bindingIndex) const; + + // Returns the exact ranges of indices that contains specific binding types. + BeginEndRange<BindingIndex> GetDynamicBufferIndices() const; + BeginEndRange<BindingIndex> GetBufferIndices() const; + BeginEndRange<BindingIndex> GetStorageTextureIndices() const; + BeginEndRange<BindingIndex> GetSamplerIndices() const; + // Getters for the dynamic binding array. bool HasDynamicArray() const; BindingNumber GetDynamicArrayStart() const; wgpu::DynamicBindingKind GetDynamicArrayKind() const; - // Signals it's an appropriate time to free unused memory. BindGroupLayout implementations often - // have SlabAllocator<BindGroup> that need an external signal. - virtual void ReduceMemoryUsage(); - // Functions necessary for the unordered_set<BGLBase*>-based cache. size_t ComputeContentHash() override; @@ -107,15 +118,6 @@ }; bool IsEmpty() const; - BindingIndex GetBindingCount() const; - // Returns |BindingIndex| because buffers are packed at the front. - BindingIndex GetBufferCount() const; - // Returns |BindingIndex| because dynamic buffers are packed at the front. - BindingIndex GetDynamicBufferCount() const; - uint32_t GetDynamicStorageBufferCount() const; - uint32_t GetUnverifiedBufferCount() const; - uint32_t GetStaticSamplerCount() const; - // Used to get counts and validate them in pipeline layout creation. It might not match the // actual number of bindings stored as external textures are expanded. Other getters should be // used to get the stored counts. @@ -149,11 +151,13 @@ BindingDataPointers ComputeBindingDataPointers(void* dataStart) const; - bool IsStorageBufferBinding(BindingIndex bindingIndex) const; - // Returns a detailed string representation of the layout entries for use in error messages. std::string EntriesToString() const; + // Signals it's an appropriate time to free unused memory. BindGroupLayout implementations often + // have SlabAllocator<BindGroup> that need an external signal. + virtual void ReduceMemoryUsage(); + protected: void DestroyImpl() override; @@ -169,13 +173,37 @@ private: BindGroupLayoutInternalBase(DeviceBase* device, ObjectBase::ErrorTag tag, StringView label); + // The entries with arbitrary BindingNumber are repacked into a compact BindingIndex range. ityp::vector<BindingIndex, BindingInfo> mBindingInfo; - // Counts for how many such bindings are in mBindingInfo. + + // When they are packed, the entries are also sorted by type for more efficient lookup and + // iteration. This enum is the order that's used and can also be used to index various ranges of + // entries. + enum BindingTypeOrder : uint32_t { + // Buffers + Order_DynamicBuffer, + Order_RegularBuffer, + // Textures + Order_SampledTexture, + Order_StorageTexture, + Order_InputAttachment, + // Samplers + Order_StaticSampler, + Order_RegularSampler, + Order_Count, + }; + static bool SortBindingsCompare(const BindingInfo& a, const BindingInfo& b); + + // Keep a list of the start indices for each kind of binding. Then (exclusive) end of a range + // of bindings is the start of the next range. (that's why we use count + 1 entry, to have the + // "end" of the last binding type) + BindingIndex GetBindingTypeStart(BindingTypeOrder type) const; + BindingIndex GetBindingTypeEnd(BindingTypeOrder type) const; + std::array<BindingIndex, Order_Count + 1> mBindingTypeStart; + + // Additional counts for types of bindings. uint32_t mUnverifiedBufferCount = 0; - uint32_t mBufferCount = 0; - uint32_t mDynamicBufferCount = 0; uint32_t mDynamicStorageBufferCount = 0; - uint32_t mStaticSamplerCount = 0; // Map from BindGroupLayoutEntry.binding as BindingNumber to packed indices as BindingIndex. BindingMap mBindingMap;
diff --git a/src/dawn/native/CommandBufferStateTracker.cpp b/src/dawn/native/CommandBufferStateTracker.cpp index a17fb58..db590cf 100644 --- a/src/dawn/native/CommandBufferStateTracker.cpp +++ b/src/dawn/native/CommandBufferStateTracker.cpp
@@ -120,9 +120,8 @@ for (BindGroupIndex groupIndex : pipelineLayout->GetBindGroupLayoutsMask()) { BindGroupLayoutInternalBase* bgl = bindGroups[groupIndex]->GetLayout(); - for (BindingIndex bindingIndex{0}; bindingIndex < bgl->GetBufferCount(); ++bindingIndex) { + for (BindingIndex bindingIndex : bgl->GetBufferIndices()) { const BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex); - // Buffer bindings are sorted to have smallest of bindingIndex. const BufferBindingInfo& layout = std::get<BufferBindingInfo>(bindingInfo.bindingLayout); @@ -158,16 +157,11 @@ } // TODO(dawn:1642): optimize: precompute start/end range of storage textures bindings. - for (BindingIndex bindingIndex{bgl->GetBufferCount()}; - bindingIndex < bgl->GetBindingCount(); ++bindingIndex) { + for (BindingIndex bindingIndex : bgl->GetStorageTextureIndices()) { const BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex); + const auto& layout = std::get<StorageTextureBindingInfo>(bindingInfo.bindingLayout); - const auto* layout = std::get_if<StorageTextureBindingInfo>(&bindingInfo.bindingLayout); - if (layout == nullptr) { - continue; - } - - switch (layout->access) { + switch (layout.access) { case wgpu::StorageTextureAccess::WriteOnly: case wgpu::StorageTextureAccess::ReadWrite: break;
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp index b303899..8d49eba 100644 --- a/src/dawn/native/ShaderModule.cpp +++ b/src/dawn/native/ShaderModule.cpp
@@ -496,11 +496,11 @@ std::vector<uint64_t> requiredBufferSizes(layout->GetUnverifiedBufferCount()); uint32_t packedIdx = 0; - for (BindingIndex bindingIndex{0}; bindingIndex < layout->GetBufferCount(); ++bindingIndex) { + for (BindingIndex bindingIndex : layout->GetBufferIndices()) { const BindingInfo& bindingInfo = layout->GetBindingInfo(bindingIndex); - const auto* bufferBindingLayout = - std::get_if<BufferBindingInfo>(&bindingInfo.bindingLayout); - if (bufferBindingLayout == nullptr || bufferBindingLayout->minBindingSize > 0) { + const auto& bufferBindingLayout = std::get<BufferBindingInfo>(bindingInfo.bindingLayout); + + if (bufferBindingLayout.minBindingSize > 0) { // Skip bindings that have minimum buffer size set in the layout continue; }
diff --git a/src/dawn/native/d3d12/BindGroupD3D12.cpp b/src/dawn/native/d3d12/BindGroupD3D12.cpp index 8a057aa..22dbf99 100644 --- a/src/dawn/native/d3d12/BindGroupD3D12.cpp +++ b/src/dawn/native/d3d12/BindGroupD3D12.cpp
@@ -214,8 +214,7 @@ // of BindingNumber. mDynamicStorageBufferLengths.resize(bgl->GetDynamicStorageBufferCount()); uint32_t dynamicStorageBufferIndex = 0; - for (BindingIndex bindingIndex(0); bindingIndex < bgl->GetDynamicBufferCount(); - ++bindingIndex) { + for (BindingIndex bindingIndex : bgl->GetDynamicBufferIndices()) { if (bgl->IsStorageBufferBinding(bindingIndex)) { mDynamicStorageBufferLengths[dynamicStorageBufferIndex++] = static_cast<uint32_t>(GetBindingAsBufferBinding(bindingIndex).size);
diff --git a/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp b/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp index 150a16f..d52b32a 100644 --- a/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp +++ b/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp
@@ -224,9 +224,7 @@ // Init root descriptors in root signatures for dynamic buffer bindings. // These are packed at the beginning of the layout binding info. mDynamicRootParameterIndices[group].resize(bindGroupLayout->GetDynamicBufferCount()); - for (BindingIndex dynamicBindingIndex{0}; - dynamicBindingIndex < bindGroupLayout->GetDynamicBufferCount(); - ++dynamicBindingIndex) { + for (BindingIndex dynamicBindingIndex : bindGroupLayout->GetDynamicBufferIndices()) { const BindingInfo& bindingInfo = bindGroupLayout->GetBindingInfo(dynamicBindingIndex); if (bindingInfo.visibility == wgpu::ShaderStage::None) { @@ -303,8 +301,7 @@ mDynamicStorageBufferLengthInfo[group].bindingAndRegisterOffsets.reserve( bgl->GetDynamicStorageBufferCount()); - for (BindingIndex bindingIndex(0); bindingIndex < bgl->GetDynamicBufferCount(); - ++bindingIndex) { + for (BindingIndex bindingIndex : bgl->GetDynamicBufferIndices()) { if (bgl->IsStorageBufferBinding(bindingIndex)) { mDynamicStorageBufferLengthInfo[group].bindingAndRegisterOffsets.push_back( {bgl->GetBindingInfo(bindingIndex).binding,
diff --git a/src/dawn/native/d3d12/SamplerHeapCacheD3D12.cpp b/src/dawn/native/d3d12/SamplerHeapCacheD3D12.cpp index 1eea3ac..f6216f4 100644 --- a/src/dawn/native/d3d12/SamplerHeapCacheD3D12.cpp +++ b/src/dawn/native/d3d12/SamplerHeapCacheD3D12.cpp
@@ -120,12 +120,8 @@ std::vector<Sampler*> samplers; samplers.reserve(samplerCount); - for (BindingIndex bindingIndex = bgl->GetDynamicBufferCount(); - bindingIndex < bgl->GetBindingCount(); ++bindingIndex) { - const BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex); - if (std::holds_alternative<SamplerBindingInfo>(bindingInfo.bindingLayout)) { - samplers.push_back(ToBackend(group->GetBindingAsSampler(bindingIndex))); - } + for (BindingIndex bindingIndex : bgl->GetSamplerIndices()) { + samplers.push_back(ToBackend(group->GetBindingAsSampler(bindingIndex))); } // Check the cache if there exists a sampler heap allocation that corresponds to the