From b21ccebac882139ab7741066913c561c035ef3f9 Mon Sep 17 00:00:00 2001
From: Michael Tang <tangm@microsoft.com>
Date: Fri, 10 Sep 2021 23:16:33 +0000
Subject: [PATCH] D3D12: Use a separate space for the index offset constants

Tint's HLSL writer can now map sets to spaces, so we can use a dedicated
space  and register to store the index offset constants. Currently, we
use the first space (0) and find a free unused register.

Bug: dawn:1103
Change-Id: Ie1fe95ec314097baf0e0952a0c78222cad493c95
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/63223
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Michael Tang <tangm@microsoft.com>
---
 src/dawn_native/d3d12/PipelineLayoutD3D12.cpp | 29 +++----------------
 src/dawn_native/d3d12/PipelineLayoutD3D12.h   |  7 +++--
 2 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp b/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp
index 448ec22c2f..372b61b6d4 100644
--- a/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp
+++ b/src/dawn_native/d3d12/PipelineLayoutD3D12.cpp
@@ -160,27 +160,6 @@ namespace dawn_native { namespace d3d12 {
         // |ranges| will have resized and the pointers in the |rootParameter|s will be invalid.
         ASSERT(rangeIndex == rangesCount);
 
-        // Since Tint's HLSL writer doesn't currently map sets to spaces, we use the default space
-        // (0).
-        mFirstIndexOffsetRegisterSpace = 0;
-        BindGroupIndex firstOffsetGroup{mFirstIndexOffsetRegisterSpace};
-        if (GetBindGroupLayoutsMask()[firstOffsetGroup]) {
-            // Find the last register used on firstOffsetGroup.
-            auto bgl = ToBackend(GetBindGroupLayout(firstOffsetGroup));
-            uint32_t maxRegister = 0;
-            for (BindingIndex bindingIndex{0}; bindingIndex < bgl->GetBindingCount();
-                 ++bindingIndex) {
-                uint32_t shaderRegister = bgl->GetShaderRegister(bindingIndex);
-                if (shaderRegister > maxRegister) {
-                    maxRegister = shaderRegister;
-                }
-            }
-            mFirstIndexOffsetShaderRegister = maxRegister + 1;
-        } else {
-            // firstOffsetGroup is not in use, we can use the first register.
-            mFirstIndexOffsetShaderRegister = 0;
-        }
-
         D3D12_ROOT_PARAMETER indexOffsetConstants{};
         indexOffsetConstants.ShaderVisibility = D3D12_SHADER_VISIBILITY_VERTEX;
         indexOffsetConstants.ParameterType = D3D12_ROOT_PARAMETER_TYPE_32BIT_CONSTANTS;
@@ -188,8 +167,8 @@ namespace dawn_native { namespace d3d12 {
         // NOTE: We should consider delaying root signature creation until we know how many values
         // we need
         indexOffsetConstants.Constants.Num32BitValues = 2;
-        indexOffsetConstants.Constants.RegisterSpace = mFirstIndexOffsetRegisterSpace;
-        indexOffsetConstants.Constants.ShaderRegister = mFirstIndexOffsetShaderRegister;
+        indexOffsetConstants.Constants.RegisterSpace = kReservedRegisterSpace;
+        indexOffsetConstants.Constants.ShaderRegister = kFirstOffsetInfoBaseRegister;
         mFirstIndexOffsetParameterIndex = rootParameters.size();
         // NOTE: We should consider moving this entry to earlier in the root signature since offsets
         // would need to be updated often
@@ -251,11 +230,11 @@ namespace dawn_native { namespace d3d12 {
     }
 
     uint32_t PipelineLayout::GetFirstIndexOffsetRegisterSpace() const {
-        return mFirstIndexOffsetRegisterSpace;
+        return kReservedRegisterSpace;
     }
 
     uint32_t PipelineLayout::GetFirstIndexOffsetShaderRegister() const {
-        return mFirstIndexOffsetShaderRegister;
+        return kFirstOffsetInfoBaseRegister;
     }
 
     uint32_t PipelineLayout::GetFirstIndexOffsetParameterIndex() const {
diff --git a/src/dawn_native/d3d12/PipelineLayoutD3D12.h b/src/dawn_native/d3d12/PipelineLayoutD3D12.h
index f20923543b..b1efc0d00f 100644
--- a/src/dawn_native/d3d12/PipelineLayoutD3D12.h
+++ b/src/dawn_native/d3d12/PipelineLayoutD3D12.h
@@ -15,6 +15,7 @@
 #ifndef DAWNNATIVE_D3D12_PIPELINELAYOUTD3D12_H_
 #define DAWNNATIVE_D3D12_PIPELINELAYOUTD3D12_H_
 
+#include "common/Constants.h"
 #include "common/ityp_array.h"
 #include "dawn_native/BindingInfo.h"
 #include "dawn_native/PipelineLayout.h"
@@ -22,6 +23,10 @@
 
 namespace dawn_native { namespace d3d12 {
 
+    // We reserve a register space that a user cannot use.
+    static constexpr uint32_t kReservedRegisterSpace = kMaxBindGroups + 1;
+    static constexpr uint32_t kFirstOffsetInfoBaseRegister = 0;
+
     class Device;
 
     class PipelineLayout final : public PipelineLayoutBase {
@@ -53,8 +58,6 @@ namespace dawn_native { namespace d3d12 {
                     ityp::array<BindingIndex, uint32_t, kMaxDynamicBuffersPerPipelineLayout>,
                     kMaxBindGroups>
             mDynamicRootParameterIndices;
-        uint32_t mFirstIndexOffsetRegisterSpace;
-        uint32_t mFirstIndexOffsetShaderRegister;
         uint32_t mFirstIndexOffsetParameterIndex;
         ComPtr<ID3D12RootSignature> mRootSignature;
     };