From e93b4b0378cdb6fecdf481501460ad83d85d34a0 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 17 Oct 2026 04:37:16 +0200 Subject: [PATCH] shader: support returning 32-bit LDS compare-exchange (DS_CMPST_RTN_B32) (#1578) * shader: support DS_CMPST_RTN_B32 Assassin's Creed Valhalla (PPSA01490, #281) stops on DS opcode 0x30 in a compute shader. DS compares DATA and stores DATA2, the opposite of BUFFER_ATOMIC_CMPSWAP, so the 32-bit atomic path swaps the two for this opcode, as the 64-bit form does. Co-Authored-By: Claude Opus 5.5 * shader: reject unsupported GDS compare-store variant --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: nmzik --- .../backend/spirv/spirvEmitterInstructions.h | 1 + .../spirv/spirvEmitterMemoryHelpers.cpp | 9 ++- .../recompiler/frontend/decode/MemoryOps.cpp | 6 ++ .../frontend/decode/ShaderDecoder.cpp | 1 + .../frontend/decode/ShaderDecoder.h | 1 + .../recompiler/frontend/translate/Memory.cpp | 2 + .../recompiler/ir/opcodes/ValueOpcodes.cpp | 1 + .../recompiler/ir/opcodes/ValueOpcodes.inc | 1 + tests/ShaderRecompilerComputeTests.cpp | 59 ++++++++++++++++++- tests/shaderCfgTests.cpp | 19 ++++++ 10 files changed, 97 insertions(+), 3 deletions(-) diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInstructions.h b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInstructions.h index 377f32733..9feea9d3f 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInstructions.h +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInstructions.h @@ -326,6 +326,7 @@ inline constexpr auto EmitSharedAtomicSMin32 = EmitAtomic32; inline constexpr auto EmitSharedAtomicUMin32 = EmitAtomic32; inline constexpr auto EmitSharedAtomicSMax32 = EmitAtomic32; inline constexpr auto EmitSharedAtomicUMax64 = EmitSharedAtomic64; +inline constexpr auto EmitSharedAtomicCmpSwap32 = EmitAtomic32; inline constexpr auto EmitSharedAtomicCmpSwap64 = EmitSharedAtomic64; inline constexpr auto EmitSharedAtomicUMax32 = EmitAtomic32; inline constexpr auto EmitSharedAtomicAnd32 = EmitAtomic32; diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemoryHelpers.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemoryHelpers.cpp index 6a65c4472..033a93059 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemoryHelpers.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemoryHelpers.cpp @@ -504,6 +504,7 @@ uint32_t EncodePackedColorComponent(EmitterState& state, const Format::BufferFor spv::Op SpirvAtomicOpcode(IR::ValueOpcode opcode) { switch (opcode) { + case IR::ValueOpcode::SharedAtomicCmpSwap32: case IR::ValueOpcode::SharedAtomicCmpSwap64: case IR::ValueOpcode::ImageAtomicCompareSwap32: case IR::ValueOpcode::BufferAtomicCmpSwap32: return spv::OpAtomicCompareExchange; @@ -561,8 +562,12 @@ uint32_t EmitAtomicOperation(ValueEmitContext& ctx, const IR::Inst& inst, uint32 const bool wide = inst.GetType() == IR::Type::U64; const auto type = wide ? TypeU64(ctx.state) : TypeU32(ctx.state); if (opcode == spv::OpAtomicCompareExchange) { - const auto desired = ctx.Arg(inst, inst.NumArgs() - 3); - const auto comparator = ctx.Arg(inst, inst.NumArgs() - 2); + auto desired = ctx.Arg(inst, inst.NumArgs() - 3); + auto comparator = ctx.Arg(inst, inst.NumArgs() - 2); + if (inst.GetOpcode() == IR::ValueOpcode::SharedAtomicCmpSwap32) { + // RDNA2 DS compares DATA and stores DATA2, opposite to BUFFER atomics. + std::swap(desired, comparator); + } ctx.state.builder.AddFunction( spv::OpAtomicCompareExchange, TypeU32(ctx.state), old, pointer, ConstantU32(ctx.state, scope), ConstantU32(ctx.state, spv::MemorySemanticsMaskNone), diff --git a/src/graphics/shader/recompiler/frontend/decode/MemoryOps.cpp b/src/graphics/shader/recompiler/frontend/decode/MemoryOps.cpp index d37fed8cc..9b177ec88 100644 --- a/src/graphics/shader/recompiler/frontend/decode/MemoryOps.cpp +++ b/src/graphics/shader/recompiler/frontend/decode/MemoryOps.cpp @@ -115,6 +115,7 @@ constexpr MemoryOpcodeInfo DS_OPCODE_LIST[] = { {0x27u, Opcode::DS_MIN_RTN_U32, 1, 32}, {0x28u, Opcode::DS_MAX_RTN_U32, 1, 32}, {0x29u, Opcode::DS_AND_RTN_B32, 1, 32}, {0x2au, Opcode::DS_OR_RTN_B32, 1, 32}, {0x2bu, Opcode::DS_XOR_RTN_B32, 1, 32}, {0x2du, Opcode::DS_WRXCHG_RTN_B32, 1, 32}, + {0x30u, Opcode::DS_CMPST_RTN_B32, 1, 32}, {0x35u, Opcode::DS_SWIZZLE_B32, 1, 32}, {0x36u, Opcode::DS_READ_B32, 1, 32}, {0x37u, Opcode::DS_READ2_B32, 2, 32}, {0x38u, Opcode::DS_READ2ST64_B32, 2, 32}, {0x39u, Opcode::DS_READ_I8, 1, 8, true}, {0x3au, Opcode::DS_READ_U8, 1, 8}, @@ -201,6 +202,7 @@ bool IsDsAtomicOpcode(Opcode opcode) { case Opcode::DS_MAX_U32: case Opcode::DS_MAX_RTN_U32: case Opcode::DS_MAX_RTN_U64: + case Opcode::DS_CMPST_RTN_B32: case Opcode::DS_CMPST_RTN_B64: case Opcode::DS_AND_B32: case Opcode::DS_AND_RTN_B32: @@ -221,6 +223,7 @@ uint32_t DsSourceCount(Opcode opcode) { case Opcode::DS_WRITE2ST64_B32: case Opcode::DS_WRITE2_B64: case Opcode::DS_WRITE2ST64_B64: + case Opcode::DS_CMPST_RTN_B32: case Opcode::DS_CMPST_RTN_B64: case Opcode::DS_MSKOR_B32: return 3u; case Opcode::DS_MIN_F32: @@ -455,6 +458,9 @@ void DecodeDs(uint32_t pc, std::span code, uint32_t word_index, if (inst.opcode == Opcode::DS_SWIZZLE_B32 && inst.offset >= 0xe000u) { SetUnsupported(inst, Family::DS, opcode, "DS swizzle FFT mode is not implemented"); } + if (inst.gds && inst.opcode == Opcode::DS_CMPST_RTN_B32) { + SetUnsupported(inst, Family::DS, opcode, "DS compare-store return B32 GDS variant is not implemented"); + } if (inst.gds && (inst.opcode == Opcode::DS_SWIZZLE_B32 || inst.opcode == Opcode::DS_PERMUTE_B32 || inst.opcode == Opcode::DS_BPERMUTE_B32 || diff --git a/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.cpp b/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.cpp index 877fe2ccd..3bbe6239c 100644 --- a/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.cpp +++ b/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.cpp @@ -718,6 +718,7 @@ std::string InstructionToString(const Instruction& inst) { case Opcode::DS_MAX_U32: case Opcode::DS_MAX_RTN_U32: case Opcode::DS_MAX_RTN_U64: + case Opcode::DS_CMPST_RTN_B32: case Opcode::DS_CMPST_RTN_B64: case Opcode::DS_AND_B32: case Opcode::DS_AND_RTN_B32: diff --git a/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.h b/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.h index 4f5e3c785..434dd7fee 100644 --- a/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.h +++ b/src/graphics/shader/recompiler/frontend/decode/ShaderDecoder.h @@ -632,6 +632,7 @@ enum class Opcode { DS_MAX_U32, DS_MAX_RTN_U32, DS_MAX_RTN_U64, + DS_CMPST_RTN_B32, DS_CMPST_RTN_B64, DS_AND_B32, DS_AND_RTN_B32, diff --git a/src/graphics/shader/recompiler/frontend/translate/Memory.cpp b/src/graphics/shader/recompiler/frontend/translate/Memory.cpp index 0b6d3ee27..11067c81c 100644 --- a/src/graphics/shader/recompiler/frontend/translate/Memory.cpp +++ b/src/graphics/shader/recompiler/frontend/translate/Memory.cpp @@ -1085,6 +1085,8 @@ void Translator::EmitMemory(const Decoder::Instruction& inst) { return DS_ATOMIC(inst, IR::ValueOpcode::SharedAtomicUMax32, true); case Decoder::Opcode::DS_MAX_RTN_U64: return DS_ATOMIC(inst, IR::ValueOpcode::SharedAtomicUMax64, true); + case Decoder::Opcode::DS_CMPST_RTN_B32: + return DS_ATOMIC(inst, IR::ValueOpcode::SharedAtomicCmpSwap32, true); case Decoder::Opcode::DS_CMPST_RTN_B64: return DS_ATOMIC(inst, IR::ValueOpcode::SharedAtomicCmpSwap64, true); case Decoder::Opcode::DS_AND_B32: diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp index 320716ebc..a2561c85d 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp @@ -178,6 +178,7 @@ SharedAccess SharedAccessOf(ValueOpcode opcode) { case ValueOpcode::SharedAtomicSMax32: case ValueOpcode::SharedAtomicUMax32: case ValueOpcode::SharedAtomicUMax64: + case ValueOpcode::SharedAtomicCmpSwap32: case ValueOpcode::SharedAtomicCmpSwap64: case ValueOpcode::SharedAtomicAnd32: case ValueOpcode::SharedAtomicOr32: diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc index 84aaebfe0..adcb24841 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc @@ -307,6 +307,7 @@ VALUE_OPCODE(SharedAtomicUMin32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicSMax32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicUMax32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicUMax64, U64, U32, U64, U1) +VALUE_OPCODE(SharedAtomicCmpSwap32, U32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicCmpSwap64, U64, U32, U64, U64, U1) VALUE_OPCODE(SharedAtomicAnd32, U32, U32, U32, U1) VALUE_OPCODE(SharedAtomicOr32, U32, U32, U32, U1) diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index af1306247..692e88d44 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -21015,6 +21015,7 @@ CoverageClass ClassifyOpcode(ShaderOpcode opcode, case Opcode::DS_ADD_U32: case Opcode::DS_ADD_U64: case Opcode::DS_MAX_RTN_U64: + case Opcode::DS_CMPST_RTN_B32: case Opcode::DS_CMPST_RTN_B64: case Opcode::DS_ADD_RTN_U32: case Opcode::DS_SUB_U32: @@ -34698,6 +34699,60 @@ TestCase DsAtomicReturn64() { return test; } +TestCase DsCompareStoreReturn32() { + using O = ShaderOpcode; + TestCase test; + test.name = "DsCompareStoreReturn32"; + auto &code = test.code; + const auto result = [&](u32 reg, u32 value) { + AppendStoreVgpr(&code, reg, static_cast(test.expected.size())); + test.expected.push_back(value); + }; + struct Case { u32 before, compare, replacement; }; + constexpr std::array cases{Case{0x12345678u, 0x12345678u, 0x9abcdef0u}, + Case{0x12345678u, 0x9abcdef0u, 0x12345678u}, + Case{0x80000000u, 0u, 0x7fffffffu}, + Case{UINT32_MAX, UINT32_MAX, 0u}}; + AppendVMovU32(&code, 7, 0); + for (const auto &item : cases) { + AppendVMovLiteral(&code, 2, item.before); + code.push_back(EncodeDs0(0x0d, 8)); + code.push_back(EncodeDs1(0, 2, 7)); + AppendVMovLiteral(&code, 5, item.compare); + AppendVMovLiteral(&code, 19, item.replacement); + AppendVMovU32(&code, 4, 8); + // CS04b812daaacd532f pc 0x3e8: DATA compares, DATA2 replaces, VDst aliases ADDR. + code.insert(code.end(), {0xd8c00000u, 0x04130504u}); + result(4, item.before); + code.push_back(EncodeDs0(0x36, 8)); + code.push_back(EncodeDs1(8, 0, 7)); + result(8, item.before == item.compare ? item.replacement : item.before); + } + AppendVMovU32(&code, 5, 0); + AppendVMovU32(&code, 4, 12); // Past the three LDS DWORDs. + code.insert(code.end(), {0xd8c00000u, 0x04130504u}); + result(4, 0); + AppendVMovLiteral(&code, 2, 0x12345678u); + code.push_back(EncodeDs0(0x0d, 8)); + code.push_back(EncodeDs1(0, 2, 7)); + AppendVMovLiteral(&code, 5, 0x12345678u); + AppendVMovU32(&code, 4, 8); + code.push_back(EncodeSMovB32(126, InlineU32(0))); + code.insert(code.end(), {0xd8c00000u, 0x04130504u}); + code.push_back(EncodeSMovB32(126, InlineU32(1))); + result(4, 8); // An inactive lane keeps its destination and leaves memory alone. + code.push_back(EncodeDs0(0x36, 8)); + code.push_back(EncodeDs1(8, 0, 7)); + result(8, 0x12345678u); + AppendEnd(&code); + test.compute_info.lds_size_dwords = 3; + test.opcodes = {O::V_MOV_B32, O::S_MOV_B32, O::DS_WRITE_B32, O::DS_CMPST_RTN_B32, + O::DS_READ_B32, O::BUFFER_STORE_DWORD, O::S_ENDPGM}; + test.required_spirv = {"OpAtomicCompareExchange %uint"}; + test.forbidden_spirv = {"OpLoopMerge"}; + return test; +} + TestCase DsAtomic64Contention(bool add, u32 wave_size) { using O = ShaderOpcode; TestCase test; @@ -42276,6 +42331,7 @@ std::vector MakeCases() { } AddCase(DsAtomicReturnVariants); AddCase(DsAtomicReturn64); + AddCase(DsCompareStoreReturn32); for (bool add : {false, true}) { cases.push_back(DsAtomic64Bounds(add)); for (u32 wave_size : {32, 64}) { @@ -47800,7 +47856,8 @@ int main(int argc, char **argv) { RunCase(&vulkan, DsWideLdsPartialBounds()); RunCase(&vulkan, DsAtomic64Bounds(false)); RunCase(&vulkan, DsAtomic64Bounds(true)); - for (auto test : {DsWideLdsPartialBounds(), DsAtomic64Bounds(false), DsAtomic64Bounds(true), DsAtomicReturn64()}) { + for (auto test : {DsWideLdsPartialBounds(), DsAtomic64Bounds(false), DsAtomic64Bounds(true), DsAtomicReturn64(), + DsCompareStoreReturn32()}) { test.compute_info.lds_storage = true; std::erase(test.required_spirv, "WorkgroupMemoryExplicitLayoutKHR"); test.required_spirv.push_back("StorageBuffer"); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index b6d660b9d..53810c31f 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -8119,6 +8119,24 @@ void TestDsReturningAtomic64Decoder() { "captured DS_CMPST_RTN_B64 lost its comparison/replacement pairs or aliases"); } +void TestDsCompareStore32Decoder() { + using namespace ShaderRecompiler::Decoder; + // CS04b812daaacd532f pc 0x3e8 (AC Valhalla, #281). + const uint32_t words[] = {0xd8c00000u, 0x04130504u}; + Instruction decoded; + DecodeInstruction(words, 0u, decoded); + Check(decoded.opcode == Opcode::DS_CMPST_RTN_B32 && decoded.word_count == 2u && + decoded.data_dwords == 1u && decoded.src_count == 3u && decoded.src0.reg == 4u && + decoded.src1.reg == 5u && decoded.src2.reg == 19u && decoded.dst.reg == 4u && + decoded.offset == 0u && !decoded.gds, + "captured DS_CMPST_RTN_B32 lost its comparison/replacement order or aliases"); + + const uint32_t gds_words[] = {words[0] | (1u << 17u), words[1]}; + DecodeInstruction(gds_words, 0u, decoded); + Check(decoded.opcode == Opcode::UNSUPPORTED && decoded.word_count == 2u && decoded.gds, + "DS_CMPST_RTN_B32 must reject the unimplemented GDS variant"); +} + void TestDsFloatMinMaxDecoder() { using namespace ShaderRecompiler::Decoder; for (bool max_value : {false, true}) { @@ -16480,6 +16498,7 @@ int main() { TestScalarAshrI64Decoder(); TestImageAtomicWidthDecoder(); TestDsReturningAtomic64Decoder(); + TestDsCompareStore32Decoder(); TestDsFloatMinMaxDecoder(); TestNewShaderDecoderArchitecture(); TestImageAddressOperands();