diff --git a/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.cpp b/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.cpp index 4fbcf93d..c9f1c2b9 100644 --- a/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.cpp @@ -1,5 +1,7 @@ #include "graphics/shader/recompiler/backend/spirv/SpirvBuilder.h" +#include "common/debug.h" + #include namespace Libs::Graphics::ShaderRecompiler::Spirv { @@ -217,7 +219,27 @@ void Builder::AddFunction(const std::vector& words) { AppendInstructionWords(m_functions, words.data(), words.size()); } +DeferredPhi Builder::AddDeferredPhi(uint32_t type, uint32_t result, size_t incoming_count) { + std::vector words {245u, type, result}; + words.resize(words.size() + incoming_count * 2u); + const DeferredPhi phi {m_functions.size(), incoming_count}; + AddFunction(words); + m_unpatched_phi_incomings += incoming_count; + return phi; +} + +void Builder::PatchDeferredPhi(DeferredPhi phi, size_t incoming, uint32_t value, uint32_t parent) { + EXIT_IF(incoming >= phi.incoming_count || value == 0 || parent == 0); + const auto value_word = phi.word_offset + 3u + incoming * 2u; + const auto parent_word = value_word + 1u; + EXIT_IF(m_functions.at(value_word) != 0 || m_functions.at(parent_word) != 0); + m_functions[value_word] = value; + m_functions[parent_word] = parent; + m_unpatched_phi_incomings--; +} + std::vector Builder::Build() const { + EXIT_IF(m_unpatched_phi_incomings != 0); std::vector module; module.reserve(5u + m_capabilities.size() + m_extensions.size() + m_ext_inst_imports.size() + diff --git a/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.h b/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.h index 663ed383..06bac8a9 100644 --- a/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.h +++ b/src/graphics/shader/recompiler/backend/spirv/SpirvBuilder.h @@ -16,6 +16,11 @@ struct TypeAnnotation { std::vector operands; }; +struct DeferredPhi { + size_t word_offset = 0; + size_t incoming_count = 0; +}; + class Builder { public: explicit Builder(uint32_t version = 0x00010300u); @@ -36,14 +41,16 @@ public: uint32_t DefineGlobalVariable(uint32_t pointer_type, uint32_t storage_class); void DefineGlobalVariable(uint32_t id, uint32_t pointer_type, uint32_t storage_class); - void AddMemoryModel(std::initializer_list operands); - void AddEntryPoint(uint32_t execution_model, uint32_t entry_point, const char* name, - const std::vector& interfaces); - void AddExecutionMode(std::initializer_list operands); - void AddName(uint32_t target, const char* name); - void AddAnnotation(std::initializer_list words); - void AddFunction(std::initializer_list words); - void AddFunction(const std::vector& words); + void AddMemoryModel(std::initializer_list operands); + void AddEntryPoint(uint32_t execution_model, uint32_t entry_point, const char* name, + const std::vector& interfaces); + void AddExecutionMode(std::initializer_list operands); + void AddName(uint32_t target, const char* name); + void AddAnnotation(std::initializer_list words); + void AddFunction(std::initializer_list words); + void AddFunction(const std::vector& words); + DeferredPhi AddDeferredPhi(uint32_t type, uint32_t result, size_t incoming_count); + void PatchDeferredPhi(DeferredPhi phi, size_t incoming, uint32_t value, uint32_t parent); [[nodiscard]] std::vector Build() const; @@ -74,6 +81,7 @@ private: std::set m_required_extensions; std::map m_import_ids; std::map, uint32_t> m_declaration_ids; + size_t m_unpatched_phi_incomings = 0; }; } // namespace Libs::Graphics::ShaderRecompiler::Spirv diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAnalysis.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAnalysis.cpp index 5cbe5d49..a9209973 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAnalysis.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAnalysis.cpp @@ -321,11 +321,11 @@ void EmitStorageImageWrite(EmitterState& state, uint32_t resource, bool uint_ima state.builder.AddFunction({OpSelectionMerge, merge_label, SelectionControlNone}); state.builder.AddFunction(words); for (uint32_t mip = 0; mip < image.mip_count; mip++) { - state.builder.AddFunction({OpLabel, labels[mip]}); + EmitLabel(state, labels[mip]); state.builder.AddFunction({OpImageWrite, LoadAt(binding.array_index + mip), coord, texel}); state.builder.AddFunction({OpBranch, merge_label}); } - state.builder.AddFunction({OpLabel, merge_label}); + EmitLabel(state, merge_label); } uint32_t ExecutionModelForStage(ShaderType stage) { diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp index 285fa41a..30ba1b7a 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp @@ -589,17 +589,17 @@ bool EmitValueImage(ValueEmitContext& ctx, const IR::Inst& inst) { state.builder.AddFunction({OpSelectionMerge, merge_label, SelectionControlNone}); state.builder.AddFunction(switch_words); std::vector phi_words {OpPhi, result_type, state.builder.AllocateId()}; - state.builder.AddFunction({OpLabel, default_label}); + EmitLabel(state, default_label); phi_words.push_back(EmitSample(image.indirect_resources[0])); phi_words.push_back(default_label); state.builder.AddFunction({OpBranch, merge_label}); for (uint32_t candidate = 1; candidate < image.indirect_resources.size(); candidate++) { - state.builder.AddFunction({OpLabel, labels[candidate - 1u]}); + EmitLabel(state, labels[candidate - 1u]); phi_words.push_back(EmitSample(image.indirect_resources[candidate])); phi_words.push_back(labels[candidate - 1u]); state.builder.AddFunction({OpBranch, merge_label}); } - state.builder.AddFunction({OpLabel, merge_label}); + EmitLabel(state, merge_label); state.builder.AddFunction(phi_words); ctx.Define(inst, ResultVector(ctx, phi_words[2], integer, dref)); return true; diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInternal.h b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInternal.h index c7479781..78bea452 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInternal.h +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterInternal.h @@ -323,15 +323,8 @@ struct EmitterState { uint32_t sampler_variable = 0; uint32_t main_func = 0; uint32_t entry_label = 0; + uint32_t current_label = 0; uint32_t pixel_valid_mask_variable = 0; - bool dispatcher_fallback = false; - uint32_t dispatch_pc_variable = 0; - uint32_t dispatch_header_label = 0; - uint32_t dispatch_select_label = 0; - uint32_t dispatch_default_label = 0; - uint32_t dispatch_after_switch_label = 0; - uint32_t dispatch_continue_label = 0; - uint32_t dispatch_merge_label = 0; uint32_t subgroup_local_invocation_id_variable = 0; uint32_t per_vertex_variable = 0; uint32_t depth_variable = 0; @@ -359,6 +352,11 @@ uint32_t TypePushConstantElementPointer(EmitterState& state); uint32_t TypeLdsArrayPointer(EmitterState& state, uint32_t storage_class); uint32_t TypeLdsElementPointer(EmitterState& state, uint32_t storage_class); +inline void EmitLabel(EmitterState& state, uint32_t label) { + state.current_label = label; + state.builder.AddFunction({OpLabel, label}); +} + struct ValueEmitContext { ValueEmitContext(EmitterState& state_, const IR::ValueProgram& program_) : state(state_), program(program_) {} @@ -378,16 +376,15 @@ struct ValueEmitContext { size_t BlockIndex(const IR::Block* block) const; void Fail(const IR::Inst& inst, const char* reason); - EmitterState& state; - const IR::ValueProgram& program; - std::unordered_map definitions; - std::unordered_map labels; - std::unordered_map phi_variables; - std::unordered_map dispatcher_variables; - const IR::Block* current_block = nullptr; - uint32_t scratch_u32_variable = 0; - bool failed = false; - std::string error; + EmitterState& state; + const IR::ValueProgram& program; + std::unordered_map definitions; + std::unordered_map labels; + const std::unordered_map* dispatcher_spills = nullptr; + const IR::Block* current_block = nullptr; + uint32_t scratch_u32_variable = 0; + bool failed = false; + std::string error; }; enum class VertexInputScalarKind { Float, Sint, Uint }; @@ -819,10 +816,10 @@ void EmitIfCondition(EmitterState& state, uint32_t condition, Fn&& fn) { const auto merge_label = state.builder.AllocateId(); state.builder.AddFunction({OpSelectionMerge, merge_label, SelectionControlNone}); state.builder.AddFunction({OpBranchConditional, condition, then_label, merge_label}); - state.builder.AddFunction({OpLabel, then_label}); + EmitLabel(state, then_label); fn(); state.builder.AddFunction({OpBranch, merge_label}); - state.builder.AddFunction({OpLabel, merge_label}); + EmitLabel(state, merge_label); } template @@ -838,14 +835,14 @@ uint32_t EmitValueOrDefaultIfCondition(EmitterState& state, uint32_t condition, const auto merge_label = state.builder.AllocateId(); state.builder.AddFunction({OpSelectionMerge, merge_label, SelectionControlNone}); state.builder.AddFunction({OpBranchConditional, condition, then_label, else_label}); - state.builder.AddFunction({OpLabel, then_label}); + EmitLabel(state, then_label); const auto then_value = fn(); state.builder.AddFunction({OpBranch, then_exit}); - state.builder.AddFunction({OpLabel, then_exit}); + EmitLabel(state, then_exit); state.builder.AddFunction({OpBranch, merge_label}); - state.builder.AddFunction({OpLabel, else_label}); + EmitLabel(state, else_label); state.builder.AddFunction({OpBranch, merge_label}); - state.builder.AddFunction({OpLabel, merge_label}); + EmitLabel(state, merge_label); const auto value = state.builder.AllocateId(); state.builder.AddFunction( {OpPhi, type, value, then_value, then_exit, default_value, else_label}); diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp index 86835cb3..71907ea5 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp @@ -289,11 +289,11 @@ uint32_t AtomicUpdate(EmitterState& state, uint32_t pointer, IR::ResourceKind ki const auto observed = state.builder.AllocateId(); const auto exchanged = state.builder.AllocateId(); state.builder.AddFunction({OpBranch, preheader}); - state.builder.AddFunction({OpLabel, preheader}); + EmitLabel(state, preheader); state.builder.AddFunction({OpAtomicLoad, TypeU32(state), initial, pointer, ConstantU32(state, scope), ConstantU32(state, MemorySemanticsNone)}); state.builder.AddFunction({OpBranch, header}); - state.builder.AddFunction({OpLabel, header}); + EmitLabel(state, header); state.builder.AddFunction( {OpPhi, TypeU32(state), observed, initial, preheader, exchanged, cont}); const auto next = desired(observed); @@ -303,9 +303,9 @@ uint32_t AtomicUpdate(EmitterState& state, uint32_t pointer, IR::ResourceKind ki const auto success = Binary(state, OpIEqual, TypeBool(state), exchanged, observed); state.builder.AddFunction({OpLoopMerge, merge, cont, LoopControlNone}); state.builder.AddFunction({OpBranchConditional, success, merge, cont}); - state.builder.AddFunction({OpLabel, cont}); + EmitLabel(state, cont); state.builder.AddFunction({OpBranch, header}); - state.builder.AddFunction({OpLabel, merge}); + EmitLabel(state, merge); state.builder.AddFunction({OpMemoryBarrier, ConstantU32(state, scope), ConstantU32(state, MemorySemanticsAcquireRelease | memory)}); return observed; diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterProgram.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterProgram.cpp index ee245933..a345ea67 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterProgram.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterProgram.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace Libs::Graphics::ShaderRecompiler::Spirv::Emitter { @@ -16,9 +17,9 @@ void EmitKillIfBoolFalse(EmitterState& state, uint32_t active) { state.builder.AddFunction({OpLogicalNot, TypeBool(state), inactive, active}); state.builder.AddFunction({OpSelectionMerge, merge_label, SelectionControlNone}); state.builder.AddFunction({OpBranchConditional, inactive, kill_label, merge_label}); - state.builder.AddFunction({OpLabel, kill_label}); + EmitLabel(state, kill_label); state.builder.AddFunction({OpKill}); - state.builder.AddFunction({OpLabel, merge_label}); + EmitLabel(state, merge_label); } void EmitKillIfPixelValidMaskInactive(EmitterState& state) { @@ -35,7 +36,7 @@ void EmitKillIfPixelValidMaskInactive(EmitterState& state) { EmitKillIfBoolFalse(state, active); } -uint32_t PhiPointerType(EmitterState& state, IR::Type type) { +uint32_t SpillPointerType(EmitterState& state, IR::Type type) { switch (type) { case IR::Type::U1: return TypePointer(state, StorageClassFunction, TypeBool(state)); case IR::Type::U8: @@ -53,18 +54,40 @@ uint32_t PhiPointerType(EmitterState& state, IR::Type type) { } } -void StorePhiEdge(ValueEmitContext& ctx, const IR::Block* from, const IR::Block* to) { +struct DeferredPhiPatch { + DeferredPhi phi; + const IR::Inst* instruction = nullptr; +}; + +struct StructuredFunctionState { + std::unordered_map block_exit_labels; + std::vector deferred_phis; +}; + +struct DispatcherFunctionState { + std::unordered_map spills; + uint32_t pc_variable = 0; + uint32_t header_label = 0; + uint32_t select_label = 0; + uint32_t default_label = 0; + uint32_t after_switch_label = 0; + uint32_t continue_label = 0; + uint32_t merge_label = 0; +}; + +void StoreDispatcherPhiEdge(ValueEmitContext& ctx, const DispatcherFunctionState& dispatcher, + const IR::Block* from, const IR::Block* to) { if (to == nullptr) { return; } for (const auto& phi: *to) { if (phi.GetOpcode() != IR::ValueOpcode::Phi) { - continue; + break; } for (size_t index = 0; index < phi.NumArgs(); index++) { if (phi.PhiBlock(index) == from) { ctx.state.builder.AddFunction( - {OpStore, ctx.phi_variables.at(&phi), ctx.Def(phi.Arg(index))}); + {OpStore, dispatcher.spills.at(&phi), ctx.Def(phi.Arg(index))}); break; } } @@ -112,7 +135,6 @@ void EmitStructuredTerminator(ValueEmitContext& ctx, const IR::Block* block, EmitReturn(ctx); return; } - StorePhiEdge(ctx, block, target); emit_merge(); ctx.state.builder.AddFunction({OpBranch, ctx.Label(target)}); return; @@ -124,8 +146,6 @@ void EmitStructuredTerminator(ValueEmitContext& ctx, const IR::Block* block, EmitReturn(ctx); return; } - StorePhiEdge(ctx, block, true_block); - StorePhiEdge(ctx, block, false_block); const auto condition = ctx.Def(info.condition); emit_merge(); ctx.state.builder.AddFunction( @@ -136,39 +156,41 @@ void EmitStructuredTerminator(ValueEmitContext& ctx, const IR::Block* block, } } -void EmitDispatcherTarget(ValueEmitContext& ctx, const IR::Block* from, uint32_t target) { +void EmitDispatcherTarget(ValueEmitContext& ctx, const DispatcherFunctionState& dispatcher, + const IR::Block* from, uint32_t target) { const auto* block = TargetBlock(ctx.program, target); if (block != nullptr) { - StorePhiEdge(ctx, from, block); + StoreDispatcherPhiEdge(ctx, dispatcher, from, block); } } -void EmitDispatcherTerminator(ValueEmitContext& ctx, const IR::Block* block, - const IR::ValueBlockInfo& info, uint32_t exit_label) { +void EmitDispatcherTerminator(ValueEmitContext& ctx, const DispatcherFunctionState& dispatcher, + const IR::Block* block, const IR::ValueBlockInfo& info, + uint32_t exit_label) { const auto& term = info.terminator; switch (term.kind) { case CFG::TerminatorKind::Branch: - EmitDispatcherTarget(ctx, block, term.true_block); + EmitDispatcherTarget(ctx, dispatcher, block, term.true_block); ctx.state.builder.AddFunction( - {OpStore, ctx.state.dispatch_pc_variable, ConstantU32(ctx.state, term.true_block)}); + {OpStore, dispatcher.pc_variable, ConstantU32(ctx.state, term.true_block)}); break; case CFG::TerminatorKind::ConditionalBranch: { - EmitDispatcherTarget(ctx, block, term.true_block); - EmitDispatcherTarget(ctx, block, term.false_block); + EmitDispatcherTarget(ctx, dispatcher, block, term.true_block); + EmitDispatcherTarget(ctx, dispatcher, block, term.false_block); const auto selected = ctx.state.builder.AllocateId(); ctx.state.builder.AddFunction({OpSelect, TypeU32(ctx.state), selected, ctx.Def(info.condition), ConstantU32(ctx.state, term.true_block), ConstantU32(ctx.state, term.false_block)}); - ctx.state.builder.AddFunction({OpStore, ctx.state.dispatch_pc_variable, selected}); + ctx.state.builder.AddFunction({OpStore, dispatcher.pc_variable, selected}); break; } case CFG::TerminatorKind::IndirectBranch: { for (const auto target: term.indirect_targets) { - EmitDispatcherTarget(ctx, block, target); + EmitDispatcherTarget(ctx, dispatcher, block, target); } for (const auto target: term.indirect_selector_targets) { - EmitDispatcherTarget(ctx, block, target); + EmitDispatcherTarget(ctx, dispatcher, block, target); } uint32_t selected = ConstantU32(ctx.state, UINT32_MAX); if (!info.indirect_target.IsEmpty()) { @@ -190,100 +212,184 @@ void EmitDispatcherTerminator(ValueEmitContext& ctx, const IR::Block* block, selected = next; } } - ctx.state.builder.AddFunction({OpStore, ctx.state.dispatch_pc_variable, selected}); + ctx.state.builder.AddFunction({OpStore, dispatcher.pc_variable, selected}); break; } default: ctx.state.builder.AddFunction( - {OpStore, ctx.state.dispatch_pc_variable, ConstantU32(ctx.state, UINT32_MAX)}); + {OpStore, dispatcher.pc_variable, ConstantU32(ctx.state, UINT32_MAX)}); break; } ctx.state.builder.AddFunction({OpBranch, exit_label}); } -bool EmitInstruction(ValueEmitContext& ctx, const IR::Inst& inst) { - const auto finish = [&]() { - if (const auto found = ctx.dispatcher_variables.find(&inst); - found != ctx.dispatcher_variables.end()) { - ctx.state.builder.AddFunction( - {OpStore, found->second, ctx.Def(IR::Value(const_cast(&inst)))}); - } - return true; - }; - if (inst.GetOpcode() == IR::ValueOpcode::Phi) { - ctx.state.builder.AddFunction( - {OpLoad, ctx.TypeId(inst.GetType()), ctx.Result(inst), ctx.phi_variables.at(&inst)}); - return finish(); - } +bool EmitDirectInstruction(ValueEmitContext& ctx, const IR::Inst& inst) { if (EmitValueFlow(ctx, inst) || EmitValueAlu(ctx, inst) || EmitValueMemory(ctx, inst) || EmitValueImage(ctx, inst)) { - return finish(); + return true; } ctx.Fail(inst, "has no direct SPIR-V emitter"); return false; } -void EmitBlock(ValueEmitContext& ctx, const IR::Block* block) { +bool EmitStructuredInstruction(ValueEmitContext& ctx, StructuredFunctionState& structured, + const IR::Inst& inst) { + if (inst.GetOpcode() == IR::ValueOpcode::Phi) { + const auto type = ctx.TypeId(inst.GetType()); + if (type == 0 || inst.NumArgs() == 0) { + ctx.Fail(inst, "has no native SPIR-V representation"); + return false; + } + for (size_t index = 0; index < inst.NumArgs(); index++) { + const auto* predecessor = inst.PhiBlock(index); + if (predecessor == nullptr || !ctx.labels.contains(predecessor)) { + ctx.Fail(inst, "has a predecessor outside the structured function"); + return false; + } + } + structured.deferred_phis.push_back( + {ctx.state.builder.AddDeferredPhi(type, ctx.Result(inst), inst.NumArgs()), &inst}); + return true; + } + return EmitDirectInstruction(ctx, inst); +} + +bool EmitDispatcherInstruction(ValueEmitContext& ctx, const DispatcherFunctionState& dispatcher, + const IR::Inst& inst) { + if (inst.GetOpcode() == IR::ValueOpcode::Phi) { + const auto type = ctx.TypeId(inst.GetType()); + if (type == 0) { + ctx.Fail(inst, "cannot be loaded by the dispatcher"); + return false; + } + ctx.state.builder.AddFunction( + {OpLoad, type, ctx.Result(inst), dispatcher.spills.at(&inst)}); + return true; + } + if (!EmitDirectInstruction(ctx, inst)) { + return false; + } + if (const auto found = dispatcher.spills.find(&inst); found != dispatcher.spills.end()) { + ctx.state.builder.AddFunction( + {OpStore, found->second, ctx.Def(IR::Value(const_cast(&inst)))}); + } + return true; +} + +template +void EmitBlock(ValueEmitContext& ctx, const IR::Block* block, EmitInstruction&& emit_instruction) { ctx.current_block = block; - ctx.state.builder.AddFunction({OpLabel, ctx.Label(block)}); + EmitLabel(ctx.state, ctx.Label(block)); + bool emitted_non_phi = false; for (const auto& inst: *block) { - if (!EmitInstruction(ctx, inst)) { + if (inst.GetOpcode() == IR::ValueOpcode::Phi) { + if (emitted_non_phi) { + ctx.Fail(inst, "appears after a non-Phi instruction"); + return; + } + } else { + emitted_non_phi = true; + } + if (!emit_instruction(inst)) { return; } } } -void EmitStructuredFunction(ValueEmitContext& ctx) { - ctx.state.builder.AddFunction({OpBranch, ctx.Label(ctx.program.blocks.front())}); - for (size_t index = 0; index < ctx.program.blocks.size() && !ctx.failed; index++) { - EmitBlock(ctx, ctx.program.blocks[index]); - EmitStructuredTerminator(ctx, ctx.program.blocks[index], ctx.program.block_info[index]); +void PatchStructuredPhis(ValueEmitContext& ctx, StructuredFunctionState& structured) { + for (const auto& deferred: structured.deferred_phis) { + for (size_t index = 0; index < deferred.instruction->NumArgs(); index++) { + const auto* predecessor = deferred.instruction->PhiBlock(index); + const auto found = structured.block_exit_labels.find(predecessor); + if (found == structured.block_exit_labels.end()) { + ctx.failed = true; + ctx.error = "typed Phi predecessor was not emitted"; + return; + } + ctx.state.builder.PatchDeferredPhi( + deferred.phi, index, ctx.Def(deferred.instruction->Arg(index)), found->second); + if (ctx.failed) { + return; + } + } } } -void EmitDispatcherFunction(ValueEmitContext& ctx) { +void EmitStructuredFunction(ValueEmitContext& ctx) { + StructuredFunctionState structured; + ctx.state.builder.AddFunction({OpBranch, ctx.Label(ctx.program.blocks.front())}); + for (size_t index = 0; index < ctx.program.blocks.size() && !ctx.failed; index++) { + const auto* block = ctx.program.blocks[index]; + EmitBlock(ctx, block, [&](const IR::Inst& inst) { + return EmitStructuredInstruction(ctx, structured, inst); + }); + if (ctx.failed) { + break; + } + structured.block_exit_labels.emplace(block, ctx.state.current_label); + EmitStructuredTerminator(ctx, block, ctx.program.block_info[index]); + } + if (!ctx.failed) { + PatchStructuredPhis(ctx, structured); + } +} + +void EmitDispatcherFunction(ValueEmitContext& ctx, const DispatcherFunctionState& dispatcher) { auto& state = ctx.state; const auto* entry = ctx.program.blocks.front(); state.builder.AddFunction({OpBranch, ctx.Label(entry)}); - EmitBlock(ctx, entry); - EmitDispatcherTerminator(ctx, entry, ctx.program.block_info.front(), - state.dispatch_header_label); + EmitBlock(ctx, entry, [&](const IR::Inst& inst) { + return EmitDispatcherInstruction(ctx, dispatcher, inst); + }); + if (ctx.failed) { + return; + } + EmitDispatcherTerminator(ctx, dispatcher, entry, ctx.program.block_info.front(), + dispatcher.header_label); - state.builder.AddFunction({OpLabel, state.dispatch_header_label}); + EmitLabel(state, dispatcher.header_label); const auto pc = state.builder.AllocateId(); - state.builder.AddFunction({OpLoad, TypeU32(state), pc, state.dispatch_pc_variable}); + state.builder.AddFunction({OpLoad, TypeU32(state), pc, dispatcher.pc_variable}); const auto done = state.builder.AllocateId(); state.builder.AddFunction( {OpIEqual, TypeBool(state), done, pc, ConstantU32(ctx.state, UINT32_MAX)}); state.builder.AddFunction( - {OpLoopMerge, state.dispatch_merge_label, state.dispatch_continue_label, LoopControlNone}); + {OpLoopMerge, dispatcher.merge_label, dispatcher.continue_label, LoopControlNone}); state.builder.AddFunction( - {OpBranchConditional, done, state.dispatch_merge_label, state.dispatch_select_label}); + {OpBranchConditional, done, dispatcher.merge_label, dispatcher.select_label}); - state.builder.AddFunction({OpLabel, state.dispatch_select_label}); + EmitLabel(state, dispatcher.select_label); state.builder.AddFunction( - {OpSelectionMerge, state.dispatch_after_switch_label, SelectionControlNone}); - std::vector words {OpSwitch, pc, state.dispatch_default_label}; + {OpSelectionMerge, dispatcher.after_switch_label, SelectionControlNone}); + std::vector words {OpSwitch, pc, dispatcher.default_label}; for (size_t index = 1; index < ctx.program.blocks.size(); index++) { words.push_back(ctx.program.block_info[index].id); words.push_back(ctx.Label(ctx.program.blocks[index])); } state.builder.AddFunction(words); - state.builder.AddFunction({OpLabel, state.dispatch_default_label}); + EmitLabel(state, dispatcher.default_label); state.builder.AddFunction( - {OpStore, state.dispatch_pc_variable, ConstantU32(ctx.state, UINT32_MAX)}); - state.builder.AddFunction({OpBranch, state.dispatch_after_switch_label}); + {OpStore, dispatcher.pc_variable, ConstantU32(ctx.state, UINT32_MAX)}); + state.builder.AddFunction({OpBranch, dispatcher.after_switch_label}); for (size_t index = 1; index < ctx.program.blocks.size() && !ctx.failed; index++) { - EmitBlock(ctx, ctx.program.blocks[index]); - EmitDispatcherTerminator(ctx, ctx.program.blocks[index], ctx.program.block_info[index], - state.dispatch_after_switch_label); + EmitBlock(ctx, ctx.program.blocks[index], [&](const IR::Inst& inst) { + return EmitDispatcherInstruction(ctx, dispatcher, inst); + }); + if (ctx.failed) { + break; + } + EmitDispatcherTerminator(ctx, dispatcher, ctx.program.blocks[index], + ctx.program.block_info[index], dispatcher.after_switch_label); } - state.builder.AddFunction({OpLabel, state.dispatch_after_switch_label}); - state.builder.AddFunction({OpBranch, state.dispatch_continue_label}); - state.builder.AddFunction({OpLabel, state.dispatch_continue_label}); - state.builder.AddFunction({OpBranch, state.dispatch_header_label}); - state.builder.AddFunction({OpLabel, state.dispatch_merge_label}); + if (ctx.failed) { + return; + } + EmitLabel(state, dispatcher.after_switch_label); + state.builder.AddFunction({OpBranch, dispatcher.continue_label}); + EmitLabel(state, dispatcher.continue_label); + state.builder.AddFunction({OpBranch, dispatcher.header_label}); + EmitLabel(state, dispatcher.merge_label); EmitReturn(ctx); } @@ -326,10 +432,9 @@ uint32_t ValueEmitContext::Def(IR::Value value) { error = "direct SPIR-V emitter received a non-value argument"; return ConstantU32(state, 0); } - if (program.dispatcher_fallback && current_block != nullptr && + if (dispatcher_spills != nullptr && current_block != nullptr && inst->Parent() != current_block) { - if (const auto found = dispatcher_variables.find(inst); - found != dispatcher_variables.end()) { + if (const auto found = dispatcher_spills->find(inst); found != dispatcher_spills->end()) { const auto id = state.builder.AllocateId(); state.builder.AddFunction({OpLoad, TypeId(inst->GetType()), id, found->second}); return id; @@ -417,42 +522,41 @@ void ValueEmitContext::Fail(const IR::Inst& inst, const char* reason) { } bool EmitValueProgram(EmitterState& state, const IR::ValueProgram& program, std::string* error) { - ValueEmitContext ctx(state, program); + ValueEmitContext ctx(state, program); + std::optional dispatcher; if (state.stage == ShaderType::Pixel && state.requirements.pixel_valid_mask) { state.pixel_valid_mask_variable = state.builder.AllocateId(); state.builder.AddName(state.pixel_valid_mask_variable, "pixel_valid_mask_active"); } for (const auto* block: program.blocks) { ctx.labels.emplace(block, state.builder.AllocateId()); - for (const auto& inst: *block) { - if (inst.GetOpcode() != IR::ValueOpcode::Phi) { - continue; - } - const auto ptr_type = PhiPointerType(state, inst.GetType()); - if (ptr_type == 0) { - ctx.failed = true; - ctx.error = fmt::format("typed opcode Phi of type {} cannot be spilled across a " - "control-flow edge", - IR::TypeName(inst.GetType())); - break; - } - ctx.phi_variables.emplace(&inst, state.builder.AllocateId()); - } - } - if (ctx.failed) { - SetError(error, ctx.error.c_str()); - return false; } if (program.dispatcher_fallback) { + auto& dispatch = dispatcher.emplace(); + for (const auto* block: program.blocks) { + for (const auto& inst: *block) { + if (inst.GetOpcode() != IR::ValueOpcode::Phi) { + continue; + } + if (SpillPointerType(state, inst.GetType()) == 0) { + ctx.Fail(inst, "cannot be stored by the dispatcher"); + break; + } + dispatch.spills.emplace(&inst, state.builder.AllocateId()); + } + } const auto mark_cross_block = [&](IR::Value value, const IR::Block* consumer) { value = value.Resolve(); const auto* definition = value.TryInstruction(); - if (definition == nullptr || definition->Parent() == consumer || - PhiPointerType(state, definition->GetType()) == 0) { + if (definition == nullptr || definition->Parent() == consumer) { return; } - if (!ctx.dispatcher_variables.contains(definition)) { - ctx.dispatcher_variables.emplace(definition, state.builder.AllocateId()); + if (SpillPointerType(state, definition->GetType()) == 0) { + ctx.Fail(*definition, "cannot be stored by the dispatcher"); + return; + } + if (!dispatch.spills.contains(definition)) { + dispatch.spills.emplace(definition, state.builder.AllocateId()); } }; for (const auto* block: program.blocks) { @@ -468,14 +572,18 @@ bool EmitValueProgram(EmitterState& state, const IR::ValueProgram& program, std: mark_cross_block(program.block_info[index].condition, program.blocks[index]); mark_cross_block(program.block_info[index].indirect_target, program.blocks[index]); } - state.dispatcher_fallback = true; - state.dispatch_pc_variable = state.builder.AllocateId(); - state.dispatch_header_label = state.builder.AllocateId(); - state.dispatch_select_label = state.builder.AllocateId(); - state.dispatch_default_label = state.builder.AllocateId(); - state.dispatch_after_switch_label = state.builder.AllocateId(); - state.dispatch_continue_label = state.builder.AllocateId(); - state.dispatch_merge_label = state.builder.AllocateId(); + dispatch.pc_variable = state.builder.AllocateId(); + dispatch.header_label = state.builder.AllocateId(); + dispatch.select_label = state.builder.AllocateId(); + dispatch.default_label = state.builder.AllocateId(); + dispatch.after_switch_label = state.builder.AllocateId(); + dispatch.continue_label = state.builder.AllocateId(); + dispatch.merge_label = state.builder.AllocateId(); + ctx.dispatcher_spills = &dispatch.spills; + } + if (ctx.failed) { + SetError(error, ctx.error.c_str()); + return false; } for (const auto* block: program.blocks) { if (std::ranges::any_of(*block, [](const IR::Inst& inst) { @@ -491,7 +599,7 @@ bool EmitValueProgram(EmitterState& state, const IR::ValueProgram& program, std: } state.builder.AddFunction( {OpFunction, TypeVoid(state), state.main_func, FunctionControlNone, TypeFunction(state)}); - state.builder.AddFunction({OpLabel, state.entry_label}); + EmitLabel(state, state.entry_label); if (state.requirements.function_lds) { state.builder.AddFunction({OpVariable, TypeLdsArrayPointer(state, StorageClassFunction), state.lds_variable, StorageClassFunction}); @@ -504,23 +612,12 @@ bool EmitValueProgram(EmitterState& state, const IR::ValueProgram& program, std: if (program.dispatcher_fallback) { state.builder.AddFunction({OpVariable, TypePointer(state, StorageClassFunction, TypeU32(state)), - state.dispatch_pc_variable, StorageClassFunction}); - } - for (const auto* block: program.blocks) { - for (const auto& inst: *block) { - if (inst.GetOpcode() != IR::ValueOpcode::Phi) { - continue; - } - state.builder.AddFunction({OpVariable, PhiPointerType(state, inst.GetType()), - ctx.phi_variables.at(&inst), StorageClassFunction}); - } - } - if (program.dispatcher_fallback) { + dispatcher->pc_variable, StorageClassFunction}); for (const auto* block: program.blocks) { for (const auto& inst: *block) { - if (const auto found = ctx.dispatcher_variables.find(&inst); - found != ctx.dispatcher_variables.end()) { - state.builder.AddFunction({OpVariable, PhiPointerType(state, inst.GetType()), + if (const auto found = dispatcher->spills.find(&inst); + found != dispatcher->spills.end()) { + state.builder.AddFunction({OpVariable, SpillPointerType(state, inst.GetType()), found->second, StorageClassFunction}); } } @@ -539,7 +636,7 @@ bool EmitValueProgram(EmitterState& state, const IR::ValueProgram& program, std: if (program.blocks.empty()) { EmitReturn(ctx); } else if (program.dispatcher_fallback) { - EmitDispatcherFunction(ctx); + EmitDispatcherFunction(ctx, *dispatcher); } else { EmitStructuredFunction(ctx); } diff --git a/src/graphics/shader/recompiler/ir/Value.cpp b/src/graphics/shader/recompiler/ir/Value.cpp index a5c9a9ba..2c3de14a 100644 --- a/src/graphics/shader/recompiler/ir/Value.cpp +++ b/src/graphics/shader/recompiler/ir/Value.cpp @@ -176,6 +176,10 @@ size_t Inst::NumArgs() const { return args.size(); } +size_t Inst::NumPhiBlocks() const { + return phi_blocks.size(); +} + Value Inst::Arg(size_t index) const { EXIT_IF(index >= args.size()); return args[index]; diff --git a/src/graphics/shader/recompiler/ir/Value.h b/src/graphics/shader/recompiler/ir/Value.h index 063e28a4..dd002839 100644 --- a/src/graphics/shader/recompiler/ir/Value.h +++ b/src/graphics/shader/recompiler/ir/Value.h @@ -116,6 +116,7 @@ public: [[nodiscard]] bool HasUses() const; [[nodiscard]] size_t UseCount() const; [[nodiscard]] size_t NumArgs() const; + [[nodiscard]] size_t NumPhiBlocks() const; [[nodiscard]] Value Arg(size_t index) const; [[nodiscard]] Block* PhiBlock(size_t index) const; [[nodiscard]] Block* Parent() const; diff --git a/src/graphics/shader/recompiler/ir/ValueProgram.cpp b/src/graphics/shader/recompiler/ir/ValueProgram.cpp index 05a68c57..9f9bea01 100644 --- a/src/graphics/shader/recompiler/ir/ValueProgram.cpp +++ b/src/graphics/shader/recompiler/ir/ValueProgram.cpp @@ -4,6 +4,7 @@ #include #include +#include #include namespace Libs::Graphics::ShaderRecompiler::IR { @@ -140,15 +141,160 @@ bool ValidateValueProgram(const ValueProgram& program, bool require_ssa, std::st program.blocks.size() != program.block_storage.size()) { return Fail(error, "value IR block storage is inconsistent"); } + std::unordered_set blocks; + std::unordered_map blocks_by_id; + std::unordered_set instructions; for (size_t block_index = 0; block_index < program.blocks.size(); block_index++) { const auto* block = program.blocks[block_index]; - if (block != program.block_storage[block_index].get()) { + if (block == nullptr || program.block_storage[block_index] == nullptr || + block != program.block_storage[block_index].get()) { return Fail(error, "value IR block pointer is inconsistent"); } + if (!blocks.insert(block).second) { + return Fail(error, "value IR block pointer is duplicated"); + } + if (!blocks_by_id.emplace(program.block_info[block_index].id, block).second) { + return Fail(error, "value IR block id is duplicated"); + } + for (const auto& inst: *block) { + if (!instructions.insert(&inst).second) { + return Fail(error, "value IR instruction is duplicated"); + } + } + } + + for (size_t block_index = 0; block_index < program.blocks.size(); block_index++) { + const auto* block = program.blocks[block_index]; + std::unordered_set predecessors; + for (const auto* predecessor: block->ImmPredecessors()) { + if (predecessor == nullptr || !blocks.contains(predecessor)) { + return Fail(error, "value IR block has a foreign predecessor"); + } + if (!predecessors.insert(predecessor).second) { + return Fail(error, "value IR block predecessor is duplicated"); + } + if (std::ranges::find(predecessor->ImmSuccessors(), block) == + predecessor->ImmSuccessors().end()) { + return Fail(error, "value IR predecessor edge is not reciprocal"); + } + } + + std::unordered_set successors; + for (const auto* successor: block->ImmSuccessors()) { + if (successor == nullptr || !blocks.contains(successor)) { + return Fail(error, "value IR block has a foreign successor"); + } + if (!successors.insert(successor).second) { + return Fail(error, "value IR block successor is duplicated"); + } + if (std::ranges::find(successor->ImmPredecessors(), block) == + successor->ImmPredecessors().end()) { + return Fail(error, "value IR successor edge is not reciprocal"); + } + } + + std::unordered_set expected_successors; + const auto add_target = [&](uint32_t id) { + const auto found = blocks_by_id.find(id); + if (found == blocks_by_id.end()) { + return false; + } + expected_successors.insert(found->second); + return true; + }; + const auto& terminator = program.block_info[block_index].terminator; + const auto validate_control_value = [&](Value value, Type type) { + if (value.IsEmpty() || value.GetType() != type) { + return false; + } + const auto* definition = value.TryInstruction(); + return definition == nullptr || instructions.contains(definition); + }; + switch (terminator.kind) { + case CFG::TerminatorKind::Branch: + if (!add_target(terminator.true_block)) { + return Fail(error, "value IR branch target is missing"); + } + break; + case CFG::TerminatorKind::ConditionalBranch: + if (!add_target(terminator.true_block) || !add_target(terminator.false_block)) { + return Fail(error, "value IR conditional branch target is missing"); + } + if (!validate_control_value(program.block_info[block_index].condition, Type::U1)) { + return Fail(error, "value IR conditional branch condition is invalid"); + } + break; + case CFG::TerminatorKind::IndirectBranch: + if (!validate_control_value(program.block_info[block_index].indirect_target, + Type::U32)) { + return Fail(error, "value IR indirect branch selector is invalid"); + } + for (const auto target: terminator.indirect_targets) { + if (!add_target(target)) { + return Fail(error, "value IR indirect branch target is missing"); + } + } + if (terminator.indirect_selector_values.size() != + terminator.indirect_selector_targets.size()) { + return Fail(error, "value IR indirect selector table is inconsistent"); + } + for (const auto target: terminator.indirect_selector_targets) { + const auto found = blocks_by_id.find(target); + if (found == blocks_by_id.end() || + !expected_successors.contains(found->second)) { + return Fail(error, + "value IR indirect selector target is not a CFG successor"); + } + } + break; + case CFG::TerminatorKind::Return: + case CFG::TerminatorKind::Unsupported: break; + } + if ((terminator.merge_block != UINT32_MAX && + !blocks_by_id.contains(terminator.merge_block)) || + (terminator.continue_block != UINT32_MAX && + !blocks_by_id.contains(terminator.continue_block))) { + return Fail(error, "value IR structured control target is missing"); + } + if (successors != expected_successors) { + return Fail(error, "value IR terminator and successor graph disagree"); + } + + bool saw_non_phi = false; for (const auto& inst: *block) { if (inst.Parent() != block) { return Fail(error, "value IR instruction has the wrong parent block"); } + if (inst.GetOpcode() == ValueOpcode::Phi) { + if (saw_non_phi) { + return Fail(error, "value IR Phi appears after a non-Phi instruction"); + } + if (inst.NumPhiBlocks() != inst.NumArgs()) { + return Fail(error, "value IR Phi parent table is inconsistent"); + } + if (inst.NumArgs() == 0) { + return Fail(error, "value IR Phi has no incoming values"); + } + std::unordered_set incoming_blocks; + for (size_t arg_index = 0; arg_index < inst.NumArgs(); arg_index++) { + const auto* predecessor = inst.PhiBlock(arg_index); + if (predecessor == nullptr || !blocks.contains(predecessor) || + !predecessors.contains(predecessor)) { + return Fail(error, "value IR Phi has a foreign or non-predecessor parent"); + } + if (!incoming_blocks.insert(predecessor).second) { + return Fail(error, "value IR Phi parent is duplicated"); + } + if (inst.Arg(arg_index).GetType() != inst.GetType()) { + return Fail(error, "value IR Phi incoming type does not match its result"); + } + } + if (incoming_blocks != predecessors) { + return Fail(error, "value IR Phi does not cover every predecessor"); + } + } else { + saw_non_phi = true; + } if (require_ssa && IsRegisterStatePseudo(inst.GetOpcode())) { return Fail(error, fmt::format("register-state pseudo {} survived SSA rewrite", ValueOpcodeName(inst.GetOpcode()))); @@ -165,8 +311,8 @@ bool ValidateValueProgram(const ValueProgram& program, bool require_ssa, std::st ValueOpcodeName(inst.GetOpcode()))); } if (const auto* definition = arg.TryInstruction(); - definition != nullptr && definition->Parent() == nullptr) { - return Fail(error, "value IR argument has a detached definition"); + definition != nullptr && !instructions.contains(definition)) { + return Fail(error, "value IR argument has a foreign definition"); } if (const auto* definition = arg.TryInstruction(); definition != nullptr) { const auto& uses = definition->Uses(); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 22f26420..983349ff 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -39,6 +39,8 @@ #include #include #include +#include +#include #include namespace Libs::Graphics { @@ -191,6 +193,63 @@ uint32_t SpirvInstructionOpcodeCount(const std::vector &binary, return count; } +void CheckSpirvPhiParents(const std::vector &binary) { + struct PhiParents { + uint32_t target = 0; + std::vector parents; + }; + std::unordered_map> successors; + std::vector phis; + uint32_t current_label = 0; + for (size_t i = 5; i < binary.size();) { + const uint32_t word_count = binary[i] >> 16u; + const uint32_t opcode = binary[i] & 0xffffu; + Check(word_count != 0u && i + word_count <= binary.size(), + "SPIR-V instruction stream is malformed"); + switch (opcode) { + case 245u: { // OpPhi + Check(current_label != 0u && word_count >= 5u && + ((word_count - 3u) % 2u) == 0u, + "OpPhi has malformed incoming operands"); + PhiParents phi{.target = current_label}; + for (size_t operand = 4; operand < word_count; operand += 2u) { + phi.parents.push_back(binary[i + operand]); + } + phis.push_back(std::move(phi)); + break; + } + case 248u: // OpLabel + Check(word_count == 2u, "OpLabel has malformed operands"); + current_label = binary[i + 1u]; + break; + case 249u: // OpBranch + successors[current_label].insert(binary[i + 1u]); + break; + case 250u: // OpBranchConditional + successors[current_label].insert(binary[i + 2u]); + successors[current_label].insert(binary[i + 3u]); + break; + case 251u: // OpSwitch + successors[current_label].insert(binary[i + 2u]); + for (size_t operand = 4; operand < word_count; operand += 2u) { + successors[current_label].insert(binary[i + operand]); + } + break; + default: + break; + } + i += word_count; + } + for (const auto &phi : phis) { + std::unordered_set unique_parents; + for (const auto parent : phi.parents) { + Check(unique_parents.insert(parent).second && + successors[parent].contains(phi.target), + "OpPhi parent is not the physical block that branches to it"); + } + } +} + struct SpirvMetrics { size_t words = 0; uint32_t instructions = 0; @@ -6481,6 +6540,37 @@ void TestNewShaderRecompilerCfgIfElse() { CheckSpirvBinaryValidates(result.spirv); } +void TestNewShaderRecompilerCfgConsecutiveNativePhis() { + const uint32_t shader[] = { + EncodeSopc(0x06, 0, 1), // s_cmp_eq_u32 s0, s1 + EncodeSopp(0x05, 3), // s_cbranch_scc1 else + EncodeSMovB32(2, 129), // then: s2 = 1 + EncodeSMovB32(3, 130), // s3 = 2 + EncodeSopp(0x02, 2), // merge + EncodeSMovB32(2, 131), // else: s2 = 3 + EncodeSMovB32(3, 132), // s3 = 4 + EncodeVop1(0x01, 0, 2), // v_mov_b32 v0, s2 + EncodeVop1(0x01, 1, 3), // v_mov_b32 v1, s3 + EncodeExp0(0x0c, 0x3), EncodeExp1(0, 0, 1, 0), // POS0.xy + EncodeSopp(0x01), + }; + + auto options = MakeCompileOptions(ShaderType::Vertex); + options.dump_ir = true; + + ShaderRecompiler::CompileResult result; + std::string error; + const bool compiled = + ShaderRecompiler::TryRecompile(shader, options, result, &error); + Check(compiled, error.c_str()); + Check(CountSourceOccurrences(result.ir_dump, " = Phi") == 2u && + SpirvInstructionOpcodeCount(result.spirv, 245u) == 2u, + "consecutive typed Phis were not emitted as two native OpPhi " + "instructions"); + CheckSpirvPhiParents(result.spirv); + CheckSpirvBinaryValidates(result.spirv); +} + void TestNewShaderRecompilerCfgTerminalExitMergePS() { const uint32_t shader[] = { EncodeSopp(0x04, 2), // s_cbranch_scc0 end @@ -6679,6 +6769,8 @@ void TestNewShaderRecompilerCfgLoopHeaderDsReadStructured() { 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(0x1c), + EncodeMubuf1(0, 4, 1), // keep the guarded LDS result live 0xbf810000u, }; @@ -6687,13 +6779,19 @@ void TestNewShaderRecompilerCfgLoopHeaderDsReadStructured() { ShaderRecompiler::CompileResult result; std::string error; - Check(ShaderRecompiler::TryRecompile(shader, options, result, &error), - error.c_str()); + const bool compiled = + ShaderRecompiler::TryRecompile(shader, options, result, &error); + Check(compiled, error.c_str()); Check(Common::ContainsStr(result.ir_dump, "mode=structured") && !result.program.dispatcher_fallback, "DS read loop header did not stay structured"); Check(!SpirvContainsOpcode(result.spirv, 251), "DS read structured SPIR-V unexpectedly contains OpSwitch"); + const auto metrics = MeasureSpirv(result.spirv); + Check(Common::ContainsStr(result.ir_dump, "Phi") && metrics.phis != 0u && + metrics.selection_merges != 0u, + "guarded LDS loop did not exercise deferred Phi exit-label patching"); + CheckSpirvPhiParents(result.spirv); CheckSpirvBinaryValidates(result.spirv); } @@ -8142,6 +8240,109 @@ void TestFinalSsaRejectsRegisterStatePseudos() { check_rejected(true); } +void TestValuePhiValidation() { + using namespace ShaderRecompiler::IR; + + enum class InvalidPhi { + None, + Empty, + MissingParent, + DuplicateParent, + NonPredecessorParent, + WrongType, + AfterInstruction, + }; + + const auto validate = [](InvalidPhi invalid, const char *expected_error) { + ValueProgram program; + const auto add_block = [&](uint32_t id) { + program.block_storage.push_back(std::make_unique()); + auto *block = program.block_storage.back().get(); + program.blocks.push_back(block); + program.block_info.push_back({.id = id}); + return block; + }; + + auto *entry = add_block(0); + auto *left = add_block(1); + auto *right = add_block(2); + auto *join = add_block(3); + program.block_info[0].terminator.kind = + ShaderRecompiler::CFG::TerminatorKind::ConditionalBranch; + program.block_info[0].terminator.true_block = 1; + program.block_info[0].terminator.false_block = 2; + program.block_info[0].condition = Value(true); + program.block_info[1].terminator.kind = + ShaderRecompiler::CFG::TerminatorKind::Branch; + program.block_info[1].terminator.true_block = 3; + program.block_info[2].terminator.kind = + ShaderRecompiler::CFG::TerminatorKind::Branch; + program.block_info[2].terminator.true_block = 3; + entry->AddBranch(left); + entry->AddBranch(right); + left->AddBranch(join); + right->AddBranch(join); + + if (invalid == InvalidPhi::AfterInstruction) { + join->AppendNewInst(ValueOpcode::IAdd32, {Value(1u), Value(2u)}); + } + auto &phi = join->AppendNewInst(ValueOpcode::Phi); + phi.SetFlags(Type::U32); + if (invalid != InvalidPhi::Empty) { + phi.AddPhiOperand( + invalid == InvalidPhi::NonPredecessorParent ? entry : left, + invalid == InvalidPhi::WrongType ? Value(true) : Value(7u)); + } + if (invalid != InvalidPhi::Empty && invalid != InvalidPhi::MissingParent && + invalid != InvalidPhi::WrongType) { + phi.AddPhiOperand(invalid == InvalidPhi::DuplicateParent ? left : right, + Value(9u)); + } else if (invalid == InvalidPhi::WrongType) { + phi.AddPhiOperand(right, Value(9u)); + } + + std::string error; + const bool valid = ValidateValueProgram(program, true, &error); + if (invalid == InvalidPhi::None) { + Check(valid, error.c_str()); + } else { + Check(!valid && Common::ContainsStr(error, expected_error), + "malformed Phi was not rejected by its structural invariant"); + } + }; + + validate(InvalidPhi::None, ""); + validate(InvalidPhi::Empty, "no incoming"); + validate(InvalidPhi::MissingParent, "cover every predecessor"); + validate(InvalidPhi::DuplicateParent, "duplicated"); + validate(InvalidPhi::NonPredecessorParent, "non-predecessor"); + validate(InvalidPhi::WrongType, "incoming type"); + validate(InvalidPhi::AfterInstruction, "after a non-Phi"); + + const auto validate_cfg = [](bool duplicate_id) { + ValueProgram program; + for (uint32_t id = 0; id < 2; id++) { + program.block_storage.push_back(std::make_unique()); + auto *block = program.block_storage.back().get(); + program.blocks.push_back(block); + program.block_info.push_back({.id = duplicate_id ? 0u : id}); + } + if (!duplicate_id) { + program.block_info[0].terminator.kind = + ShaderRecompiler::CFG::TerminatorKind::Branch; + program.block_info[0].terminator.true_block = 1; + } + std::string error; + Check(!ValidateValueProgram(program, true, &error) && + Common::ContainsStr(error, duplicate_id + ? "id is duplicated" + : "successor graph disagree"), + "malformed Value CFG was not rejected before SPIR-V emission"); + }; + validate_cfg(true); + validate_cfg(false); +} + void TestNewShaderRecompilerZeroInitialRegisterState() { const uint32_t shader[] = { EncodeVop1(0x01, 0, 100), // v_mov_b32 v0, s100 @@ -9847,8 +10048,9 @@ void TestNewShaderRecompilerSpirvSizeBaselines() { ShaderRecompiler::CompileResult result; std::string error; - Check(ShaderRecompiler::TryRecompile(shader, options, result, &error), - error.c_str()); + const bool compiled = + ShaderRecompiler::TryRecompile(shader, options, result, &error); + Check(compiled, error.c_str()); CheckSpirvBinaryValidates(result.spirv); CheckSpirvBudget(name, result.spirv, budget); return result; @@ -9875,18 +10077,32 @@ void TestNewShaderRecompilerSpirvSizeBaselines() { EncodeSopp(0x01), }; const auto structured_result = compile("structured-phi", structured_phi, - {.words = 147, - .instructions = 43, - .variables = 1, - .function_variables = 1, - .loads = 1, - .stores = 2, + {.words = 136, + .instructions = 39, + .phis = 1, .labels = 7, .loop_merges = 1, .branches = 5, .conditional_branches = 1}); + const auto structured_metrics = MeasureSpirv(structured_result.spirv); Check(Common::ContainsStr(structured_result.ir_dump, "Phi"), "structured Phi size fixture no longer contains an IR Phi"); + Check(structured_metrics.phis == 1u && + structured_metrics.function_variables == 0u && + structured_metrics.loads == 0u && structured_metrics.stores == 0u, + "structured Phi retained a spill variable or edge memory operation"); + CheckSpirvPhiParents(structured_result.spirv); + const auto structured_repeat = + compile("structured-phi-repeat", structured_phi, + {.words = 136, + .instructions = 39, + .phis = 1, + .labels = 7, + .loop_merges = 1, + .branches = 5, + .conditional_branches = 1}); + Check(structured_repeat.spirv == structured_result.spirv, + "deferred Phi patching is not deterministic"); const uint32_t wide_buffer[] = { EncodeMubuf0(0x0e, 0), @@ -9958,6 +10174,8 @@ void TestNewShaderRecompilerSpirvSizeBaselines() { .conditional_branches = 1, .switches = 1}); Check(dispatcher_result.program.dispatcher_fallback && + Common::ContainsStr(dispatcher_result.ir_dump, "Phi") && + SpirvInstructionOpcodeCount(dispatcher_result.spirv, 245u) == 0u && SpirvInstructionOpcodeCount(dispatcher_result.spirv, 251u) == 1u, "dispatcher size fixture no longer exercises whole-function dispatch"); } @@ -9991,6 +10209,7 @@ int main() { TestNewShaderRecompilerFlatAddressProvenanceBoundaries(); TestNewShaderRecompilerCfgStraightLine(); TestNewShaderRecompilerCfgIfElse(); + TestNewShaderRecompilerCfgConsecutiveNativePhis(); TestNewShaderRecompilerCfgTerminalExitMergePS(); TestNewShaderRecompilerCfgPostEndTargetMergePS(); TestNewShaderRecompilerCfgLoopBreakContinue(); @@ -10028,6 +10247,7 @@ int main() { TestNewShaderRecompilerSetpcDwordJumpTable(); TestTypedEntryStateIsMinimal(); TestFinalSsaRejectsRegisterStatePseudos(); + TestValuePhiValidation(); TestNewShaderRecompilerZeroInitialRegisterState(); TestNewShaderRecompilerVertexSystemInputsWithoutMirrors(); TestNewShaderRecompilerVertexExportUsesLaneExecMask();