diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index a814a3326..615077006 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -664,7 +664,6 @@ static std::vector ResourceControlFlow(const Program& program, co } } std::vector blocks(program.blocks.size()); - std::vector> predecessors(blocks.size()); for (uint32_t i = 0; i < blocks.size(); i++) { auto& block = blocks[i]; const auto& info = program.block_info[i]; @@ -688,7 +687,6 @@ static std::vector ResourceControlFlow(const Program& program, co return {}; } block.successors.push_back(found->second); - predecessors[found->second].push_back(i); } for (const auto& inst: *program.blocks[i]) { const auto op = inst.GetOpcode(); @@ -718,30 +716,7 @@ static std::vector ResourceControlFlow(const Program& program, co block.sources.erase(std::unique(block.sources.begin(), block.sources.end()), block.sources.end()); } - // Host branches only matter when they guard a later resource access. Leave - // data-only branches (for example a mutable flag checked before a trap) on the GPU. - std::vector resource_reachable(blocks.size()); - std::vector pending; - for (uint32_t i = 0; i < blocks.size(); ++i) { - if (!blocks[i].sources.empty() || !blocks[i].srt_reads.empty()) { - resource_reachable[i] = 1; - pending.push_back(i); - } - } - while (!pending.empty()) { - const auto index = pending.back(); - pending.pop_back(); - for (const auto predecessor: predecessors[index]) { - if (resource_reachable[predecessor]) continue; - resource_reachable[predecessor] = 1; - pending.push_back(predecessor); - } - } - for (auto& block: blocks) { - block.condition = std::ranges::any_of(block.successors, [&](uint32_t successor) { - return resource_reachable[successor] != 0; - }) ? predicate(block.condition) : Value {}; - } + for (auto& block: blocks) block.condition = predicate(block.condition); if (std::ranges::none_of( blocks, [](const ResourceBlock& block) { return !block.condition.IsEmpty(); })) { return {}; diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index f0035bfc0..03047fc8c 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -742,6 +742,15 @@ private: } } } + for (auto* block: m_program.blocks) { + for (auto& inst: *block) { + uint32_t index = 0; + if (inst.GetOpcode() == ValueOpcode::LoadAddressU32 && + ScalarReadMemory(inst, index) != nullptr && inst.Arg(1).Resolve().IsImmediate() && + ValidateRuntimeValue(m_program, Value(&inst))) + CollectScalarRead(Value(&inst), inst.Flags().pc); + } + } for (auto* read: m_scalar_reads) { const auto flags = read->Flags(); auto& memory = m_program.memory_info[flags.index]; diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp index 4ec6beca7..610d49eb1 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp @@ -23,6 +23,9 @@ namespace { constexpr uint64_t AddressMask = 0x0000ffffffffffffull; bool AddSignedAddress(uint64_t base, int64_t offset, uint64_t& result) { + if (base > AddressMask) { + return false; + } if (offset < 0) { const auto magnitude = uint64_t {0} - static_cast(offset); if (magnitude > base) { @@ -32,7 +35,7 @@ bool AddSignedAddress(uint64_t base, int64_t offset, uint64_t& result) { return true; } const auto magnitude = static_cast(offset); - if (magnitude > UINT64_MAX - base) { + if (magnitude > AddressMask - base) { return false; } result = base + magnitude; @@ -477,7 +480,7 @@ bool SrtWalker::EvaluateRawRead(const Inst& inst, uint64_t& result) { if (!Arg(*handle, 0, low) || !Arg(*handle, 1, high) || !Arg(inst, 1, offset)) { return false; } - const auto base = (high << 32u) | static_cast(low); + const auto base = ((high << 32u) | static_cast(low)) & AddressMask; const auto immediate = static_cast(static_cast(mem.offset)); uint64_t address = 0; if (inst.GetOpcode() == ValueOpcode::ReadConstBuffer || vector) { @@ -507,7 +510,7 @@ bool SrtWalker::EvaluateRawRead(const Inst& inst, uint64_t& result) { if (byte_offset > size || size - byte_offset < sizeof(uint32_t)) { return false; } - address = (base & AddressMask & ~uint64_t {3}) + byte_offset; + address = (base & ~uint64_t {3}) + byte_offset; } else { const auto relative = (immediate & ~int64_t {3}) + static_cast(static_cast(offset) & ~3u); diff --git a/tests/ResourceMaterializationTests.cpp b/tests/ResourceMaterializationTests.cpp index ae09cbece..b144769c5 100644 --- a/tests/ResourceMaterializationTests.cpp +++ b/tests/ResourceMaterializationTests.cpp @@ -32,8 +32,7 @@ AddValueBlock(Libs::Graphics::ShaderRecompiler::IR::Program &program) { return *result; } -Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address, - bool buffer = false) { +Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address) { using namespace Libs::Graphics::ShaderRecompiler::IR; Program program; program.stage = Libs::Graphics::ShaderType::Compute; @@ -42,22 +41,16 @@ Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address, auto &value_block = AddValueBlock(program); MemoryInfo memory; - memory.kind = buffer ? ResourceKind::ScalarBuffer : ResourceKind::ScalarAddress; + memory.kind = ResourceKind::ScalarAddress; memory.planning_only = true; program.memory_info.push_back(memory); const auto low = Value(static_cast(address)); const auto high = Value(static_cast(address >> 32u)); - auto &handle = buffer - ? value_block.AppendNewInst(ValueOpcode::GetBufferResource, - {low, high, Value(4u), Value(0u)}) - : value_block.AppendNewInst(ValueOpcode::GetAddressResource, - {low, high}); - auto &raw = buffer - ? value_block.AppendNewInst(ValueOpcode::ReadConstBuffer, - {Value(&handle), Value(0u)}) - : value_block.AppendNewInst( - ValueOpcode::LoadAddressU32, - {Value(&handle), Value(0u), Value(0u), Value(true)}); + auto &handle = + value_block.AppendNewInst(ValueOpcode::GetAddressResource, {low, high}); + auto &raw = value_block.AppendNewInst( + ValueOpcode::LoadAddressU32, + {Value(&handle), Value(0u), Value(0u), Value(true)}); raw.SetFlags(MemoryFlags{.index = 0, .pc = 0x40}); program.srt_reads.push_back({Value(&raw), 0}); @@ -175,32 +168,6 @@ void TestMappedSrtUsesDirectReaderByDefault() { "cache rematerialization did not use the direct reader by default"); } -void TestRawScalarAddressPreservesHighBits() { - using namespace Libs::Graphics::ShaderRecompiler::IR; - constexpr uint64_t address = 0xabcd000156780003ull; - for (const bool buffer : {false, true}) { - auto plan = SrtPlan(address, buffer); - uint64_t observed = 0; - const SrtRuntime runtime{ - .read_memory = [](void *userdata, uint64_t requested, - std::span words) { - if (words.size() != 1) return false; - *static_cast(userdata) = requested; - words[0] = 0x12345678; - return true; - }, - .userdata = &observed}; - ResourceSnapshot snapshot; - ResourceSpecialization specialization; - Check(MaterializeResources(plan, runtime, snapshot, specialization), - "scalar address materialization failed"); - Check(observed == (buffer ? 0x0000000156780000ull : address & ~uint64_t{3}), - "raw pointer high bits and buffer descriptor flags were conflated"); - Check(snapshot.flattened_srt == std::vector{0x12345678}, - "scalar address read did not reach its flattened slot"); - } -} - void TestIntegerRuntimeValueFollowsSrtReads() { using namespace Libs::Graphics::ShaderRecompiler::IR; auto plan = SrtPlan(0x10000); @@ -599,7 +566,6 @@ void DbgExit(int) { std::abort(); } int main() { TestMappedSrtUsesDirectReaderByDefault(); - TestRawScalarAddressPreservesHighBits(); TestIntegerRuntimeValueFollowsSrtReads(); TestUniformVectorDescriptorRead(); TestExactReciprocalDescriptorArithmetic(); diff --git a/tests/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index fa87c9dc6..de3272753 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -3096,79 +3096,6 @@ ResourcePlan ConditionalBufferPlan(ConditionalBufferUse use) { return ExtractResourcePlan(fixture.program); } -void TestNativeScalarAtomicDataDoesNotEnterResourcePlan() { - namespace CFG = Libs::Graphics::ShaderRecompiler::CFG; - Fixture fixture; - auto *entry = fixture.block; - for (uint32_t i = 1; i < 5; ++i) fixture.AddBlock(); - fixture.program.block_info[0].terminator = { - .kind = CFG::TerminatorKind::ConditionalBranch, .true_block = 1, .false_block = 4}; - for (uint32_t i = 1; i < 4; ++i) { - fixture.program.block_info[i].terminator = { - .kind = CFG::TerminatorKind::Branch, .true_block = i + 1}; - } - fixture.program.block_info[4].terminator.kind = CFG::TerminatorKind::Return; - - const auto table = fixture.Address(fixture.UserData(0), fixture.UserData(1)); - MemoryInfo scalar; - scalar.kind = ResourceKind::ScalarAddress; - scalar.component_count = 2; - scalar.offset = 136; - const auto low = fixture.Emit(ValueOpcode::LoadAddressU32, - {table, Value(0u), Value(0u), Value(true)}, fixture.AddMemory(scalar, 0x560)); - scalar.offset = 140; - scalar.component_index = 1; - const auto high = fixture.Emit(ValueOpcode::LoadAddressU32, - {table, Value(0u), Value(0u), Value(true)}, fixture.AddMemory(scalar, 0x560)); - const auto address = fixture.Address(low, high); - scalar.offset = 24; - scalar.component_count = 1; - scalar.component_index = 0; - const auto data_flags = fixture.AddMemory(scalar, 0x584); - const auto data = fixture.Emit(ValueOpcode::LoadAddressU32, - {address, Value(0u), Value(0u), Value(true)}, data_flags); - const auto descriptor = fixture.Buffer({low, - fixture.Emit(ValueOpcode::BitwiseOr32, {high, Value(256u << 16u)}), - Value(1u), Value(90628u)}, 0x58c); - MemoryInfo vector; - vector.kind = ResourceKind::Buffer; - vector.offset = 24; - vector.data_dwords = 2; - fixture.Emit(ValueOpcode::BufferAtomicOr64, - {descriptor, Value(0u), Value(0u), Value(0u), Value(uint64_t{1}), Value(true)}, - fixture.AddMemory(vector, 0x58c)); - const auto flag = fixture.Emit(ValueOpcode::BitwiseAnd32, {data, Value(16u)}); - fixture.program.block_info[0].condition = - fixture.Emit(ValueOpcode::INotEqual32, {flag, Value(0u)}); - fixture.PlanAndTrack(); - Check(data.Instruction()->Parent() == entry && - !fixture.program.memory_info[data_flags.index].planning_only && - fixture.program.info.buffers.size() == 1 && fixture.program.info.buffers[0].atomic, - "mutable scalar data was replaced by a host snapshot before its native atomic write"); - auto plan = ExtractResourcePlan(fixture.program); - Check(plan.srt_reads.size() == 2 && plan.control_flow.empty() && - !plan.capture_specialization_reads, - "resource-free successor chain retained the shader's scalar data predicate"); - struct Reads { uint32_t descriptors = 0; uint32_t data = 0; } reads; - const auto Read = +[](void *userdata, uint64_t address, std::span words) { - auto &reads = *static_cast(userdata); - if (address == 0x2018u) { ++reads.data; return false; } - if (words.size() != 1 || (address != 0x1088u && address != 0x108cu)) return false; - ++reads.descriptors; - words[0] = address == 0x1088u ? 0x2000u : 0u; - return true; - }; - const std::array user_data{0x1000u, 0u}; - const SrtRuntime runtime{.user_data = user_data, .read_memory = Read, .userdata = &reads, - .read_specialization_memory = Read}; - ResourceSnapshot snapshot; - ResourceSpecialization specialization; - Check(MaterializeResources(plan, runtime, snapshot, specialization) && - reads.descriptors == 2 && reads.data == 0 && snapshot.specialization_reads.empty() && - snapshot.buffers[0].dwords[0] == 0x2000u, - "resource materialization read mutable scalar data or lost its descriptor pointer"); -} - void TestConditionalBufferMaterialization() { auto plan = ConditionalBufferPlan(ConditionalBufferUse::Optional); // GTA III leaves packet words in s[12:15] when its scalar control word is zero. @@ -3728,7 +3655,6 @@ int main() { Run("DMA address materialization", TestDmaAddressMaterialization); Run("dynamic FLAT address", TestDynamicFlatAddressesUseDma); Run("buffer swizzle specialization", TestBufferSwizzleSpecialization); - Run("native scalar atomic data", TestNativeScalarAtomicDataDoesNotEnterResourcePlan); Run("conditional buffer materialization", TestConditionalBufferMaterialization); Run("guarded scalar descriptor reads", TestGuardedScalarDescriptorReads); Run("conservative buffer reachability", TestConservativeBufferReachability); diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index b3eceda53..89817d65b 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -27021,18 +27021,15 @@ TestCase ScalarMemoryLoadVariants() { std::vector expected = initial; expected.insert(expected.end(), initial.begin(), initial.end()); - TestCase test{"ScalarMemoryLoadVariants", - code, - initial, - expected, - {O::S_MOV_B32, O::S_LOAD_DWORD, O::S_LOAD_DWORDX2, O::S_LOAD_DWORDX4, - O::S_LOAD_DWORDX8, O::S_LOAD_DWORDX16, O::S_BUFFER_LOAD_DWORD, - O::S_BUFFER_LOAD_DWORDX2, O::S_BUFFER_LOAD_DWORDX4, - O::S_BUFFER_LOAD_DWORDX8, O::S_BUFFER_LOAD_DWORDX16, O::V_MOV_B32, - O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; - test.bda_mappings = {{0, 0}}; - test.required_spirv = {"get_bda_pointer"}; - return test; + return {"ScalarMemoryLoadVariants", + code, + initial, + expected, + {O::S_MOV_B32, O::S_LOAD_DWORD, O::S_LOAD_DWORDX2, O::S_LOAD_DWORDX4, + O::S_LOAD_DWORDX8, O::S_LOAD_DWORDX16, O::S_BUFFER_LOAD_DWORD, + O::S_BUFFER_LOAD_DWORDX2, O::S_BUFFER_LOAD_DWORDX4, + O::S_BUFFER_LOAD_DWORDX8, O::S_BUFFER_LOAD_DWORDX16, O::V_MOV_B32, + O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } TestCase ScalarBufferOffsetAlignmentAndCarry() { @@ -27076,15 +27073,11 @@ TestCase ScalarLoadSignedImmediateOffsetAddsSoffset() { AppendStoreSgpr(&code, 1, 0); AppendEnd(&code); - TestCase test{"ScalarLoadSignedImmediateOffsetAddsSoffset", - code, - {0x11111111u, 0x22222222u}, - {0x22222222u, 0x22222222u}, - {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; - test.bda_mappings = {{0, 0}}; - test.required_spirv = {"get_bda_pointer"}; - test.forbidden_spirv = {"flattened_srt"}; - return test; + return {"ScalarLoadSignedImmediateOffsetAddsSoffset", + code, + {0x11111111u, 0x22222222u}, + {0x22222222u, 0x22222222u}, + {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } TestCase BufferLoadStore() { @@ -29458,27 +29451,23 @@ TestCase BranchVccnzUsesCarryProducedWaveMask() { return test; } -TestCase ScalarLoadAlignsComponents() { +TestCase ScalarLoadAlignsComponentsAndMasksAddress() { using O = ShaderOpcode; std::vector code; AppendSMovLiteral(&code, 0, 1u); AppendSMovLiteral(&code, 2, 3u); - AppendSMovLiteral(&code, 3, 0u); + AppendSMovLiteral(&code, 3, 0xffff0000u); code.push_back(EncodeSmem0(0x00, 1, 1)); code.push_back(EncodeSmem1(3, 0)); AppendStoreSgpr(&code, 1, 0); AppendEnd(&code); - TestCase test{"ScalarLoadAlignsComponents", - code, - {0x11111111u, 0x22222222u}, - {0x11111111u, 0x22222222u}, - {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; - test.bda_mappings = {{0, 0}}; - test.required_spirv = {"get_bda_pointer"}; - test.forbidden_spirv = {"flattened_srt"}; - return test; + return {"ScalarLoadAlignsComponentsAndMasksAddress", + code, + {0x11111111u, 0x22222222u}, + {0x11111111u, 0x22222222u}, + {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } TestCase ScalarLoadAlignsDynamicBase() { @@ -34881,7 +34870,7 @@ std::vector MakeCases() { AddCase(ScalarMemoryLoadVariants); AddCase(ScalarBufferOffsetAlignmentAndCarry); AddCase(ScalarLoadSignedImmediateOffsetAddsSoffset); - AddCase(ScalarLoadAlignsComponents); + AddCase(ScalarLoadAlignsComponentsAndMasksAddress); AddCase(ScalarLoadAlignsDynamicBase); cases.push_back(ScalarBufferFromLoopReadlane(32)); cases.push_back(ScalarBufferFromLoopReadlane(64)); @@ -37792,19 +37781,17 @@ void CheckResourcePlanHandoff() { ValidateSpirv(name, compiled.spirv); const auto &bindings = compiled.program.bindings; Require(name, "GPU data requirements", - bindings.user_data_registers == (numeric_read ? std::vector{0, 1} - : std::vector{}) && - bindings.memory_offset_count == 1 && - FindBinding(bindings, DescriptorBindingKind::FlattenedSrt) == nullptr && - (FindBinding(bindings, DescriptorBindingKind::BdaPagetable) != nullptr) == numeric_read, - "descriptor-only shader retained SRT uploads or native scalar data lost its address"); + bindings.user_data_registers.empty() && bindings.memory_offset_count == 1 && + (FindBinding(bindings, DescriptorBindingKind::FlattenedSrt) != nullptr) == + numeric_read, + "descriptor-only shader retained SRT uploads or live scalar data was removed"); memory[0] = 0x2000u; memory[4] = 13u; Require(name, "independent host plan", MaterializeResources(plan, runtime, snapshot, specialization) && snapshot.buffers[0].dwords[0] == 0x2000u && - snapshot.flattened_srt.size() == 4, + (!numeric_read || snapshot.flattened_srt.back() == 13u), "compiled shader cleanup invalidated or froze the host resource plan"); } std::printf("[host] %-32s ok\n", name);