From 3009d0bd7d30b341a8697d85d9d42a9ee4910d19 Mon Sep 17 00:00:00 2001
From: Liam <byteslice@airmail.cc>
Date: Thu, 17 Mar 2022 14:45:38 -0400
Subject: [PATCH] Address review comments

---
 .../spirv/emit_spirv_context_get_set.cpp      | 15 ++---
 .../backend/spirv/spirv_emit_context.cpp      | 11 +---
 .../ir_opt/collect_shader_info_pass.cpp       | 61 +++++++++----------
 src/shader_recompiler/shader_info.h           |  1 +
 4 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/src/shader_recompiler/backend/spirv/emit_spirv_context_get_set.cpp b/src/shader_recompiler/backend/spirv/emit_spirv_context_get_set.cpp
index 9f6d5e3a5a..80b4bbd277 100644
--- a/src/shader_recompiler/backend/spirv/emit_spirv_context_get_set.cpp
+++ b/src/shader_recompiler/backend/spirv/emit_spirv_context_get_set.cpp
@@ -125,7 +125,6 @@ std::optional<OutAttr> OutputAttrPointer(EmitContext& ctx, IR::Attribute attr) {
 Id GetCbuf(EmitContext& ctx, Id result_type, Id UniformDefinitions::*member_ptr, u32 element_size,
            const IR::Value& binding, const IR::Value& offset, const Id indirect_func) {
     Id buffer_offset;
-
     const Id uniform_type{ctx.uniform_types.*member_ptr};
     if (offset.IsImmediate()) {
         // Hardware been proved to read the aligned offset (e.g. LDC.U32 at 6 will read offset 4)
@@ -138,16 +137,12 @@ Id GetCbuf(EmitContext& ctx, Id result_type, Id UniformDefinitions::*member_ptr,
     } else {
         buffer_offset = ctx.Def(offset);
     }
-
-    if (binding.IsImmediate()) {
-        const Id cbuf{ctx.cbufs[binding.U32()].*member_ptr};
-        const Id access_chain{
-            ctx.OpAccessChain(uniform_type, cbuf, ctx.u32_zero_value, buffer_offset)};
-        return ctx.OpLoad(result_type, access_chain);
-    } else {
-        const std::array<Id, 2> arguments{ctx.Def(binding), buffer_offset};
-        return ctx.OpFunctionCall(result_type, indirect_func, arguments);
+    if (!binding.IsImmediate()) {
+        return ctx.OpFunctionCall(result_type, indirect_func, ctx.Def(binding), buffer_offset);
     }
+    const Id cbuf{ctx.cbufs[binding.U32()].*member_ptr};
+    const Id access_chain{ctx.OpAccessChain(uniform_type, cbuf, ctx.u32_zero_value, buffer_offset)};
+    return ctx.OpLoad(result_type, access_chain);
 }
 
 Id GetCbufU32(EmitContext& ctx, const IR::Value& binding, const IR::Value& offset) {
diff --git a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp
index f85541a2e2..aa5b6c9b73 100644
--- a/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp
+++ b/src/shader_recompiler/backend/spirv/spirv_emit_context.cpp
@@ -994,7 +994,7 @@ void EmitContext::DefineConstantBuffers(const Info& info, u32& binding) {
         }
         return;
     }
-    IR::Type types{info.used_constant_buffer_types};
+    IR::Type types{info.used_constant_buffer_types | info.used_indirect_cbuf_types};
     if (True(types & IR::Type::U8)) {
         if (profile.support_int8) {
             DefineConstBuffers(*this, info, &UniformDefinitions::U8, binding, U8, 'u', sizeof(u8));
@@ -1032,7 +1032,6 @@ void EmitContext::DefineConstantBufferIndirectFunctions(const Info& info) {
     if (!info.uses_cbuf_indirect) {
         return;
     }
-
     const auto make_accessor{[&](Id buffer_type, Id UniformDefinitions::*member_ptr) {
         const Id func_type{TypeFunction(buffer_type, U32[1], U32[1])};
         const Id func{OpFunction(buffer_type, spv::FunctionControlMask::MaskNone, func_type)};
@@ -1050,10 +1049,8 @@ void EmitContext::DefineConstantBufferIndirectFunctions(const Info& info) {
             buf_labels[i] = OpLabel();
             buf_literals[i] = Sirit::Literal{i};
         }
-
         OpSelectionMerge(merge_label, spv::SelectionControlMask::MaskNone);
         OpSwitch(binding, buf_labels[0], buf_literals, buf_labels);
-
         for (u32 i = 0; i < Info::MAX_CBUFS; i++) {
             AddLabel(buf_labels[i]);
             const Id cbuf{cbufs[i].*member_ptr};
@@ -1061,16 +1058,12 @@ void EmitContext::DefineConstantBufferIndirectFunctions(const Info& info) {
             const Id result{OpLoad(buffer_type, access_chain)};
             OpReturnValue(result);
         }
-
         AddLabel(merge_label);
         OpUnreachable();
         OpFunctionEnd();
-
         return func;
     }};
-
-    IR::Type types{info.used_constant_buffer_types};
-
+    IR::Type types{info.used_indirect_cbuf_types};
     if (True(types & IR::Type::U8)) {
         load_const_func_u8 = make_accessor(U8, &UniformDefinitions::U8);
     }
diff --git a/src/shader_recompiler/ir_opt/collect_shader_info_pass.cpp b/src/shader_recompiler/ir_opt/collect_shader_info_pass.cpp
index b54894a9b5..0b2c608427 100644
--- a/src/shader_recompiler/ir_opt/collect_shader_info_pass.cpp
+++ b/src/shader_recompiler/ir_opt/collect_shader_info_pass.cpp
@@ -45,6 +45,30 @@ void AddRegisterIndexedLdc(Info& info) {
     }
 }
 
+u32 GetElementSize(IR::Type& used_type, Shader::IR::Opcode opcode) {
+    switch (opcode) {
+    case IR::Opcode::GetCbufU8:
+    case IR::Opcode::GetCbufS8:
+        used_type |= IR::Type::U8;
+        return 1;
+    case IR::Opcode::GetCbufU16:
+    case IR::Opcode::GetCbufS16:
+        used_type |= IR::Type::U16;
+        return 2;
+    case IR::Opcode::GetCbufU32:
+        used_type |= IR::Type::U32;
+        return 4;
+    case IR::Opcode::GetCbufF32:
+        used_type |= IR::Type::F32;
+        return 4;
+    case IR::Opcode::GetCbufU32x2:
+        used_type |= IR::Type::U32x2;
+        return 8;
+    default:
+        throw InvalidArgument("Invalid opcode {}", opcode);
+    }
+}
+
 void GetPatch(Info& info, IR::Patch patch) {
     if (!IR::IsGeneric(patch)) {
         throw NotImplementedException("Reading non-generic patch {}", patch);
@@ -481,45 +505,16 @@ void VisitUsages(Info& info, IR::Inst& inst) {
         const IR::Value offset{inst.Arg(1)};
         if (index.IsImmediate()) {
             AddConstantBufferDescriptor(info, index.U32(), 1);
-        } else {
-            AddRegisterIndexedLdc(info);
-        }
-
-        u32 element_size{};
-        switch (inst.GetOpcode()) {
-        case IR::Opcode::GetCbufU8:
-        case IR::Opcode::GetCbufS8:
-            info.used_constant_buffer_types |= IR::Type::U8;
-            element_size = 1;
-            break;
-        case IR::Opcode::GetCbufU16:
-        case IR::Opcode::GetCbufS16:
-            info.used_constant_buffer_types |= IR::Type::U16;
-            element_size = 2;
-            break;
-        case IR::Opcode::GetCbufU32:
-            info.used_constant_buffer_types |= IR::Type::U32;
-            element_size = 4;
-            break;
-        case IR::Opcode::GetCbufF32:
-            info.used_constant_buffer_types |= IR::Type::F32;
-            element_size = 4;
-            break;
-        case IR::Opcode::GetCbufU32x2:
-            info.used_constant_buffer_types |= IR::Type::U32x2;
-            element_size = 8;
-            break;
-        default:
-            break;
-        }
-
-        if (index.IsImmediate()) {
+            u32 element_size = GetElementSize(info.used_constant_buffer_types, inst.GetOpcode());
             u32& size{info.constant_buffer_used_sizes[index.U32()]};
             if (offset.IsImmediate()) {
                 size = Common::AlignUp(std::max(size, offset.U32() + element_size), 16u);
             } else {
                 size = 0x10'000;
             }
+        } else {
+            AddRegisterIndexedLdc(info);
+            GetElementSize(info.used_indirect_cbuf_types, inst.GetOpcode());
         }
         break;
     }
diff --git a/src/shader_recompiler/shader_info.h b/src/shader_recompiler/shader_info.h
index 9cff2e42d9..9d36bd9ebc 100644
--- a/src/shader_recompiler/shader_info.h
+++ b/src/shader_recompiler/shader_info.h
@@ -177,6 +177,7 @@ struct Info {
 
     IR::Type used_constant_buffer_types{};
     IR::Type used_storage_buffer_types{};
+    IR::Type used_indirect_cbuf_types{};
 
     u32 constant_buffer_mask{};
     std::array<u32, MAX_CBUFS> constant_buffer_used_sizes{};