diff --git a/src/graphics/shader/recompiler/ShaderRecompiler.cpp b/src/graphics/shader/recompiler/ShaderRecompiler.cpp index f6927ede2..09fbd9648 100644 --- a/src/graphics/shader/recompiler/ShaderRecompiler.cpp +++ b/src/graphics/shader/recompiler/ShaderRecompiler.cpp @@ -78,85 +78,6 @@ void LogDispatcherFallback(const CompileOptions& options, const CFG::Graph& cfg, static_cast(cfg.back_edges.size()), reason.c_str()); } -bool InstructionMaySplitSpirvBlock(const IR::Instruction& inst) { - switch (inst.op) { - case IR::Opcode::SLoadDword: - case IR::Opcode::SBufferLoadDword: - case IR::Opcode::BufferLoadUbyte: - case IR::Opcode::BufferLoadSbyte: - case IR::Opcode::BufferLoadUshort: - case IR::Opcode::BufferLoadSshort: - case IR::Opcode::BufferLoadDword: - case IR::Opcode::BufferStoreByte: - case IR::Opcode::BufferStoreShort: - case IR::Opcode::BufferStoreDword: - case IR::Opcode::AtomicSwapU32: - case IR::Opcode::AtomicAddU32: - case IR::Opcode::AtomicSubU32: - case IR::Opcode::AtomicSMinI32: - case IR::Opcode::AtomicUMinU32: - case IR::Opcode::AtomicSMaxI32: - case IR::Opcode::AtomicUMaxU32: - case IR::Opcode::AtomicAndU32: - case IR::Opcode::AtomicOrU32: - case IR::Opcode::AtomicXorU32: - case IR::Opcode::AtomicFMinF32: - case IR::Opcode::AtomicFMaxF32: - case IR::Opcode::FlatLoadUbyte: - case IR::Opcode::FlatLoadSbyte: - case IR::Opcode::FlatLoadUshort: - case IR::Opcode::FlatLoadSshort: - case IR::Opcode::FlatLoadDword: - case IR::Opcode::FlatStoreByte: - case IR::Opcode::FlatStoreShort: - case IR::Opcode::FlatStoreDword: - case IR::Opcode::DsReadUbyte: - case IR::Opcode::DsReadSbyte: - case IR::Opcode::DsReadUshort: - case IR::Opcode::DsReadSshort: - case IR::Opcode::DsReadB32: - case IR::Opcode::DsMinF32: - case IR::Opcode::DsMaxF32: - case IR::Opcode::DsWriteByte: - case IR::Opcode::DsWriteShort: - case IR::Opcode::DsWriteB32: - case IR::Opcode::DsWriteAddtidB32: - case IR::Opcode::DsReadAddtidB32: - case IR::Opcode::DsAppend: - case IR::Opcode::DsConsume: - case IR::Opcode::ImageLoad: - case IR::Opcode::ImageStore: - case IR::Opcode::ImageSample: - case IR::Opcode::ImageGather4: - case IR::Opcode::Export: return true; - default: return false; - } -} - -bool NeedsDispatcherForStructuredLoopHeader(const IR::Program& ir, uint32_t* failure_block, - std::string* reason) { - for (const auto& block: ir.blocks) { - if (!block.terminator.loop_header) { - continue; - } - for (const auto& inst: block.instructions) { - if (!InstructionMaySplitSpirvBlock(inst)) { - continue; - } - if (reason != nullptr) { - *reason = fmt::format("loop header block {} contains an instruction whose SPIR-V " - "lowering emits internal control flow: {}", - block.id, IR::InstructionToString(inst).c_str()); - } - if (failure_block != nullptr) { - *failure_block = block.id; - } - return true; - } - } - return false; -} - enum class EmbeddedFetchValueType { Unknown, Constant, @@ -771,69 +692,6 @@ bool TryRecompile(std::span code, const CompileOptions& options, return false; } - if (!dispatcher_fallback) { - const auto select_dispatcher = [&](uint32_t failure_block, const std::string& reason, - const CFG::Graph& diagnostic_cfg) { - dispatcher_fallback = true; - dispatcher_reason = reason; - cfg.failure_kind = CFG::FailureKind::StructuredControlFlow; - cfg.failure_block = failure_block; - cfg.unsupported_reason = reason; - ir.cfg_failure_kind = CFG::FailureKind::StructuredControlFlow; - ir.fallback_reason = reason; - LogDispatcherFallback(options, diagnostic_cfg, "emit", reason); - }; - const auto retry_budget = cfg.natural_loops.size(); - for (size_t retry = 0;; retry++) { - std::string emitter_reason; - uint32_t failure_block = UINT32_MAX; - if (!NeedsDispatcherForStructuredLoopHeader(ir, &failure_block, &emitter_reason)) { - break; - } - if (retry >= retry_budget) { - select_dispatcher(failure_block, - emitter_reason + "; no-clone loop-header isolation exhausted", - cfg); - break; - } - - const auto* source_block = cfg.FindBlock(failure_block); - const auto source_pc = source_block != nullptr ? source_block->start_pc : UINT32_MAX; - const auto previous_cfg = cfg; - const auto blocks_before = cfg.blocks.size(); - std::string isolation_error; - if (!CFG::IsolateLoopHeader(cfg, failure_block, &isolation_error)) { - const auto failed_cfg = cfg; - cfg = previous_cfg; - select_dispatcher(failure_block, - fmt::format("{}; no-clone loop-header isolation failed: {}", - emitter_reason, isolation_error), - failed_cfg); - break; - } - - IR::Program isolated_ir; - if (!lower_program(isolated_ir)) { - const auto lower_error = error != nullptr ? *error : "IR lowering failed"; - if (error != nullptr) { - error->clear(); - } - cfg = previous_cfg; - select_dispatcher(failure_block, - fmt::format("{}; isolated loop-header lowering failed: {}", - emitter_reason, lower_error), - cfg); - break; - } - ir = std::move(isolated_ir); - LOGF("%s CFG isolated semantic loop header: stage=%s hash=0x%016" PRIx64 - " source_block=%" PRIu32 " pc=0x%08" PRIx32 " blocks=%" PRIu64 "->%" PRIu64 - " reason=%s\n", - GetDumpLabel(options), StageName(options.stage), options.shader_hash, - failure_block, source_pc, static_cast(blocks_before), - static_cast(cfg.blocks.size()), emitter_reason.c_str()); - } - } LOGF("%s phase end: stage=%s hash=0x%016" PRIx64 " IR LowerProgram blocks=%" PRIu64 " elapsed_ms=%" PRIu64 "\n", GetDumpLabel(options), StageName(options.stage), options.shader_hash, diff --git a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp index 3a9f2fffc..5ebc73d7f 100644 --- a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp +++ b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.cpp @@ -1264,6 +1264,11 @@ bool IsInnermostLoopControlConditional(const Graph& graph, const BasicBlock& blo }; return is_repeat_target(true_target) && is_repeat_target(false_target); } + const bool true_in_body = Contains(loop->body_blocks, true_target); + const bool false_in_body = Contains(loop->body_blocks, false_target); + if (true_in_body != false_in_body) { + return true; + } const auto is_control_target = [&](uint32_t target) { return target == loop->merge || target == loop->continue_block; }; @@ -1342,6 +1347,37 @@ bool CanonicalizeNaturalLoops(Graph& graph, std::string* error) { return false; } +bool IsolateSemanticLoopHeaders(Graph& graph, std::string* error) { + const auto isolation_budget = graph.natural_loops.size() + 1u; + for (size_t isolation = 0; isolation < isolation_budget; isolation++) { + const auto loop = std::find_if(graph.natural_loops.begin(), graph.natural_loops.end(), + [&](const auto& value) { + const auto* header = graph.FindBlock(value.header); + return header != nullptr && + header->inst_begin != header->inst_end; + }); + if (loop == graph.natural_loops.end()) { + return true; + } + + // SPIR-V requires OpLoopMerge and its branch to remain in the loop header's + // physical block. Keep that header as a dedicated control node, exactly like + // the separate Loop node in shadPS4's structured AST. Guest instructions live + // in the body so later lowering may introduce bounds/EXEC control flow without + // displacing OpLoopMerge into a helper-created block. + if (!IsolateSemanticLoopHeader(graph, loop->header)) { + SetFailure(graph, FailureKind::StructuredControlFlow, loop->header, + fmt::format("failed to isolate semantic loop header {}", loop->header), + error); + return false; + } + } + + SetFailure(graph, FailureKind::StructuredControlFlow, graph.entry_block, + "CFG semantic loop-header isolation exceeded rewrite budget", error); + return false; +} + bool SplitSharedMergeBlock(Graph& graph, uint32_t merge, const std::vector& construct_blocks, bool force_split = false) { @@ -2113,6 +2149,9 @@ bool StructurizeImpl(Graph& graph, std::string* error) { if (!SplitSharedMergeBlocks(graph, error)) { return false; } + if (!IsolateSemanticLoopHeaders(graph, error)) { + return false; + } ClearStructuredTerminators(graph); std::map merge_headers; @@ -2139,6 +2178,14 @@ bool StructurizeImpl(Graph& graph, std::string* error) { error); return false; } + if (header->inst_begin != header->inst_end || + header->terminator.kind != TerminatorKind::Branch) { + SetFailure(graph, FailureKind::StructuredControlFlow, loop.header, + fmt::format("loop header {} is not a dedicated empty control block", + loop.header), + error); + return false; + } if (!reserve_merge_block(loop.header, loop.merge)) { return false; } @@ -2207,31 +2254,6 @@ bool Structurize(Graph& graph, std::string* error) { return false; } -bool IsolateLoopHeader(Graph& graph, uint32_t header_id, std::string* error) { - const auto loop = - std::find_if(graph.natural_loops.begin(), graph.natural_loops.end(), - [header_id](const auto& value) { return value.header == header_id; }); - if (loop == graph.natural_loops.end()) { - SetFailure(graph, FailureKind::StructuredControlFlow, header_id, - fmt::format("block {} is not a natural loop header", header_id), error); - return false; - } - - ClearStructuredTerminators(graph); - if (!IsolateSemanticLoopHeader(graph, header_id)) { - SetFailure(graph, FailureKind::StructuredControlFlow, header_id, - fmt::format("loop header block {} has no semantic instructions", header_id), - error); - return false; - } - - graph.unsupported = false; - graph.failure_kind = FailureKind::None; - graph.failure_block = UINT32_MAX; - graph.unsupported_reason.clear(); - return Structurize(graph, error); -} - std::string BranchConditionToString(BranchCondition condition) { switch (condition) { case BranchCondition::Always: return "always"; diff --git a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.h b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.h index 868ff5ef5..cb47f3100 100644 --- a/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.h +++ b/src/graphics/shader/recompiler/frontend/cfg/ShaderCFG.h @@ -110,7 +110,6 @@ struct Graph { bool BuildGraph(const Decoder::Program& program, Graph& graph, std::string* error); bool Structurize(Graph& graph, std::string* error); -bool IsolateLoopHeader(Graph& graph, uint32_t header_id, std::string* error); std::string BranchConditionToString(BranchCondition condition); std::string FailureKindToString(FailureKind kind); std::string GraphToString(const Graph& graph); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index df96d382a..c980b9de5 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -6957,11 +6957,8 @@ void TestNewShaderRecompilerCfgLoopHeaderDsAppendConsumeStructured() { CfgInstructionCoverage(graph, decoded.instructions.size()); Check(ShaderRecompiler::CFG::Structurize(graph, &error), error.c_str()); Check(graph.natural_loops.size() == 1u, "DS loop was not preserved"); - Check(ShaderRecompiler::CFG::IsolateLoopHeader( - graph, graph.natural_loops.front().header, &error), - error.c_str()); Check(graph.blocks.size() == original_block_count + 1u, - "DS loop did not add exactly one empty header"); + "DS loop structurization did not add exactly one empty header"); Check(CfgInstructionCoverage(graph, decoded.instructions.size()) == original_coverage, "DS loop canonicalization duplicated semantic instructions"); @@ -7018,6 +7015,36 @@ void TestNewShaderRecompilerCfgLoopHeaderDsReadStructured() { CheckSpirvBinaryValidates(result.spirv); } +void TestNewShaderRecompilerCfgLoopHeaderDsRead2B64Structured() { + const uint32_t shader[] = { + EncodeDs0(0x77, (48u << 8u) | 32u), // loop: ds_read2_b64 v[0:3], v1 + EncodeDs1(0, 0, 1), + EncodeSop2(0x00, 2, 2, 129), // s_add_u32 s2, s2, 1 + EncodeSopc(0x0a, 2, 130), // s_cmp_lt_u32 s2, 2 + EncodeSopp(0x05, 0xfffbu), // s_cbranch_scc1 loop + EncodeMubuf0(0x1e), + EncodeMubuf1(0, 4, 1), // keep all four guarded LDS results live + 0xbf810000u, + }; + + auto options = MakeCompileOptions(ShaderType::Compute); + options.dump_ir = true; + + ShaderRecompiler::CompileResult result; + std::string error; + Check(ShaderRecompiler::TryRecompile(shader, options, result, &error), + error.c_str()); + Check(Common::ContainsStr(result.ir_dump, "mode=structured") && + !result.program.dispatcher_fallback, + "DS read2 b64 loop header did not stay structured"); + Check(Common::ContainsStr(result.decoded_dump, "DS_READ2_B64"), + "DS read2 b64 loop regression did not decode the captured opcode"); + Check(!SpirvContainsOpcode(result.spirv, 251), + "DS read2 b64 structured SPIR-V unexpectedly contains OpSwitch"); + CheckSpirvPhiParents(result.spirv); + CheckSpirvBinaryValidates(result.spirv); +} + void TestNewShaderRecompilerCfgSharedOuterAndLoopMerge() { const uint32_t shader[] = { EncodeSopc(0x06, 0, 0), // s_cmp_eq_u32 s0, s0 @@ -7439,8 +7466,8 @@ void TestNewShaderRecompilerCfgMultipleLoopLatches() { Check(graph.back_edges.size() == 2u, "multiple-latch fixture lacks two native backedges"); Check(ShaderRecompiler::CFG::Structurize(graph, &error), error.c_str()); - Check(graph.blocks.size() == original_block_count + 1u, - "multiple native latches did not create one synthetic continue"); + Check(graph.blocks.size() == original_block_count + 2u, + "multiple native latches did not create one synthetic continue and one empty header"); Check(graph.back_edges.size() == 1u && graph.natural_loops.size() == 1u, "multiple native latches were not coalesced to one SPIR-V backedge"); const auto &loop = graph.natural_loops.front(); @@ -7527,13 +7554,13 @@ void TestNewShaderRecompilerCfgNestedEarlyExitLoopForwarders() { "nested early-exit fixture has the wrong native CFG"); const bool structured = ShaderRecompiler::CFG::Structurize(graph, &error); Check(structured, error.c_str()); - Check(graph.blocks.size() == original_block_count + 3u && + Check(graph.blocks.size() == original_block_count + 4u && CfgInstructionCoverage(graph, decoded.instructions.size()) == original_coverage && std::ranges::count_if(graph.blocks, [](const auto &block) { return block.inst_begin == block.inst_end; - }) == original_empty_blocks + 3, + }) == original_empty_blocks + 4, "nested early-exit structurization changed semantic coverage"); const auto *preheader = graph.FindBlockByPc(0x00u); const auto *outer = graph.FindBlockByPc(0x08u); @@ -11213,12 +11240,12 @@ void TestNewShaderRecompilerSpirvSizeBaselines() { EncodeSopp(0x01), }; const auto structured_result = compile("structured-phi", structured_phi, - {.words = 136, - .instructions = 39, + {.words = 140, + .instructions = 41, .phis = 1, - .labels = 7, + .labels = 8, .loop_merges = 1, - .branches = 5, + .branches = 6, .conditional_branches = 1}); const auto structured_metrics = MeasureSpirv(structured_result.spirv); Check(Common::ContainsStr(structured_result.ir_dump, "Phi"), @@ -11230,12 +11257,12 @@ void TestNewShaderRecompilerSpirvSizeBaselines() { CheckSpirvPhiParents(structured_result.spirv); const auto structured_repeat = compile("structured-phi-repeat", structured_phi, - {.words = 136, - .instructions = 39, + {.words = 140, + .instructions = 41, .phis = 1, - .labels = 7, + .labels = 8, .loop_merges = 1, - .branches = 5, + .branches = 6, .conditional_branches = 1}); Check(structured_repeat.spirv == structured_result.spirv, "deferred Phi patching is not deterministic"); @@ -11467,6 +11494,7 @@ int main() { TestNewShaderRecompilerCfgLoopHeaderBufferLoadDispatcher(); TestNewShaderRecompilerCfgLoopHeaderDsAppendConsumeStructured(); TestNewShaderRecompilerCfgLoopHeaderDsReadStructured(); + TestNewShaderRecompilerCfgLoopHeaderDsRead2B64Structured(); TestNewShaderRecompilerCfgSharedOuterAndLoopMerge(); TestNewShaderRecompilerCfgLoopEarlyBreakNoSelection(); TestNewShaderRecompilerCfgNestedLoopNonlocalExitDispatcher();