diff --git a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp index 5352da78d..7a708d91d 100644 --- a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp +++ b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp @@ -1160,9 +1160,16 @@ bool HasLinearPathToTerminal(const Graph& graph, uint32_t start) { bool IsEnclosingLinearExit(const Graph& graph, uint32_t header, uint32_t block_id) { // AGC commonly lowers nested early returns through a terminal epilogue shared with an // enclosing conditional. Such a path is an exit boundary, not part of the inner selection. + if (!HasLinearPathToTerminal(graph, block_id)) { + return false; + } const auto* block = graph.FindBlock(block_id); - if (block == nullptr || graph.Dominates(header, block_id) || - !HasLinearPathToTerminal(graph, block_id) || block->predecessors.empty()) { + while (block != nullptr && graph.Dominates(header, block->id) && + block->successors.size() == 1u) { + block = graph.FindBlock(block->successors.front()); + } + if (block == nullptr || graph.Dominates(header, block->id) || + block->predecessors.empty()) { return false; } return std::ranges::all_of(block->predecessors, [&](uint32_t predecessor) { @@ -1199,7 +1206,7 @@ uint32_t FindSelectionMerge(const Graph& graph, const BasicBlock& block) { const auto* global_block = graph.FindBlock(global_merge); const auto true_target = block.terminator.true_block; const auto false_target = block.terminator.false_block; - if (global_block != nullptr && global_block->successors.empty()) { + if (global_block == nullptr || global_block->successors.empty()) { const bool false_reaches_true = CanReachBefore(graph, false_target, true_target, global_merge); const bool true_reaches_false = @@ -1207,6 +1214,26 @@ uint32_t FindSelectionMerge(const Graph& graph, const BasicBlock& block) { if (false_reaches_true != true_reaches_false) { return false_reaches_true ? true_target : false_target; } + if (global_merge == UINT32_MAX) { + // An enclosing selection's shared return needs its own inner merge + // gateway; the other arm can return directly without joining live state. + if (IsEnclosingLinearExit(graph, block.id, true_target)) { + return true_target; + } + if (IsEnclosingLinearExit(graph, block.id, false_target)) { + return false_target; + } + // A return can leave a selection without reaching its merge. Keep the + // continuing arm as the merge instead of joining live state with a return. + if (graph.Dominates(block.id, false_target) && + HasLinearPathToTerminal(graph, true_target)) { + return false_target; + } + if (graph.Dominates(block.id, true_target) && + HasLinearPathToTerminal(graph, false_target)) { + return true_target; + } + } } return global_merge; } @@ -1755,8 +1782,7 @@ bool RouteOneSharedArm(Graph& graph, uint32_t original_block_count, uint32_t out } const auto first_arm = std::min(shared, other); if (first_arm >= original_block_count || outer_id >= inner_id || - inner_id >= first_arm || - graph.FindNearestCommonPostDominator(shared, other) == UINT32_MAX) { + inner_id >= first_arm) { continue; } @@ -1819,8 +1845,7 @@ bool RouteOneSharedArm(Graph& graph, uint32_t original_block_count, uint32_t out const auto first_arm = std::min(continuation, other); if (outer_predecessors.empty() || inner_predecessors.empty() || external_predecessor || first_arm >= original_block_count || - outer_id >= inner_id || inner_id >= first_arm || - graph.FindNearestCommonPostDominator(continuation, other) == UINT32_MAX) { + outer_id >= inner_id || inner_id >= first_arm) { continue; } @@ -1999,9 +2024,7 @@ Graph BuildGraph(const Decoder::Program& program) { const auto& last = program.instructions[block.inst_end - 1u]; const auto next_pc = InstructionEndPc(last); if (last.opcode == Opcode::S_ENDPGM) { - block.terminator.kind = TerminatorKind::Branch; - block.terminator.condition = BranchCondition::Always; - block.terminator.true_block = pc_to_block.at(end_pc); + block.terminator.kind = TerminatorKind::Return; } else if (last.opcode == Opcode::S_SETPC_B64) { const auto& target_info = setpc_targets.at(last.pc); if (target_info.indirect) { diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index ac629dabf..4f437a674 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -17057,6 +17057,51 @@ TestCase SimpleLoop() { O::S_BRANCH, O::V_MOV_B32, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } +TestCase SharedReturnKeepsSelectedValues() { + using O = ShaderOpcode; + + std::vector code = { + EncodeVop1(0x01, 5, 4), + EncodeVop2(0x1a, 5, InlineU32(6), 5), + EncodeVop2(0x25, 5, Vgpr(0), 5), + EncodeSMovB32(8, InlineU32(10)), + EncodeSopc(0x06, 4, InlineU32(0)), + 0, + EncodeSMovB32(8, InlineU32(20)), + EncodeSopc(0x06, 4, InlineU32(1)), + 0, + EncodeSMovB32(8, InlineU32(30)), + }; + AppendStoreSgprAtLaneDwordOffset(&code, 8, 5, 0); + AppendEnd(&code); + const auto shared_return = static_cast(code.size()); + AppendStoreSgprAtLaneDwordOffset(&code, 8, 5, 0); + AppendEnd(&code); + code[5] = EncodeSopp(0x05, shared_return - 6u); + code[8] = EncodeSopp(0x05, shared_return - 9u); + + TestCase test; + test.name = "SharedReturnKeepsSelectedValues"; + test.code = code; + for (u32 group = 0; group < 3; group++) { + test.expected.insert(test.expected.end(), 64, (group + 1u) * 10u); + } + test.opcodes = {O::V_MOV_B32, O::V_LSHLREV_B32, O::V_ADD_NC_U32, + O::S_MOV_B32, O::S_CMP_EQ_U32, O::S_CBRANCH_SCC1, + O::BUFFER_STORE_DWORD, O::S_ENDPGM}; + test.required_spirv = {"OpSelectionMerge"}; + test.forbidden_spirv = {"OpSwitch"}; + test.compute_info.threads_num[0] = 64; + test.compute_info.threads_num[1] = 1; + test.compute_info.threads_num[2] = 1; + test.compute_info.group_id[0] = true; + test.compute_info.thread_ids_num = 1; + test.compute_info.workgroup_register = 4; + test.has_compute_info = true; + test.dispatch_x = 3; + return test; +} + TestCase BranchVccnzUsesWaveMask() { using O = ShaderOpcode; @@ -21983,6 +22028,7 @@ std::vector MakeCases() { AddCase(VectorCompareInvertedMaskSelect); AddCase(BranchSelect); AddCase(SimpleLoop); + AddCase(SharedReturnKeepsSelectedValues); AddCase(BranchVccnzUsesWaveMask); AddCase(BranchVccnzUsesCarryProducedWaveMask); AddCase(ScalarMemoryLoadVariants); @@ -26322,6 +26368,7 @@ int main(int argc, char **argv) { RunCase(&vulkan, BranchVccnzUsesWaveMask()); RunCase(&vulkan, BranchVccnzUsesCarryProducedWaveMask()); RunCase(&vulkan, ImageSampleAndGather()); + RunCase(&vulkan, SharedReturnKeepsSelectedValues()); return 0; } if (argc == 2 && std::strcmp(argv[1], "--clip-control-only") == 0) { diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 1bdf419fc..df78be31d 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -7762,7 +7762,7 @@ void TestNewShaderRecompilerCfgNestedEarlyExitLoopForwarders() { std::ranges::count_if(graph.blocks, [](const auto &block) { return block.inst_begin == block.inst_end; }); - Check(original_block_count == 8u && graph.natural_loops.size() == 1u, + Check(original_block_count == 7u && graph.natural_loops.size() == 1u, "nested early-exit fixture has the wrong native CFG"); const bool structured = ShaderRecompiler::CFG::Structurize(graph); Check(structured, graph.unsupported_reason.c_str()); @@ -7832,52 +7832,19 @@ void TestNewShaderRecompilerCfgExecSccSharedArm() { ShaderRecompiler::CFG::BranchCondition::GotoVariable; route_sets += block.terminator.goto_value >= 0; } - Check(graph.blocks.size() == 10u && route_selects == 1u && route_sets == 3u, - "shared selection arm was not routed through typed goto state"); + Check(graph.blocks.size() == 5u && route_selects == 0u && route_sets == 0u, + "shared return arm introduced synthetic routing state"); Check(CfgInstructionCoverage(graph, decoded.instructions.size()) == original_coverage, "EXEC/SCC shared-arm structurization changed semantic coverage"); - ShaderRecompiler::IR::Program value_ir; - ShaderComputeInputInfo compute_info{}; - ShaderRecompiler::Frontend::TranslateOptions translate_options{}; - translate_options.stage = ShaderType::Compute; - translate_options.wave_size = 64u; - translate_options.compute = &compute_info; - value_ir = ShaderRecompiler::Frontend::TranslateProgram(decoded, graph, - translate_options); - uint32_t goto_sets = 0; - uint32_t goto_gets = 0; - for (const auto *block : value_ir.blocks) { - for (const auto &inst : *block) { - goto_sets += inst.GetOpcode() == - ShaderRecompiler::IR::ValueOpcode::SetGotoVariable; - goto_gets += inst.GetOpcode() == - ShaderRecompiler::IR::ValueOpcode::GetGotoVariable; - } - } - Check(goto_sets == 3u && goto_gets == 1u, - "shared-arm route was not represented by typed goto pseudo-ops"); - ShaderRecompiler::IR::RewriteToSsa(value_ir.blocks); - ShaderRecompiler::IR::RemoveIdentities(value_ir.blocks); - ShaderRecompiler::IR::EliminateDeadCode(value_ir.blocks); - for (const auto *block : value_ir.blocks) { - for (const auto &inst : *block) { - Check(inst.GetOpcode() != - ShaderRecompiler::IR::ValueOpcode::GetGotoVariable && - inst.GetOpcode() != - ShaderRecompiler::IR::ValueOpcode::SetGotoVariable, - "typed goto pseudo-op survived SSA rewriting"); - } - } - auto options = MakeCompileOptions(ShaderType::Compute); options.dump_ir = true; auto result = RecompileForTest(shader, options); Check(!result.program.dispatcher_fallback && Common::ContainsStr(result.ir_dump, "mode=structured"), "EXEC/SCC shared-arm epilogue did not stay structured"); - Check(SpirvInstructionOpcodeCount(result.spirv, 247) == 3u, + Check(SpirvInstructionOpcodeCount(result.spirv, 247) == 2u, "EXEC/SCC shared-arm SPIR-V has the wrong selection-merge count"); Check(SpirvInstructionOpcodeCount(result.spirv, 251) == 0u, "EXEC/SCC shared-arm SPIR-V unexpectedly used dispatcher OpSwitch"); @@ -7887,6 +7854,58 @@ void TestNewShaderRecompilerCfgExecSccSharedArm() { CheckSpirvBinaryValidates(result.spirv); } +void TestSharedReturnPreservesDescriptorDominance() { + const uint32_t shader[] = { + EncodeSmem0(0x02, 32, 14), (125u << 25u) | 0x60u, + EncodeSopc(0x06, 0, 128), + EncodeSopp(0x04, 10), // first early return + EncodeSmem0(0x02, 32, 14), (125u << 25u) | 0x50u, + EncodeSopc(0x06, 1, 128), + EncodeSopp(0x04, 6), // second early return after descriptor overwrite + EncodeSmem0(0x08, 56, 16), 125u << 25u, + EncodeVop1(0x01, 0, 56), + EncodeExp0(0x00, 0x1), EncodeExp1(0, 0, 0, 0), + EncodeSopp(0x01), + EncodeSopp(0x01), // shared return epilogue + }; + ShaderRecompiler::Decoder::Program decoded; + ShaderRecompiler::Decoder::DecodeProgram(std::span{shader}, decoded); + auto graph = ShaderRecompiler::CFG::BuildGraph(decoded); + const auto original_coverage = + CfgInstructionCoverage(graph, decoded.instructions.size()); + Check(ShaderRecompiler::CFG::Structurize(graph), + graph.unsupported_reason.c_str()); + const auto *overwrite = graph.FindBlockByPc(0x10u); + const auto *body = graph.FindBlockByPc(0x20u); + Check(graph.blocks.size() == 5u && overwrite != nullptr && body != nullptr && + graph.Dominates(overwrite->id, body->id) && + std::ranges::count_if(graph.blocks, [](const auto &block) { + return block.terminator.kind == + ShaderRecompiler::CFG::TerminatorKind::Return; + }) == 2 && + CfgInstructionCoverage(graph, decoded.instructions.size()) == + original_coverage, + "shared return routing lost the live descriptor's dominance"); + + std::array table{}; + table[0x50u / 4u] = 0x2000u; + table[0x50u / 4u + 2u] = 4u; + table[0x60u / 4u] = 0x3000u; + table[0x60u / 4u + 2u] = 4u; + std::array user_data{}; + const auto address = reinterpret_cast(table.data()); + user_data[28] = static_cast(address); + user_data[29] = static_cast(address >> 32u); + auto options = MakeCompileOptions(ShaderType::Pixel); + options.user_data = user_data; + auto result = RecompileForTest(shader, options, ReadHostTestMemory); + Check(!result.program.dispatcher_fallback && + result.resources.buffers.size() == 1u && + result.resources.buffers[0].dwords[0] == 0x2000u, + "return-only descriptor reached the surviving buffer operation"); + CheckSpirvBinaryValidates(result.spirv); +} + void TestNewShaderRecompilerCfgNestedTailEarlyExit() { const uint32_t shader[] = { EncodeSopc(0x06, 0, 0), // preceding outer condition @@ -7942,7 +7961,7 @@ void TestNewShaderRecompilerCfgNestedTailEarlyExit() { CheckSpirvBinaryValidates(result.spirv); } -void TestNewShaderRecompilerCfgRoutesInnerSharedExitFirst() { +void TestNewShaderRecompilerCfgSharedReturnAfterNestedSelections() { const uint32_t shader[] = { EncodeSopc(0x06, 0, 0), // outer forward-skip condition EncodeSopp(0x04, 4), // outer -> shared or nested condition @@ -7979,15 +7998,8 @@ void TestNewShaderRecompilerCfgRoutesInnerSharedExitFirst() { std::ranges::count_if(graph.blocks, [](const auto &block) { return block.terminator.goto_value >= 0; }); - const bool has_early_route = - std::ranges::any_of(graph.blocks, [](const auto &block) { - return block.start_pc < 0x30u && - (block.terminator.condition == - ShaderRecompiler::CFG::BranchCondition::GotoVariable || - block.terminator.goto_value >= 0); - }); - Check(route_selects == 1u && route_sets == 3u && !has_early_route, - "shared exits were routed before the innermost blocking construct"); + Check(route_selects == 0u && route_sets == 0u, + "nested selections introduced routing state for a shared return"); auto options = MakeCompileOptions(ShaderType::Compute); options.dump_ir = true; @@ -8072,9 +8084,9 @@ void TestNewShaderRecompilerCfgOverlappingEarlyExitLadder() { EncodeSopc(0x06, 3, 3), // block 3 EncodeSopp(0x04, 2), // block 3 -> 5 or 4 EncodeSMovB32(4, 129), // block 4 - 0xbf810000u, // block 4 -> 6 + 0xbf810000u, // block 4 returns EncodeSMovB32(5, 129), // block 5 - 0xbf810000u, // block 5 -> 6 + 0xbf810000u, // block 5 returns }; ShaderRecompiler::Decoder::Program decoded; @@ -8082,7 +8094,7 @@ void TestNewShaderRecompilerCfgOverlappingEarlyExitLadder() { ShaderRecompiler::CFG::Graph graph; graph = ShaderRecompiler::CFG::BuildGraph(decoded); Check( - graph.blocks.size() == 7u && + graph.blocks.size() == 6u && graph.blocks[0].successors == std::vector({1, 2}) && graph.blocks[0].terminator.true_block == 2u && graph.blocks[0].terminator.false_block == 1u && @@ -8095,8 +8107,12 @@ void TestNewShaderRecompilerCfgOverlappingEarlyExitLadder() { graph.blocks[3].successors == std::vector({4, 5}) && graph.blocks[3].terminator.true_block == 5u && graph.blocks[3].terminator.false_block == 4u && - graph.blocks[4].successors == std::vector({6}) && - graph.blocks[5].successors == std::vector({6}), + graph.blocks[4].successors.empty() && + graph.blocks[5].successors.empty() && + graph.blocks[4].terminator.kind == + ShaderRecompiler::CFG::TerminatorKind::Return && + graph.blocks[5].terminator.kind == + ShaderRecompiler::CFG::TerminatorKind::Return, "overlapping early-exit fixture does not match the observed shader CFG"); const auto original_block_count = graph.blocks.size(); const auto original_coverage = @@ -11916,8 +11932,9 @@ int main() { TestNewShaderRecompilerCfgDuplicateMergeStructuredSplit(); TestNewShaderRecompilerCfgNestedEarlyExitLoopForwarders(); TestNewShaderRecompilerCfgExecSccSharedArm(); + TestSharedReturnPreservesDescriptorDominance(); TestNewShaderRecompilerCfgNestedTailEarlyExit(); - TestNewShaderRecompilerCfgRoutesInnerSharedExitFirst(); + TestNewShaderRecompilerCfgSharedReturnAfterNestedSelections(); TestNewShaderRecompilerCfgLoopSharedRegion(); TestNewShaderRecompilerCfgOverlappingEarlyExitLadder(); TestNewShaderRecompilerCfgNestedEarlyExitSharedTerminal();