From ec3e48cdde8059ec8ed22ad13ba4eba8518dc44a Mon Sep 17 00:00:00 2001 From: nmzik Date: Sun, 4 Oct 2026 01:49:30 +0400 Subject: [PATCH] shader: synchronize GPU-produced direct writable descriptors Co-authored-by: TheCruZ --- .../ir/passes/ResourceMaterialization.cpp | 19 ++--- tests/ResourceMaterializationTests.cpp | 32 ++++++++- tests/shaderCfgTests.cpp | 70 +++++++++++++++++++ 3 files changed, 111 insertions(+), 10 deletions(-) diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index 88e48ca9..5005c203 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -975,12 +975,14 @@ ResourcePlan ExtractResourcePlan(const Program& program) { } } plan.capture_specialization_reads = capture_indirect_reads || !plan.control_flow.empty(); - // Writable descriptor addresses must come from clean backing for the alias proof. - for (const auto& buffer: plan.info.buffers) { - if (buffer.written) MarkCleanFlatSlots(plan, Source(plan, buffer.source), plan.clean_flat_slots); - } - for (const auto& image: plan.info.images) { - if (image.written) MarkCleanFlatSlots(plan, Source(plan, image.source), plan.clean_flat_slots); + if (plan.capture_specialization_reads) { + // Clean writable addresses prove that shader writes cannot overlap captured reads. + for (const auto& buffer: plan.info.buffers) { + if (buffer.written) MarkCleanFlatSlots(plan, Source(plan, buffer.source), plan.clean_flat_slots); + } + for (const auto& image: plan.info.images) { + if (image.written) MarkCleanFlatSlots(plan, Source(plan, image.source), plan.clean_flat_slots); + } } if (capture_indirect_reads) plan.resource_tracking_complete &= !program.has_address_writes; return plan; @@ -1004,7 +1006,8 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime observed.read_memory = CaptureOrdinaryRead; } SrtWalker clean(program, CleanRuntime(observed)); - SrtWalker walker(program, observed, program.clean_flat_slots, &clean); + SrtWalker walker(program, observed, program.clean_flat_slots, + capture_reads || program.requires_specialization_memory ? &clean : nullptr); if (!walker.RefreshFlatBuffer(snapshot.flattened_srt)) { return false; } @@ -1026,7 +1029,7 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime return false; } if (active.empty() || active[source]) { - return (written ? clean : walker).EvaluateDescriptor(source, value); + return (capture_reads && written ? clean : walker).EvaluateDescriptor(source, value); } value = {}; value.dword_count = program.descriptor_sources[source].dword_count; diff --git a/tests/ResourceMaterializationTests.cpp b/tests/ResourceMaterializationTests.cpp index 5e0caa1c..d4192c86 100644 --- a/tests/ResourceMaterializationTests.cpp +++ b/tests/ResourceMaterializationTests.cpp @@ -227,6 +227,7 @@ void TestWrittenDescriptorUsesStrictReaderOnce() { using namespace Libs::Graphics::ShaderRecompiler::IR; Program program; program.stage = Libs::Graphics::ShaderType::Compute; + program.user_data_count = 1; program.srt_plan_complete = true; program.resource_tracking_complete = true; auto &block = AddValueBlock(program); @@ -243,7 +244,32 @@ void TestWrittenDescriptorUsesStrictReaderOnce() { source.dword_count = 4; program.descriptor_sources.push_back(source); program.info.buffers.push_back({.source = 0, .written = true}); + // A host-evaluable branch captures resource reads and needs the writable + // descriptor's clean provenance for the renderer's disjointness proof. + auto &condition = block.AppendNewInst(ValueOpcode::IEqual32, + {Value(&offset), Value(4u)}); + auto &store_block = AddValueBlock(program); + AddValueBlock(program); + program.block_info[0].condition = Value(&condition); + program.block_info[0].terminator.kind = + Libs::Graphics::ShaderRecompiler::CFG::TerminatorKind::ConditionalBranch; + program.block_info[0].terminator.true_block = 1; + program.block_info[0].terminator.false_block = 2; + program.block_info[1].id = 1; + program.block_info[1].terminator.kind = + Libs::Graphics::ShaderRecompiler::CFG::TerminatorKind::Return; + program.block_info[2].id = 2; + program.block_info[2].terminator.kind = + Libs::Graphics::ShaderRecompiler::CFG::TerminatorKind::Return; + program.memory_info.push_back({.kind = ResourceKind::Buffer, .resource = 0}); + auto &output = store_block.AppendNewInst(ValueOpcode::GetBufferResource, + {source.dwords[0], source.dwords[1], source.dwords[2], source.dwords[3]}); + store_block.AppendNewInst(ValueOpcode::StoreBufferU32, + {Value(&output), Value(0u), Value(0u), Value(0u), Value(1u), Value(true)}) + .SetFlags(MemoryFlags{.index = 1}); auto plan = ExtractResourcePlan(program); + Check(plan.capture_specialization_reads, + "conditional writable descriptor lost its alias proof"); struct Reads { uint32_t ordinary = 0; uint32_t strict = 0; bool clean = false; } reads; const std::array user_data{4u}; const SrtRuntime runtime{ @@ -269,7 +295,9 @@ void TestWrittenDescriptorUsesStrictReaderOnce() { reads.clean = true; reads.strict = 0; Check(MaterializeResources(plan, runtime, snapshot, specialization) && - reads.ordinary == 0 && reads.strict == 1 && snapshot.buffers[0].dwords[0] == 0x8000u, + reads.ordinary == 0 && reads.strict == 1 && snapshot.buffers[0].dwords[0] == 0x8000u && + snapshot.specialization_reads == + std::vector>{{0x1004u, 4u}}, "writable descriptor was evaluated twice or scalar EXEC suppressed its read"); } @@ -313,7 +341,7 @@ void TestFiniteImageRefreshReusesScalarReads() { DescriptorSource root; root.dword_count = 8; root.dwords.fill(Value(0u)); - root.indirect_image.emplace(DescriptorSource::IndirectImage{}).sources = {0, 1, 2, 1}; + root.indirect_descriptor.emplace(DescriptorSource::IndirectDescriptor{}).sources = {0, 1, 2, 1}; program.descriptor_sources.push_back(root); program.info.images.push_back({ .source = 3, diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 581ca6da..023cfa64 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -13174,6 +13174,75 @@ void TestComputeImageFill() { Run(mutation); } +void TestGpuProducedWritableDescriptor() { + using namespace ShaderRecompiler::IR; + // Reduced from 994af59d50bedca8: the GPU-built BVH header supplies a relative + // output address and record count. CMPX changes EXEC without clobbering VCC. + const uint32_t shader[] = { + 0xf4041a81u, 0xfa000028u, // s_load_dwordx2 vcc, s[2:3], 0x28 + 0xf4000281u, 0xfa000074u, // s_load_dword s10, s[2:3], 0x74 + 0xbe8b03ffu, 0x00016204u, // s_mov_b32 s11, 0x16204 + 0xbf8cc07fu, // s_waitcnt lgkmcnt(0) + 0x8008026au, // s_add_u32 s8, vcc_lo, s2 + EncodeVopc(0xd5, 129, 0), // v_cmpx_ne_u32 1, v0 + 0x8209036bu, // s_addc_u32 s9, vcc_hi, s3 + 0xbe891d97u, // s_bitset1_b32 s9, 23 (stride = 128) + EncodeMubuf0(0x1c), EncodeMubuf1(0, 2, 0), + EncodeSopp(0x01), + }; + const std::array user_data{0u, 0u, 0x00315000u, 3u}; + 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 && + !plan.capture_specialization_reads && plan.srt_reads.size() == 3, + "native GPU-built writable descriptor acquired an unnecessary alias proof"); + struct Reads { + std::array header{0x26c0u, 0u, 151u}; + uint32_t ordinary = 0; + uint32_t strict = 0; + } reads; + const SrtRuntime runtime{ + .user_data = user_data, + .read_memory = +[](void *data, uint64_t address, std::span words) { + if (words.size() != 1) return false; + auto &reads = *static_cast(data); + const uint32_t index = address == 0x300315028ull ? 0u + : address == 0x30031502cull ? 1u + : address == 0x300315074ull ? 2u : 3u; + if (index == 3u) return false; + ++reads.ordinary; + words[0] = reads.header[index]; + return true; + }, + .userdata = &reads, + .read_specialization_memory = +[](void *data, uint64_t, std::span) { + ++static_cast(data)->strict; + return false; // Current GPU bytes are available only through the ordinary reader. + }}; + ResourceSnapshot snapshot; + ResourceSpecialization specialization; + const auto check = [&](uint32_t low, uint32_t high, uint32_t count, uint32_t reads_expected) { + Check(MaterializeResources(plan, runtime, snapshot, specialization), + "GPU-produced direct writable descriptor was rejected"); + Check(snapshot.buffers.size() == 1 && + snapshot.buffers[0].dwords[0] == low && + snapshot.buffers[0].dwords[1] == high && + snapshot.buffers[0].dwords[2] == count && + snapshot.buffers[0].dwords[3] == 0x16204u && + snapshot.specialization_reads.empty() && reads.strict == 0 && + reads.ordinary == reads_expected, + "GPU-produced descriptor lost current scalar data or repeated its reads"); + }; + check(0x003176c0u, 0x00800003u, 151u, 3u); + const auto previous_specialization = specialization; + reads.header = {0xffd00000u, 0u, 302u}; // Exercise the low-address carry on refresh. + check(0x00015000u, 0x00800004u, 302u, 6u); + Check(specialization == previous_specialization, + "GPU-updated address or record count created a shader permutation"); +} + void TestTypedDescriptorRealCarryAndScalarLoads() { const uint32_t carry_shader[] = { EncodeSop1(0x1f, 0, 0), // s_getpc_b64 s[0:1] @@ -14590,6 +14659,7 @@ int main() { TestNewShaderRecompilerEarlyZDisabledWhenPixelKillEnabled(); TestTypedDescriptorRealWideMoveTranslation(); TestComputeImageFill(); + TestGpuProducedWritableDescriptor(); TestTypedDescriptorRealCarryAndScalarLoads(); TestSrtWalkerRealSmemTranslation(); TestSrtWalkerVccBaseTranslation();