diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index 472e3a02a..0e5ebc9a8 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -609,23 +609,37 @@ private: return selected.IsEmpty() ? value : selected; } - Value LowerDescriptorPhi(Value value) { - value = value.Resolve(); - const auto* phi = value.TryInstruction(); - if (m_shader_writes || phi == nullptr || phi->GetOpcode() != ValueOpcode::Phi || - phi->NumArgs() != 2u || phi->NumPhiBlocks() != 2u || phi->GetType() != Type::U32) { - return value; - } - 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; - } - for (const auto& [original, selected]: m_descriptor_selections) { - if (original == phi) { - return selected; + Value LowerDescriptorValue(Value value) { + value = value.Resolve(); + const auto* inst = value.TryInstruction(); + if (inst == nullptr) return value; + // The scalar planner owns reads and lowers their address handles separately. + uint32_t memory_index = 0; + if (ScalarReadMemory(*inst, memory_index) != nullptr) return value; + const auto [cached, inserted] = m_descriptor_values.emplace(inst, value); + if (!inserted) return cached->second; + if (inst->GetOpcode() != ValueOpcode::Phi) { + Inst* projected = nullptr; + for (size_t arg = 0; arg < inst->NumArgs(); ++arg) { + const auto lowered = LowerDescriptorValue(inst->Arg(arg)); + if (lowered == inst->Arg(arg).Resolve()) continue; + if (projected == nullptr) { + projected = &m_program.value_storage.emplace_back(inst->GetOpcode(), inst->Flags()); + for (size_t i = 0; i < inst->NumArgs(); ++i) projected->SetArg(i, inst->Arg(i)); + } + projected->SetArg(arg, lowered); } + if (projected != nullptr) cached->second = Value(projected); + return cached->second; + } + if (inst->NumArgs() != 2u || inst->NumPhiBlocks() != 2u || inst->GetType() != Type::U32) { + return value; + } + const auto* merge = inst->Parent(); + const auto* branch = inst->PhiBlock(0); + if (merge == nullptr || branch == nullptr || inst->PhiBlock(1) == nullptr || + branch == inst->PhiBlock(1)) { + return value; } if (branch->ImmSuccessors().size() != 2u) { if (branch->ImmPredecessors().size() != 1u) { @@ -638,7 +652,7 @@ private: } std::array targets; for (uint32_t arm = 0; arm < 2; arm++) { - const auto* incoming = phi->PhiBlock(arm); + const auto* incoming = inst->PhiBlock(arm); if (incoming == merge || (incoming != branch && (incoming->ImmPredecessors().size() != 1u || @@ -654,19 +668,20 @@ private: if (term.kind != CFG::TerminatorKind::ConditionalBranch || !((term.true_block == targets[0] && term.false_block == targets[1]) || (term.false_block == targets[0] && term.true_block == targets[1])) || - !ValidateRuntimeValue(m_program, info.condition, RuntimeValueType::Integer) || - !ValidateRuntimeValue(m_program, phi->Arg(0)) || - !ValidateRuntimeValue(m_program, phi->Arg(1))) { + !ValidateRuntimeValue(m_program, info.condition, m_shader_writes + ? RuntimeValueType::ImmutableInteger : RuntimeValueType::Integer) || + !ValidateRuntimeValue(m_program, inst->Arg(0)) || + !ValidateRuntimeValue(m_program, inst->Arg(1))) { return value; } // Retain a host expression; replacing the GPU Phi would break SSA dominance. const auto true_arg = term.true_block == targets[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); + selected.SetArg(1, inst->Arg(true_arg)); + selected.SetArg(2, inst->Arg(true_arg ^ 1u)); + cached->second = Value(&selected); + return cached->second; } void MakeSource(const Inst& handle, uint32_t width, bool sampler, bool sample_adjust, @@ -686,7 +701,7 @@ private: for (uint32_t i = 0; i < width; i++) { const auto value = base_reg != UINT32_MAX ? NativeDescriptorSource(handle.Arg(i), base_reg + i, pc) : handle.Arg(i); - descriptor.dwords[i] = LowerDescriptorPhi(value); + descriptor.dwords[i] = LowerDescriptorValue(value); } if (sample_adjust) { descriptor.dwords[3] = CanonicalizeSampleAdjustDword3(descriptor.dwords[3]); @@ -2397,7 +2412,7 @@ private: std::vector m_handle_patches; std::vector m_memory_patches; std::vector m_indirect_descriptors; - std::vector> m_descriptor_selections; + std::map m_descriptor_values; bool m_shader_writes = false; }; diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp index 12ecdfc90..ca294b421 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp @@ -159,7 +159,7 @@ private: bool Validate(Value value, bool require_uniform = true) { value = value.Resolve(); // Host floating-point evaluation does not model shader rounding/denormal modes. - if (m_type == RuntimeValueType::Integer && + if (m_type != RuntimeValueType::Any && TypesOverlap(value.GetType(), Type::F16 | Type::F32 | Type::F32x2)) { return false; } @@ -188,6 +188,13 @@ private: return valid; }; const auto op = inst->GetOpcode(); + if (m_type == RuntimeValueType::ImmutableInteger && + (op == ValueOpcode::ReadConst || BufferAccessOf(op) != BufferAccess::None || + AddressOpcodeInfoOf(op).access != AddressAccess::None || + ImageOpcodeInfoOf(op).access != ImageAccess::None || + SharedAccessOf(op) != SharedAccess::None)) { + return finish(false); + } if (op == ValueOpcode::ReadConst) { const auto slot = inst->NumArgs() == 2 ? inst->Arg(1).Resolve() : Value {}; if (inst->NumArgs() != 2 || inst->Arg(0).Resolve().TryInstruction() == nullptr || @@ -197,7 +204,7 @@ private: slot.U32() >= m_program.srt_reads.size()) { return finish(false); } - if (m_type == RuntimeValueType::Integer) { + if (m_type != RuntimeValueType::Any) { const auto active_mask = m_active_mask; m_active_mask = {}; const bool valid = Validate(m_program.srt_reads[slot.U32()].value); @@ -209,7 +216,7 @@ private: if (!m_active_mask.IsEmpty() && IsRuntimeSelect(op) && inst->NumArgs() == 3 && inst->Arg(0).Resolve() == m_active_mask) { // Empty EXEC reads lane zero, so ignored operands still require integer types. - if (m_type == RuntimeValueType::Integer && !Validate(inst->Arg(2), false)) { + if (m_type != RuntimeValueType::Any && !Validate(inst->Arg(2), false)) { return finish(false); } return finish(Validate(inst->Arg(1))); @@ -237,7 +244,7 @@ private: return finish(true); } if (op == ValueOpcode::Phi) { - if (m_type == RuntimeValueType::Integer && !ValidateArguments(*inst, false)) { + if (m_type != RuntimeValueType::Any && !ValidateArguments(*inst, false)) { return finish(false); } const auto invariant = ResolveInvariantPhi(m_program, value); @@ -251,7 +258,7 @@ private: inst->Arg(1).GetType() != Type::U1) { return finish(false); } - if (m_type == RuntimeValueType::Integer && !Validate(inst->Arg(1), false)) { + if (m_type != RuntimeValueType::Any && !Validate(inst->Arg(1), false)) { return finish(false); } const auto active_mask = m_active_mask; diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.h b/src/graphics/shader/recompiler/ir/passes/SrtWalker.h index 32a057c6e..2628a4fa2 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.h +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.h @@ -27,7 +27,7 @@ struct SrtReadFlags { uint32_t clean = 0; }; -enum class RuntimeValueType { Any, Integer }; +enum class RuntimeValueType { Any, Integer, ImmutableInteger }; bool ValidateRuntimeValue(const ResourcePlan& program, Value value, RuntimeValueType type = RuntimeValueType::Any); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index f603117c1..ec1c23051 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -13682,6 +13682,127 @@ void TestGpuProducedWritableDescriptor() { "GPU-updated address or record count created a shader permutation"); } +void TestImmutableDescriptorPredicate() { + using namespace ShaderRecompiler::IR; + ResourcePlan plan; + const auto emit = [&](ValueOpcode op, std::initializer_list args) { + auto &inst = plan.value_storage.emplace_back(op); + uint32_t index = 0; + for (const auto arg : args) inst.SetArg(index++, arg); + return Value(&inst); + }; + const auto handle = emit(ValueOpcode::GetBufferResource, + {Value(0x1000u), Value(4u << 16u), Value(16u), Value(0u)}); + for (const bool coherent : {false, true}) { + plan.memory_info.push_back({.kind = ResourceKind::Buffer, + .idxen = !coherent, .coherent = coherent}); + const auto loaded = emit(ValueOpcode::LoadBufferU32, + {handle, Value(0u), Value(0u), Value(0u), Value(true)}); + loaded.ResolveInstruction()->SetFlags(MemoryFlags{ + .index = static_cast(plan.memory_info.size() - 1u)}); + const auto mask = emit(ValueOpcode::INotEqual32, {loaded, Value(0u)}); + const auto selected = emit(ValueOpcode::SelectU32, {mask, Value(1u), Value(0u)}); + const auto reduced = emit(ValueOpcode::ReadFirstLane, {selected, mask}); + Check(ValidateRuntimeValue(plan, reduced, RuntimeValueType::Integer), + "integer EXEC reduction lost its active-lane proof"); + Check(!ValidateRuntimeValue(plan, reduced, RuntimeValueType::ImmutableInteger), + "memory-dependent EXEC reduction was classified as immutable"); + } +} + +void TestUniformSelectedWritableDescriptor() { + using namespace ShaderRecompiler::IR; + // SAROS b9da5e64f4c5b10a builds s[8:11] from two scalar address expressions, + // then sets the stride in the high address word after the branch merge. + const uint32_t shader[] = { + 0xbfa00002u, 0xbf070280u, 0xbf84000du, 0xf4001a82u, + 0xfa00009cu, 0xbf8cc07fu, 0x8f6b856au, 0xf4001a82u, + 0xfa000080u, 0xbf8cc07fu, 0x98eb6b6au, 0xb7eb0080u, + 0x80086b03u, 0xbf800000u, 0x82098000u, 0xbf820006u, + 0xf4041a82u, 0xfa000040u, 0xbf8cc07fu, 0x8008066au, + 0xbf800000u, 0x8209016bu, 0xd7460001u, 0x04010c07u, + 0x7e000280u, 0xf4001a82u, 0xfa000084u, 0xbf8cc07fu, + 0x810a846au, 0xbe8b03ffu, 0x00016204u, 0x4a020284u, + 0xbe891d92u, 0xe0702000u, 0x80020001u, 0xbf810000u, + }; + std::array user_data{0x30u, 0x1cu, 1u, 0xfffffff0u, + 0x1000u, 0u, 0xfffffffcu, 0u}; + auto options = MakeCompileOptions(ShaderType::Compute); + options.user_data = user_data; + auto translated = ShaderRecompiler::TranslateProgram(shader, options); + auto plan = ExtractResourcePlan(translated.program); + Check(plan.info.buffers.size() == 1 && plan.info.buffers[0].written, + "uniform address selection lost its writable buffer"); + const auto read = +[](void *data, uint64_t address, std::span words) { + const bool alternate = *static_cast(data) == 0; + if (words.size() != 1) return false; + switch (address) { + case 0x1040: words[0] = 8u; return alternate; + case 0x1044: words[0] = 1u; return alternate; + case 0x1080: words[0] = 3u; return !alternate; + case 0x109c: words[0] = 2u; return !alternate; + case 0x1084: words[0] = 10u; return true; + default: return false; + } + }; + const SrtRuntime runtime{.user_data = user_data, .read_memory = read, + .userdata = &user_data[2], .read_specialization_memory = read}; + ResourceSnapshot snapshot; + ResourceSpecialization specialization; + for (const uint32_t flag : {1u, 0u, 1u}) { + user_data[2] = flag; + Check(MaterializeResources(plan, runtime, snapshot, specialization), + "uniform selection evaluated an unavailable untaken scalar source"); + const auto &descriptor = snapshot.buffers[0].dwords; + Check(descriptor[0] == (flag ? 0xe0u : 4u) && + descriptor[1] == (flag ? 0x40031u : 0x4001eu) && + descriptor[2] == 14u && descriptor[3] == 0x16204u, + "uniform writable descriptor lost its branch, carry, or post-merge stride"); + } +} + +void TestUniformSelectedDescriptorLoadAddress() { + using namespace ShaderRecompiler::IR; + const uint32_t shader[] = { + EncodeSopc(0x06, 0, 128), // s_cmp_eq_u32 s0, 0 + EncodeSopp(0x04, 2), // s_cbranch_scc0 alternate + EncodeSop1(0x04, 6, 2), // s_mov_b64 s[6:7], s[2:3] + EncodeSopp(0x02, 1), // s_branch load + EncodeSop1(0x04, 6, 4), // s_mov_b64 s[6:7], s[4:5] + EncodeSmem0(0x02, 8, 3), 125u << 25u, // s_load_dwordx4 s[8:11], s[6:7], 0 + EncodeMubuf0(0x1c), EncodeMubuf1(0, 2, 0), + EncodeSopp(0x01), + }; + std::array user_data{0u, 0u, 0x1000u, 0u, 0x2000u, 0u}; + auto options = MakeCompileOptions(ShaderType::Compute); + options.user_data = user_data; + auto translated = ShaderRecompiler::TranslateProgram(shader, options); + auto plan = ExtractResourcePlan(translated.program); + Check(plan.srt_reads.size() == 4u && plan.info.buffers.size() == 1u && + plan.info.buffers[0].written, + "conditional scalar address duplicated or detached descriptor reads"); + const auto read = +[](void *data, uint64_t address, std::span words) { + const auto flag = *static_cast(data); + const uint64_t base = flag ? 0x2000u : 0x1000u; + const std::array descriptor{0x3000u + flag * 0x1000u, + 4u << 16u, 16u, 0x16204u}; + if (words.size() != 1u || address < base || address >= base + 16u || + (address & 3u) != 0u) return false; + words[0] = descriptor[(address - base) / 4u]; + return true; + }; + const SrtRuntime runtime{.user_data = user_data, .read_memory = read, + .userdata = &user_data[0], .read_specialization_memory = read}; + ResourceSnapshot snapshot; + ResourceSpecialization specialization; + for (const uint32_t flag : {0u, 1u, 0u}) { + user_data[0] = flag; + Check(MaterializeResources(plan, runtime, snapshot, specialization) && + snapshot.buffers[0].dwords[0] == 0x3000u + flag * 0x1000u, + "conditional descriptor load read the untaken pointer"); + } +} + void TestTypedDescriptorRealCarryAndScalarLoads() { const uint32_t carry_shader[] = { EncodeSop1(0x1f, 0, 0), // s_getpc_b64 s[0:1] @@ -15153,6 +15274,9 @@ int main() { TestTypedDescriptorRealWideMoveTranslation(); TestComputeImageFill(); TestGpuProducedWritableDescriptor(); + TestUniformSelectedWritableDescriptor(); + TestUniformSelectedDescriptorLoadAddress(); + TestImmutableDescriptorPredicate(); TestTypedDescriptorRealCarryAndScalarLoads(); TestSrtWalkerRealSmemTranslation(); TestSrtWalkerVccBaseTranslation();