Address review comments
This commit is contained in:
		| @@ -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, | 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) { |            const IR::Value& binding, const IR::Value& offset, const Id indirect_func) { | ||||||
|     Id buffer_offset; |     Id buffer_offset; | ||||||
|  |  | ||||||
|     const Id uniform_type{ctx.uniform_types.*member_ptr}; |     const Id uniform_type{ctx.uniform_types.*member_ptr}; | ||||||
|     if (offset.IsImmediate()) { |     if (offset.IsImmediate()) { | ||||||
|         // Hardware been proved to read the aligned offset (e.g. LDC.U32 at 6 will read offset 4) |         // 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 { |     } else { | ||||||
|         buffer_offset = ctx.Def(offset); |         buffer_offset = ctx.Def(offset); | ||||||
|     } |     } | ||||||
|  |     if (!binding.IsImmediate()) { | ||||||
|     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); |  | ||||||
|     } else { |  | ||||||
|         const std::array<Id, 2> arguments{ctx.Def(binding), buffer_offset}; |  | ||||||
|         return ctx.OpFunctionCall(result_type, indirect_func, arguments); |  | ||||||
|     } |     } | ||||||
|  |     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) { | Id GetCbufU32(EmitContext& ctx, const IR::Value& binding, const IR::Value& offset) { | ||||||
|   | |||||||
| @@ -994,7 +994,7 @@ void EmitContext::DefineConstantBuffers(const Info& info, u32& binding) { | |||||||
|         } |         } | ||||||
|         return; |         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 (True(types & IR::Type::U8)) { | ||||||
|         if (profile.support_int8) { |         if (profile.support_int8) { | ||||||
|             DefineConstBuffers(*this, info, &UniformDefinitions::U8, binding, U8, 'u', sizeof(u8)); |             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) { |     if (!info.uses_cbuf_indirect) { | ||||||
|         return; |         return; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     const auto make_accessor{[&](Id buffer_type, Id UniformDefinitions::*member_ptr) { |     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_type{TypeFunction(buffer_type, U32[1], U32[1])}; | ||||||
|         const Id func{OpFunction(buffer_type, spv::FunctionControlMask::MaskNone, func_type)}; |         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_labels[i] = OpLabel(); | ||||||
|             buf_literals[i] = Sirit::Literal{i}; |             buf_literals[i] = Sirit::Literal{i}; | ||||||
|         } |         } | ||||||
|  |  | ||||||
|         OpSelectionMerge(merge_label, spv::SelectionControlMask::MaskNone); |         OpSelectionMerge(merge_label, spv::SelectionControlMask::MaskNone); | ||||||
|         OpSwitch(binding, buf_labels[0], buf_literals, buf_labels); |         OpSwitch(binding, buf_labels[0], buf_literals, buf_labels); | ||||||
|  |  | ||||||
|         for (u32 i = 0; i < Info::MAX_CBUFS; i++) { |         for (u32 i = 0; i < Info::MAX_CBUFS; i++) { | ||||||
|             AddLabel(buf_labels[i]); |             AddLabel(buf_labels[i]); | ||||||
|             const Id cbuf{cbufs[i].*member_ptr}; |             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)}; |             const Id result{OpLoad(buffer_type, access_chain)}; | ||||||
|             OpReturnValue(result); |             OpReturnValue(result); | ||||||
|         } |         } | ||||||
|  |  | ||||||
|         AddLabel(merge_label); |         AddLabel(merge_label); | ||||||
|         OpUnreachable(); |         OpUnreachable(); | ||||||
|         OpFunctionEnd(); |         OpFunctionEnd(); | ||||||
|  |  | ||||||
|         return func; |         return func; | ||||||
|     }}; |     }}; | ||||||
|  |     IR::Type types{info.used_indirect_cbuf_types}; | ||||||
|     IR::Type types{info.used_constant_buffer_types}; |  | ||||||
|  |  | ||||||
|     if (True(types & IR::Type::U8)) { |     if (True(types & IR::Type::U8)) { | ||||||
|         load_const_func_u8 = make_accessor(U8, &UniformDefinitions::U8); |         load_const_func_u8 = make_accessor(U8, &UniformDefinitions::U8); | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -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) { | void GetPatch(Info& info, IR::Patch patch) { | ||||||
|     if (!IR::IsGeneric(patch)) { |     if (!IR::IsGeneric(patch)) { | ||||||
|         throw NotImplementedException("Reading non-generic patch {}", 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)}; |         const IR::Value offset{inst.Arg(1)}; | ||||||
|         if (index.IsImmediate()) { |         if (index.IsImmediate()) { | ||||||
|             AddConstantBufferDescriptor(info, index.U32(), 1); |             AddConstantBufferDescriptor(info, index.U32(), 1); | ||||||
|         } else { |             u32 element_size = GetElementSize(info.used_constant_buffer_types, inst.GetOpcode()); | ||||||
|             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& size{info.constant_buffer_used_sizes[index.U32()]}; |             u32& size{info.constant_buffer_used_sizes[index.U32()]}; | ||||||
|             if (offset.IsImmediate()) { |             if (offset.IsImmediate()) { | ||||||
|                 size = Common::AlignUp(std::max(size, offset.U32() + element_size), 16u); |                 size = Common::AlignUp(std::max(size, offset.U32() + element_size), 16u); | ||||||
|             } else { |             } else { | ||||||
|                 size = 0x10'000; |                 size = 0x10'000; | ||||||
|             } |             } | ||||||
|  |         } else { | ||||||
|  |             AddRegisterIndexedLdc(info); | ||||||
|  |             GetElementSize(info.used_indirect_cbuf_types, inst.GetOpcode()); | ||||||
|         } |         } | ||||||
|         break; |         break; | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -177,6 +177,7 @@ struct Info { | |||||||
|  |  | ||||||
|     IR::Type used_constant_buffer_types{}; |     IR::Type used_constant_buffer_types{}; | ||||||
|     IR::Type used_storage_buffer_types{}; |     IR::Type used_storage_buffer_types{}; | ||||||
|  |     IR::Type used_indirect_cbuf_types{}; | ||||||
|  |  | ||||||
|     u32 constant_buffer_mask{}; |     u32 constant_buffer_mask{}; | ||||||
|     std::array<u32, MAX_CBUFS> constant_buffer_used_sizes{}; |     std::array<u32, MAX_CBUFS> constant_buffer_used_sizes{}; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user