diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index 46c1fd407..5e0678469 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -182,47 +182,61 @@ private: return selected; } } - const auto* merge = phi->Parent(); - for (uint32_t first = 0; first < 2; first++) { - const auto* branch = phi->PhiBlock(first); - const auto* alternate = phi->PhiBlock(first ^ 1u); - if (branch == nullptr || alternate == nullptr || merge == nullptr || branch == merge || - alternate == merge || branch == alternate || branch->ImmSuccessors().size() != 2u || - alternate->ImmPredecessors().size() != 1u || - alternate->ImmPredecessors()[0] != branch || - alternate->ImmSuccessors().size() != 1u || alternate->ImmSuccessors()[0] != merge) { - continue; - } - const auto branch_it = std::ranges::find(m_program.blocks, branch); - const auto alternate_it = std::ranges::find(m_program.blocks, alternate); - const auto merge_it = std::ranges::find(m_program.blocks, merge); - if (branch_it == m_program.blocks.end() || alternate_it == m_program.blocks.end() || - merge_it == m_program.blocks.end()) { - continue; - } - const auto& info = m_program.block_info[branch_it - m_program.blocks.begin()]; - const auto alternate_id = - m_program.block_info[alternate_it - m_program.blocks.begin()].id; - const auto merge_id = m_program.block_info[merge_it - m_program.blocks.begin()].id; - const auto& term = info.terminator; - if (term.kind != CFG::TerminatorKind::ConditionalBranch || - !((term.true_block == merge_id && term.false_block == alternate_id) || - (term.false_block == merge_id && term.true_block == alternate_id)) || - !ValidateRuntimeValue(m_program, info.condition, RuntimeValueType::Integer) || - !ValidateRuntimeValue(m_program, phi->Arg(0)) || - !ValidateRuntimeValue(m_program, phi->Arg(1))) { - continue; - } - // Retain a host expression; replacing the GPU Phi would break SSA dominance. - const auto true_arg = term.true_block == merge_id ? first : first ^ 1u; - auto& selected = m_program.value_storage.emplace_back(ValueOpcode::SelectU32); - selected.SetArg(0, info.condition); - selected.SetArg(1, phi->Arg(true_arg)); - selected.SetArg(2, phi->Arg(true_arg ^ 1u)); - m_descriptor_selections.emplace_back(phi, Value(&selected)); - return Value(&selected); + const auto* merge = phi->Parent(); + const auto* branch = phi->PhiBlock(0); + if (merge == nullptr || branch == nullptr || phi->PhiBlock(1) == nullptr || + branch == phi->PhiBlock(1)) { + return value; } - return value; + if (branch->ImmSuccessors().size() != 2u) { + if (branch->ImmPredecessors().size() != 1u) { + return value; + } + branch = branch->ImmPredecessors()[0]; + } + if (branch == merge || branch->ImmSuccessors().size() != 2u) { + return value; + } + std::array target_ids; + for (uint32_t arm = 0; arm < 2; arm++) { + const auto* incoming = phi->PhiBlock(arm); + if (incoming == merge || + (incoming != branch && + (incoming->ImmPredecessors().size() != 1u || + incoming->ImmPredecessors()[0] != branch || + incoming->ImmSuccessors().size() != 1u || + incoming->ImmSuccessors()[0] != merge))) { + return value; + } + const auto* target = incoming == branch ? merge : incoming; + const auto it = std::ranges::find(m_program.blocks, target); + if (it == m_program.blocks.end()) { + return value; + } + target_ids[arm] = m_program.block_info[it - m_program.blocks.begin()].id; + } + const auto branch_it = std::ranges::find(m_program.blocks, branch); + if (branch_it == m_program.blocks.end()) { + return value; + } + const auto& info = m_program.block_info[branch_it - m_program.blocks.begin()]; + const auto& term = info.terminator; + if (term.kind != CFG::TerminatorKind::ConditionalBranch || + !((term.true_block == target_ids[0] && term.false_block == target_ids[1]) || + (term.false_block == target_ids[0] && term.true_block == target_ids[1])) || + !ValidateRuntimeValue(m_program, info.condition, RuntimeValueType::Integer) || + !ValidateRuntimeValue(m_program, phi->Arg(0)) || + !ValidateRuntimeValue(m_program, phi->Arg(1))) { + return value; + } + // Retain a host expression; replacing the GPU Phi would break SSA dominance. + const auto true_arg = term.true_block == target_ids[0] ? 0u : 1u; + auto& selected = m_program.value_storage.emplace_back(ValueOpcode::SelectU32); + selected.SetArg(0, info.condition); + selected.SetArg(1, phi->Arg(true_arg)); + selected.SetArg(2, phi->Arg(true_arg ^ 1u)); + m_descriptor_selections.emplace_back(phi, Value(&selected)); + return Value(&selected); } void MakeSource(const Inst& handle, uint32_t width, bool sampler, bool sample_adjust, diff --git a/tests/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index 51774c938..cd8de932e 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -1208,28 +1208,40 @@ void TestPhiValidation() { "control-dependent descriptor phi was not rejected transactionally"); } -ResourcePlan ConditionalSamplerPlan(bool reverse, bool nonuniform = false, +ResourcePlan ConditionalSamplerPlan(bool diamond, bool reverse, bool reverse_phi, + bool nonuniform = false, bool writable = false) { namespace CFG = Libs::Graphics::ShaderRecompiler::CFG; Fixture fixture(ShaderType::Pixel); auto *entry = fixture.block; + auto *initial = diamond ? fixture.AddBlock() : entry; auto *alternate = fixture.AddBlock(); auto *merge = fixture.AddBlock(); + const uint32_t alternate_id = diamond ? 2u : 1u; + const uint32_t merge_id = alternate_id + 1; + const uint32_t initial_target = diamond ? 1u : merge_id; entry->AddBranch(alternate); - entry->AddBranch(merge); + entry->AddBranch(diamond ? initial : merge); alternate->AddBranch(merge); + if (diamond) { + initial->AddBranch(merge); + fixture.program.block_info[1].terminator = { + .kind = CFG::TerminatorKind::Branch, .true_block = merge_id}; + } fixture.program.block_info[0].terminator = { .kind = CFG::TerminatorKind::ConditionalBranch, - .true_block = reverse ? 1u : 2u, .false_block = reverse ? 2u : 1u}; - fixture.program.block_info[1].terminator = { - .kind = CFG::TerminatorKind::Branch, .true_block = 2}; - fixture.program.block_info[2].terminator.kind = CFG::TerminatorKind::Return; + .true_block = reverse ? alternate_id : initial_target, + .false_block = reverse ? initial_target : alternate_id}; + fixture.program.block_info[alternate_id].terminator = { + .kind = CFG::TerminatorKind::Branch, .true_block = merge_id}; + fixture.program.block_info[merge_id].terminator.kind = + CFG::TerminatorKind::Return; const auto control = - fixture.Buffer({Value(0x2000u), Value(0u), Value(4u), Value(0u)}); + fixture.Buffer({Value(0x2000u), Value(0u), Value(200u), Value(0u)}); MemoryInfo scalar; scalar.kind = ResourceKind::ScalarBuffer; - auto flag = fixture.Emit(ValueOpcode::ReadConstBuffer, - {control, Value(0u)}, fixture.AddMemory(scalar, 0x18c)); + auto flag = fixture.Emit(ValueOpcode::ReadConstBuffer, {control, Value(196u)}, + fixture.AddMemory(scalar, 0x498)); if (nonuniform) { flag = fixture.Emit(ValueOpcode::LaneId); } @@ -1238,46 +1250,55 @@ ResourcePlan ConditionalSamplerPlan(bool reverse, bool nonuniform = false, if (writable) { MemoryInfo memory; memory.kind = ResourceKind::Buffer; - fixture.Emit(ValueOpcode::StoreBufferU32, - {control, Value(0u), Value(0u), Value(0u), Value(1u), Value(true)}, - fixture.AddMemory(memory, 0x170)); + fixture.Emit( + ValueOpcode::StoreBufferU32, + {control, Value(0u), Value(0u), Value(0u), Value(1u), Value(true)}, + fixture.AddMemory(memory, 0x170)); } std::array sampler_words; for (uint32_t word = 0; word < sampler_words.size(); ++word) { const auto read = [&](Block *block, uint32_t address, uint32_t pc) { const auto handle = fixture.Emit(ValueOpcode::GetAddressResource, - {Value(address), Value(0u)}, 0, block); + {Value(address), Value(0u)}, 0, block); MemoryInfo memory; memory.kind = ResourceKind::ScalarAddress; memory.offset = word * 4; return fixture.Emit(ValueOpcode::LoadAddressU32, - {handle, Value(0u), Value(0u), Value(true)}, - fixture.AddMemory(memory, pc), block); + {handle, Value(0u), Value(0u), Value(true)}, + fixture.AddMemory(memory, pc), block); }; - const auto initial = read(entry, 0x1000, 0x180); - const auto override = read(alternate, 0x1010, 0x1a0); + // PS 2190adcc312b2e6e selects SRT+448 or SRT+480 before its sample at + // 0x4d4. + const auto first = read(initial, 0x1000 + 448, 0x4c8); + const auto second = read(alternate, 0x1000 + 480, 0x4bc); auto &phi = merge->AppendNewInst(ValueOpcode::Phi, {}, - static_cast(Type::U32)); - phi.AddPhiOperand(entry, initial); - phi.AddPhiOperand(alternate, override); + static_cast(Type::U32)); + if (reverse_phi) { + phi.AddPhiOperand(alternate, second); + phi.AddPhiOperand(initial, first); + } else { + phi.AddPhiOperand(initial, first); + phi.AddPhiOperand(alternate, second); + } sampler_words[word] = Value(&phi); } fixture.block = merge; - const auto image = fixture.Image({Value(0u), Value(0u), Value(0u), Value(0u), - Value(0u), Value(0u), Value(0u), Value(0u)}); + const auto image = + fixture.Image({Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u)}); const auto address = fixture.ImageAddress(); for (uint32_t use = 0; use < 2; ++use) { const auto sampler = fixture.Sampler(sampler_words); MemoryInfo sample; sample.kind = ResourceKind::Image; sample.image_dimension = Decoder::ImageDimension::Dim2D; - const auto result = fixture.Emit(ValueOpcode::ImageSampleRaw, - {image, sampler, address}, - fixture.AddMemory(sample, 0x214)); + const auto result = + fixture.Emit(ValueOpcode::ImageSampleRaw, {image, sampler, address}, + fixture.AddMemory(sample, 0x4d4)); fixture.Emit(ValueOpcode::ReferenceU32, {fixture.Emit(ValueOpcode::CompositeExtractU32x4, - {result, Value(0u)})}); + {result, Value(0u)})}); } fixture.PlanAndTrack(); Check(fixture.program.value_storage.size() == 4, @@ -1290,41 +1311,51 @@ ResourcePlan ConditionalSamplerPlan(bool reverse, bool nonuniform = false, } void TestConditionalSamplerPhi() { - for (const bool reverse : {false, true}) { - auto plan = ConditionalSamplerPlan(reverse); - const auto source = plan.info.samplers[0].source; - LinearTestMemory memory; - for (uint32_t word = 0; word < 8; ++word) { - memory.words[word] = 0x400u + word; - } - const SrtRuntime runtime{.read_memory = ReadLinearTestMemory, - .userdata = &memory, - .read_specialization_memory = ReadLinearTestMemory}; - for (uint32_t flag = 0; flag < 2; ++flag) { - memory.words[0x1000 / 4] = flag; - const uint32_t first = (flag != 0u) != reverse ? 4u : 0u; - memory.fail_address = first == 0u ? 0x1010u : 0x1000u; - DescriptorValue selected; - Check(EvaluateDescriptorSource(plan, source, runtime, selected), - "conditional sampler did not survive detached plan lifetime"); - for (uint32_t word = 0; word < 4; ++word) { - Check(selected.dwords[word] == memory.words[first + word], - "conditional sampler chose the wrong incoming descriptor"); + for (const bool diamond : {false, true}) { + for (const bool reverse : {false, true}) { + for (const bool reverse_phi : {false, true}) { + auto plan = ConditionalSamplerPlan(diamond, reverse, reverse_phi); + const auto source = plan.info.samplers[0].source; + LinearTestMemory memory; + for (uint32_t word = 448 / 4; word < (480 + 16) / 4; ++word) { + memory.words[word] = 0x400u + word; + } + const SrtRuntime runtime{.read_memory = ReadLinearTestMemory, + .userdata = &memory, + .read_specialization_memory = + ReadLinearTestMemory}; + for (uint32_t flag = 0; flag < 2; ++flag) { + memory.words[(0x1000 + 196) / 4] = flag; + const uint32_t first = (flag != 0u) != reverse ? 480 / 4 : 448 / 4; + memory.fail_address = 0x1000 + (first == 448 / 4 ? 480u : 448u); + DescriptorValue selected; + Check(EvaluateDescriptorSource(plan, source, runtime, selected), + "conditional sampler did not survive detached plan lifetime"); + for (uint32_t word = 0; word < 4; ++word) { + Check(selected.dwords[word] == memory.words[first + word], + "conditional sampler chose the wrong incoming descriptor"); + } + } + DescriptorValue selected; + auto no_clean_reader = runtime; + no_clean_reader.read_specialization_memory = nullptr; + Check( + !EvaluateDescriptorSource(plan, source, no_clean_reader, selected), + "conditional sampler used unchecked memory for its predicate"); + memory.fail_address = 0x2000 + 196; + Check(!EvaluateDescriptorSource(plan, source, runtime, selected), + "conditional sampler ignored unavailable coherent predicate " + "memory"); } } - DescriptorValue selected; - auto no_clean_reader = runtime; - no_clean_reader.read_specialization_memory = nullptr; - Check(!EvaluateDescriptorSource(plan, source, no_clean_reader, selected), - "conditional sampler used unchecked memory for its predicate"); - memory.fail_address = 0x2000; - Check(!EvaluateDescriptorSource(plan, source, runtime, selected), - "conditional sampler ignored unavailable coherent predicate memory"); + CheckFatal([&] { ConditionalSamplerPlan(diamond, false, false, true); }, + "not a valid runtime value", + "nonuniform sampler selection was accepted"); + CheckFatal( + [&] { ConditionalSamplerPlan(diamond, false, false, false, true); }, + "not a valid runtime value", + "shader-written sampler predicate was accepted"); } - CheckFatal([] { ConditionalSamplerPlan(false, true); }, - "not a valid runtime value", "nonuniform sampler selection was accepted"); - CheckFatal([] { ConditionalSamplerPlan(false, false, true); }, - "not a valid runtime value", "shader-written sampler predicate was accepted"); } void TestLoopCycleEnteredThroughRuntimeValue() {