From ab4b10cf486bea8889bdfd99dfe75488711e4e87 Mon Sep 17 00:00:00 2001 From: nmzik Date: Sun, 27 Sep 2026 21:42:59 +0400 Subject: [PATCH] shader: fix LDS counter bounds --- .../backend/spirv/spirvEmitterMemory.cpp | 39 ++++--- .../recompiler/frontend/translate/Memory.cpp | 4 +- .../recompiler/ir/opcodes/ValueOpcodes.inc | 4 +- tests/ShaderRecompilerComputeTests.cpp | 106 +++++++++++++++++- 4 files changed, 129 insertions(+), 24 deletions(-) diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp index 7b9314a97..8e10b4266 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp @@ -1104,16 +1104,27 @@ uint32_t EmitAppendConsume(ValueEmitContext& ctx, const IR::Inst& inst) { if (ctx.half == 1) { return ctx.other_half->Def(IR::Value(const_cast(&inst))); } - const auto m0 = ctx.Arg(inst, 0); - const auto base = - Binary(state, spv::OpShiftRightLogical, TypeU32(state), m0, ConstantU32(state, 16)); - const auto size = - Binary(state, spv::OpBitwiseAnd, TypeU32(state), m0, ConstantU32(state, 0xffffu)); - const auto address = Binary(state, spv::OpIAdd, TypeU32(state), base, - ConstantU32(state, ctx.Memory(inst).offset)); + const auto mem = ctx.Memory(inst); + const auto offset = mem.offset & 0xfffcu; + auto address = ConstantU32(state, offset); + uint32_t region_bounds = 0; + if (mem.kind == IR::ResourceKind::Gds) { + const auto m0 = ctx.Arg(inst, 0); + const auto base = + Binary(state, spv::OpShiftRightLogical, TypeU32(state), m0, ConstantU32(state, 16)); + const auto size = + Binary(state, spv::OpBitwiseAnd, TypeU32(state), m0, ConstantU32(state, 0xffffu)); + address = Binary(state, spv::OpIAdd, TypeU32(state), base, address); + // The entire M0 region must fit the PS5's 48 KiB GDS partition. + region_bounds = AndCondition( + state, Binary(state, spv::OpULessThan, TypeBool(state), + ConstantU32(state, offset + 3u), size), + Binary(state, spv::OpULessThanEqual, TypeBool(state), + Binary(state, spv::OpIAdd, TypeU32(state), base, size), + ConstantU32(state, 0xc000u))); + } const auto raw_index = Binary(state, spv::OpShiftRightLogical, TypeU32(state), address, ConstantU32(state, 2)); - const auto mem = ctx.Memory(inst); const auto access = PrepareMemoryResourceAccess(state, mem); const auto index = EmitMemoryElementIndex(state, access, raw_index); const auto exec = ctx.Arg(inst, 1); @@ -1132,19 +1143,17 @@ uint32_t EmitAppendConsume(ValueEmitContext& ctx, const IR::Inst& inst) { : first; const auto is_first = Binary(state, spv::OpIEqual, TypeBool(state), EmitSubgroupLocalInvocationId(state), source_lane); - const auto storage_bounds = EmitMemoryElementInBounds(state, access, index); - const auto m0_bounds = - mem.kind == IR::ResourceKind::Gds - ? Binary(state, spv::OpINotEqual, TypeBool(state), size, ConstantU32(state, 0)) - : Binary(state, spv::OpULessThan, TypeBool(state), - ConstantU32(state, ctx.Memory(inst).offset + 3u), size); + auto bounds = EmitMemoryElementInBounds(state, access, index); + if (mem.kind == IR::ResourceKind::Gds) { + bounds = AndCondition(state, bounds, region_bounds); + } const auto condition = AndCondition( state, is_first, AndCondition(state, state.lane_count == 2 ? Binary(state, spv::OpINotEqual, TypeBool(state), count, ConstantU32(state, 0)) : exec, - AndCondition(state, storage_bounds, m0_bounds))); + bounds)); const auto atomic = EmitValueOrZeroIfCondition(state, condition, [&]() { const auto value = state.builder.AllocateId(); state.builder.AddFunction(append ? spv::OpAtomicIAdd : spv::OpAtomicISub, TypeU32(state), diff --git a/src/graphics/shader/recompiler/frontend/translate/Memory.cpp b/src/graphics/shader/recompiler/frontend/translate/Memory.cpp index 95e12c0c0..db7de4002 100644 --- a/src/graphics/shader/recompiler/frontend/translate/Memory.cpp +++ b/src/graphics/shader/recompiler/frontend/translate/Memory.cpp @@ -235,6 +235,7 @@ Decoder::Operand MemorySourceAt(const Decoder::Instruction& decoded, uint32_t in return index == 0u ? decoded.src0 : MakeImmediate(decoded.offset & 0xffffu); case Decoder::Opcode::DS_CONSUME: case Decoder::Opcode::DS_APPEND: + return decoded.gds ? MakeM0Operand() : MakeImmediate(0); case Decoder::Opcode::DS_READ_ADDTID_B32: return MakeM0Operand(); case Decoder::Opcode::DS_WRITE_ADDTID_B32: return index == 0u ? decoded.src1 : MakeM0Operand(); @@ -845,8 +846,7 @@ void Translator::DS_MINMAX_F32(const Decoder::Instruction& inst, IR::ValueOpcode void Translator::DS_APPEND_CONSUME(const Decoder::Instruction& inst, IR::ValueOpcode opcode) { const auto memory = MemoryInfoFromDecoded(inst); WriteOperand(inst.dst, ir.Emit(opcode, - {ReadU32(MemorySourceAt(inst, 0)), ir.GetExec(), ir.GetExecLo(), - ir.GetExecHi()}, + {ReadU32(MemorySourceAt(inst, 0)), ir.GetExec()}, AddMemoryInfo(memory, inst.pc))); } diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc index 74c3da30f..768679402 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc @@ -275,8 +275,8 @@ VALUE_OPCODE(SharedAtomicUMax32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicAnd32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicOr32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicXor32, U32, U32, U32, U1) -VALUE_OPCODE(DataAppend, U32, U32, U1, U32, U32) -VALUE_OPCODE(DataConsume, U32, U32, U1, U32, U32) +VALUE_OPCODE(DataAppend, U32, U32, U1) +VALUE_OPCODE(DataConsume, U32, U32, U1) VALUE_OPCODE(SwizzleU32, U32, U32, U32, U1) VALUE_OPCODE(ImageQueryDimensions, U32x4, ImageResource, ImageAddress) diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index edede023c..dae62a3c2 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -27279,17 +27279,17 @@ TestCase DsAppendConsumeUsesEncodedLdsSelector() { using O = ShaderOpcode; std::vector code; - AppendSMovLiteral(&code, 124, 0x0000ffffu); + AppendSMovLiteral(&code, 124, 0xffff0000u); AppendVMovU32(&code, 1, 0); AppendVMovU32(&code, 2, 10); code.push_back(EncodeDs0(0x0d, 0)); code.push_back(EncodeDs1(0, 2, 1)); - code.push_back(EncodeDs0(0x3e, 0)); + code.push_back(EncodeDs0(0x3e, 1)); code.push_back(EncodeDs1(3, 0, 0)); - code.push_back(EncodeDs0(0x3d, 0)); + code.push_back(EncodeDs0(0x3d, 2)); code.push_back(EncodeDs1(4, 0, 0)); AppendSMovLiteral(&code, 124, 0); - code.push_back(EncodeDs0(0x3e, 0)); + code.push_back(EncodeDs0(0x3e, 3)); code.push_back(EncodeDs1(5, 0, 0)); code.push_back(EncodeDs0(0x36, 0)); code.push_back(EncodeDs1(6, 0, 1)); @@ -27302,11 +27302,64 @@ TestCase DsAppendConsumeUsesEncodedLdsSelector() { return {"DsAppendConsumeLdsSelector", code, {}, - {10, 11, 0, 10}, + {10, 11, 10, 11}, {O::S_MOV_B32, O::V_MOV_B32, O::DS_WRITE_B32, O::DS_READ_B32, O::DS_APPEND, O::DS_CONSUME, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } +TestCase DsAppendAllocatesAcrossWaves(u32 wave_size) { + using O = ShaderOpcode; + TestCase test; + test.name = wave_size == 64 ? "DsAppendAllocatesAcrossWaves64" + : "DsAppendAllocatesAcrossWaves32"; + auto &code = test.code; + AppendVMovU32(&code, 2, 0); + AppendVMovU32(&code, 5, 1); + // Only workgroup lane zero initializes the shared allocation counter. + code.push_back(EncodeSop1(0x04, 10, 126)); + code.push_back(EncodeVopc(0xc2, InlineU32(0), 0)); + code.push_back(EncodeSop1(0x04, 126, 106)); + code.push_back(EncodeDs0(0x0d, 0)); + code.push_back(EncodeDs1(0, 2, 2)); + code.push_back(EncodeSop1(0x04, 126, 10)); + code.push_back(EncodeSopp(0x0a, 0)); + for (u32 batch = 0; batch < 2; ++batch) { + // Captured CS ea0aceac sets M0=0 and uses OFFSET=1. LDS ignores M0 + // and aligns OFFSET before allocating once for every active wave. + AppendSMovLiteral(&code, 124, batch == 0 ? 0 : 0xffff0000u); + code.push_back(EncodeDs0(0x3e, batch == 0 ? 1 : 3)); + code.push_back(EncodeDs1(3, 0, 0)); + code.push_back(EncodeVop2(0x24, 6, 127, 3)); + code.push_back(EncodeVop2(0x23, 6, 126, 6)); + code.push_back(EncodeVop2(0x1a, 7, InlineU32(2), 6)); + // A histogram detects duplicate allocations without assuming wave order. + AppendBufferStoreOpcode(&code, 0x32, 5, 7); + } + code.push_back(EncodeSopp(0x0a, 0)); + code.push_back(EncodeDs0(0x36, 0)); + code.push_back(EncodeDs1(8, 0, 2)); + AppendStoreVgprAtLaneDwordOffset(&code, 8, 0, 512); + AppendEnd(&code); + test.expected.assign(512, 1); + test.expected.resize(768, 512); + test.initial.resize(test.expected.size()); + test.compute_info.threads_num[0] = 256; + test.compute_info.threads_num[1] = 1; + test.compute_info.threads_num[2] = 1; + test.compute_info.thread_ids_num = 1; + test.compute_info.wave_size = wave_size; + test.compute_info.lds_size_dwords = 1; + test.has_compute_info = true; + test.forbidden_spirv = {"\"gds\""}; + test.opcodes = {O::S_MOV_B32, O::S_MOV_B64, O::S_BARRIER, + O::V_MOV_B32, O::V_CMP_EQ_U32, O::DS_WRITE_B32, + O::DS_APPEND, O::V_MBCNT_HI_U32_B32, + O::V_MBCNT_LO_U32_B32, O::V_LSHLREV_B32, + O::BUFFER_ATOMIC_ADD, O::DS_READ_B32, + O::BUFFER_STORE_DWORD, O::S_ENDPGM}; + return test; +} + TestCase DsAppendUsesEncodedGdsSelector() { using O = ShaderOpcode; @@ -27344,6 +27397,46 @@ TestCase DsAppendUsesEncodedGdsSelector() { return test; } +TestCase DsAppendConsumeGdsRegionBounds() { + using O = ShaderOpcode; + struct Access { + u32 m0, offset; + bool valid; + }; + constexpr Access accesses[] = { + {0x00000004u, 3, true}, + {0x00000004u, 4, false}, + {0x00000003u, 0, false}, + {0xbffc0004u, 3, true}, + {0xbffc0008u, 0, false}, + {0xc0000004u, 0, false}, + {0x0000c000u, 0xbfff, true}, + {0x0000c000u, 0xc000, false}, + }; + TestCase test; + test.name = "DsAppendConsumeGdsRegionBounds"; + auto &code = test.code; + for (const auto &access : accesses) { + AppendSMovLiteral(&code, 124, access.m0); + code.push_back(EncodeDs0(0x3e, access.offset, true)); + code.push_back(EncodeDs1(0, 0, 0)); + code.push_back(EncodeDs0(0x3d, access.offset, true)); + code.push_back(EncodeDs1(1, 0, 0)); + AppendStoreVgpr(&code, 0, static_cast(test.expected.size())); + test.expected.push_back(access.valid ? 10 : 0); + AppendStoreVgpr(&code, 1, static_cast(test.expected.size())); + test.expected.push_back(access.valid ? 11 : 0); + } + AppendEnd(&code); + test.initial.resize(test.expected.size()); + // Backing beyond the guest partition must not make an invalid region valid. + test.gds_initial.assign(0x10000u / sizeof(u32), 10); + test.expected_gds = test.gds_initial; + test.opcodes = {O::S_MOV_B32, O::DS_APPEND, O::DS_CONSUME, + O::BUFFER_STORE_DWORD, O::S_ENDPGM}; + return test; +} + TestCase DsGdsSubdwordAndAtomicWrites() { using O = ShaderOpcode; @@ -29686,7 +29779,10 @@ std::vector MakeCases() { AddCase(DsReadU16D16CapturedPreservesHighHalf); AddCase(DsReadU16D16HiCapturedPreservesLowHalf); AddCase(DsAppendConsumeUsesEncodedLdsSelector); + cases.push_back(DsAppendAllocatesAcrossWaves(32)); + cases.push_back(DsAppendAllocatesAcrossWaves(64)); AddCase(DsAppendUsesEncodedGdsSelector); + AddCase(DsAppendConsumeGdsRegionBounds); AddCase(DsGdsSubdwordAndAtomicWrites); AddCase(DsReadWrite2Variants); AddCase(DsWideReadSnapshotsOverlappingAddress);