diff --git a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp index 241b270ab..eaadfd952 100644 --- a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp +++ b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp @@ -974,6 +974,41 @@ void RenderExecutor::CommitBindings(CommandBuffer& buffer, (pipeline_bind_point == vk::PipelineBindPoint::eCompute && shader_stage != vk::ShaderStageFlagBits::eCompute)); } + for (const auto* reader: prepared_bindings) { + const auto& reads = reader->runtime->resources->specialization_reads; + if (reads.empty()) continue; + for (const auto* writer: prepared_bindings) { + if (writer->runtime->program->has_address_writes) { + EXIT("scalar resource reads cannot be proven disjoint from shader address writes\n"); + } + } + for (const auto [address, size]: reads) { + for (const auto id: m_bound_images) { + const auto* image = m_context.GetTextureCache().m_slot_images.try_get(id); + if (image == nullptr || + (!image->binding.shader_write && !image->binding.is_target)) continue; + for (const auto written: {image->info.data, image->info.stencil, + image->info.metadata.range}) { + if (written.size != 0 && ImageRangeOverlaps(address, size, + written.address, written.size)) { + EXIT("scalar resource reads overlap an image or attachment write\n"); + } + } + } + for (const auto* writer: prepared_bindings) { + const auto& program = *writer->runtime->program; + for (uint32_t i = 0; i < writer->buffer_sources.size(); ++i) { + const auto resource = program.bindings.descriptors.front().resources[i]; + if (!program.info.buffers[resource].written) continue; + const auto& written = writer->buffer_sources[i]; + if (written.size != 0 && ImageRangeOverlaps(address, size, + written.address, written.size)) { + EXIT("scalar resource reads overlap a shader buffer write\n"); + } + } + } + } + } m_descriptor_buffers.clear(); m_descriptor_images.clear(); m_descriptor_writes.clear(); diff --git a/src/graphics/host_gpu/renderer/renderCompute.cpp b/src/graphics/host_gpu/renderer/renderCompute.cpp index 715ef9af2..b56fdf2d6 100644 --- a/src/graphics/host_gpu/renderer/renderCompute.cpp +++ b/src/graphics/host_gpu/renderer/renderCompute.cpp @@ -264,15 +264,14 @@ void RenderExecutor::DispatchDirect(uint64_t submit_id, CommandBuffer& buffer, const auto& program = *input_info.stage.program; const auto& resources = *input_info.stage.resources; - if (TryConsumeComputeMetaClear(input_info, buffer)) { - ResetBindings(); - return; - } - if (TryConsumeComputeImageClear(input_info, buffer, thread_group_x, thread_group_y, - thread_group_z, mode)) { + if (resources.specialization_reads.empty() && + (TryConsumeComputeMetaClear(input_info, buffer) || + TryConsumeComputeImageClear(input_info, buffer, thread_group_x, thread_group_y, + thread_group_z, mode))) { ResetBindings(); return; } + const bool large_workgroup = (input_info.threads_num[0] * input_info.threads_num[1] * input_info.threads_num[2] >= 512); const bool has_sampler = !program.info.samplers.empty(); diff --git a/src/graphics/shader/recompiler/ir/Program.cpp b/src/graphics/shader/recompiler/ir/Program.cpp index 51ce341f0..7c9f16805 100644 --- a/src/graphics/shader/recompiler/ir/Program.cpp +++ b/src/graphics/shader/recompiler/ir/Program.cpp @@ -142,6 +142,7 @@ CompiledShaderInfo Program::TakeCompiledInfo() && { .user_data_base = user_data_base, .user_data_count = user_data_count, .scratch_dwords = scratch_dwords, + .has_address_writes = has_address_writes, .info = std::move(info), .bindings = std::move(bindings), }; diff --git a/src/graphics/shader/recompiler/ir/ResourceSnapshot.h b/src/graphics/shader/recompiler/ir/ResourceSnapshot.h index 4734fa07b..3e7586a7f 100644 --- a/src/graphics/shader/recompiler/ir/ResourceSnapshot.h +++ b/src/graphics/shader/recompiler/ir/ResourceSnapshot.h @@ -3,6 +3,7 @@ #include #include +#include #include namespace Libs::Graphics::ShaderRecompiler::IR { @@ -34,6 +35,7 @@ struct ResourceSnapshot { std::vector samplers; std::vector flattened_srt; std::vector user_data; + std::vector> specialization_reads; UniformFill uniform_fill; }; diff --git a/src/graphics/shader/recompiler/ir/ShaderIR.h b/src/graphics/shader/recompiler/ir/ShaderIR.h index e95830b51..2fa4081d1 100644 --- a/src/graphics/shader/recompiler/ir/ShaderIR.h +++ b/src/graphics/shader/recompiler/ir/ShaderIR.h @@ -499,6 +499,7 @@ struct ResourceBlock { Value condition; std::vector successors; std::vector sources; + std::vector srt_reads; }; // Stable shader metadata consumed by the renderer after native IR has been discarded. @@ -510,6 +511,7 @@ struct CompiledShaderInfo { uint32_t user_data_count = 64; uint32_t scratch_dwords = 0; uint32_t param_export_mask = 0; + bool has_address_writes = false; ShaderInfo info; BindingLayout bindings; }; @@ -564,7 +566,6 @@ struct ResourcePlan { mutable std::vector visited_blocks; mutable std::vector pending_blocks; mutable std::vector material_keys; - mutable std::vector> specialization_reads; }; struct Program: ResourcePlan { diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index 418c4be3a..29d786e1e 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -157,34 +157,15 @@ bool CaptureStrictRead(void* userdata, uint64_t address, std::span val bool CaptureOrdinaryRead(void* userdata, uint64_t address, std::span values) { auto& capture = *static_cast(userdata); - if (!capture.source.read_memory(capture.source.userdata, address, values)) { - return false; + if (capture.source.read_memory != nullptr) { + if (!capture.source.read_memory(capture.source.userdata, address, values)) return false; + } else { + std::memcpy(values.data(), reinterpret_cast(address), values.size_bytes()); } capture.ranges.emplace_back(address, values.size_bytes()); return true; } -bool WrittenBuffersDisjoint(const ResourcePlan& program, const ResourceSnapshot& snapshot, - std::span> reads) { - for (uint32_t i = 0; i < program.info.buffers.size(); ++i) { - if (!program.info.buffers[i].written) continue; - ShaderBufferResource buffer; - if (!DecodeBufferDescriptor(snapshot.buffers[i], buffer)) return false; - const auto base = buffer.Base48(); - const auto size = buffer.GetSize(); - if (size == 0u) continue; - if (size - 1u > AddressMask - base) return false; - for (const auto [address, bytes]: reads) { - if (bytes == 0u) continue; - if (address > AddressMask || bytes - 1u > AddressMask - address || - (address <= base ? base - address < bytes : address - base < size)) { - return false; - } - } - } - return true; -} - const DescriptorSource* Source(const ResourcePlan& program, uint32_t source) { if (source >= program.descriptor_sources.size()) { return nullptr; @@ -638,9 +619,10 @@ bool BuildSamplerPlan(const ShaderInfo& base, const Images& images, SamplerPlan& return true; } -static std::vector ResourceControlFlow(const Program& program) { - // Any shader write may alias a scalar predicate read, including on a later loop visit. - if (program.blocks.size() != program.block_info.size() || HasShaderMemoryWrites(program)) { +template +static std::vector ResourceControlFlow(const Program& program, const Predicate& predicate) { + // The renderer checks captured scalar reads against final buffer and image write ranges. + if (program.blocks.size() != program.block_info.size() || program.has_address_writes) { return {}; } std::unordered_map indices; @@ -659,9 +641,7 @@ static std::vector ResourceControlFlow(const Program& program) { case CFG::TerminatorKind::Branch: successors.push_back(terminator.true_block); break; case CFG::TerminatorKind::ConditionalBranch: successors = {terminator.true_block, terminator.false_block}; - if (ValidateRuntimeValue(program, info.condition, RuntimeValueType::Integer)) { - block.condition = info.condition; - } + block.condition = info.condition; break; case CFG::TerminatorKind::IndirectBranch: successors = terminator.indirect_targets; @@ -678,13 +658,17 @@ static std::vector ResourceControlFlow(const Program& program) { } for (const auto& inst: *program.blocks[i]) { const auto op = inst.GetOpcode(); + if (op == ValueOpcode::ReadConst) { + block.srt_reads.push_back(inst.Arg(1).Resolve().U32()); + continue; + } const auto buffer = BufferAccessOf(op); const auto image = ImageOpcodeInfoOf(op); if (buffer == BufferAccess::None && image.access == ImageAccess::None) { continue; } const auto& memory = program.memory_info.at(inst.Flags().index); - if (memory.planning_only) { + if (memory.planning_only || memory.kind == ResourceKind::IndirectBuffer) { continue; } if (buffer != BufferAccess::None) { @@ -700,6 +684,7 @@ static std::vector ResourceControlFlow(const Program& program) { block.sources.erase(std::unique(block.sources.begin(), block.sources.end()), block.sources.end()); } + for (auto& block: blocks) block.condition = predicate(block.condition); if (std::ranges::none_of( blocks, [](const ResourceBlock& block) { return !block.condition.IsEmpty(); })) { return {}; @@ -914,10 +899,38 @@ ResourcePlan ExtractResourcePlan(const Program& program) { for (const auto& read: program.srt_reads) { plan.srt_reads.push_back({Clone(read.value), read.flat_offset}); } - plan.control_flow = ResourceControlFlow(program); - for (auto& block: plan.control_flow) { - block.condition = Clone(block.condition); - } + // A proven uniform factor can decide a branch even when its other lanes are unknown. + // Keep only that Boolean structure, never the varying shader dependency graph. + Value unknown; + std::function ClonePredicate = [&](Value value) -> Value { + value = value.Resolve(); + if (value.IsEmpty()) return {}; + if (ValidateRuntimeValue(program, value, RuntimeValueType::Integer)) return Clone(value); + const auto* inst = value.TryInstruction(); + if (inst == nullptr) return {}; + const auto op = inst->GetOpcode(); + if (op == ValueOpcode::ConditionRef || op == ValueOpcode::LogicalNot) { + const auto operand = ClonePredicate(inst->Arg(0)); + if (operand.IsEmpty() || op == ValueOpcode::ConditionRef) return operand; + auto& node = plan.value_storage.emplace_back(op); + node.SetArg(0, operand); + return Value(&node); + } + if (op != ValueOpcode::LogicalAnd && op != ValueOpcode::LogicalOr) return {}; + auto left = ClonePredicate(inst->Arg(0)); + auto right = ClonePredicate(inst->Arg(1)); + if (left.IsEmpty() && right.IsEmpty()) return {}; + if (left.IsEmpty() || right.IsEmpty()) { + if (unknown.IsEmpty()) unknown = Value(&plan.value_storage.emplace_back(ValueOpcode::UndefU1)); + if (left.IsEmpty()) left = unknown; + if (right.IsEmpty()) right = unknown; + } + auto& node = plan.value_storage.emplace_back(op); + node.SetArg(0, left); + node.SetArg(1, right); + return Value(&node); + }; + plan.control_flow = ResourceControlFlow(program, ClonePredicate); plan.uniform_fill = AnalyzeUniformFill(program); for (uint32_t i = 0; i < plan.uniform_fill.fill.words; ++i) { plan.uniform_fill.values[i] = Clone(plan.uniform_fill.values[i]); @@ -936,12 +949,15 @@ ResourcePlan ExtractResourcePlan(const Program& program) { MarkCleanFlatSlots(plan, Source(plan, source->indirect_image->table_source), plan.clean_flat_slots); } - if (masked_image) { - plan.resource_tracking_complete &= !program.has_address_writes && - !std::ranges::any_of(plan.info.images, &ImageResource::written); - plan.capture_specialization_reads = - std::ranges::any_of(plan.info.buffers, &BufferResource::written); + plan.capture_specialization_reads = masked_image || !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 (masked_image) plan.resource_tracking_complete &= !program.has_address_writes; return plan; } @@ -952,21 +968,22 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime return false; } const bool capture_reads = program.capture_specialization_reads; - auto& reads = program.specialization_reads; + auto& reads = snapshot.specialization_reads; + reads.clear(); ReadCapture capture {runtime, reads}; SrtRuntime observed = runtime; if (capture_reads) { - reads.clear(); observed.userdata = &capture; - observed.read_specialization_memory = CaptureStrictRead; - if (observed.read_memory != nullptr) observed.read_memory = CaptureOrdinaryRead; + observed.read_specialization_memory = runtime.read_specialization_memory != nullptr + ? CaptureStrictRead : nullptr; + observed.read_memory = CaptureOrdinaryRead; } SrtWalker clean(program, CleanRuntime(observed)); SrtWalker walker(program, observed, program.clean_flat_slots, &clean); - const auto active = clean.FindActiveSources(); if (!walker.RefreshFlatBuffer(snapshot.flattened_srt)) { return false; } + const auto active = std::span(program.active_sources); snapshot.uniform_fill = {}; const auto& fill = program.uniform_fill; const auto words = fill.fill.words; @@ -979,12 +996,12 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime snapshot.uniform_fill = fill.fill; snapshot.uniform_fill.value = stored[0]; } - const auto evaluate = [&](uint32_t source, DescriptorValue& value) { + const auto evaluate = [&](uint32_t source, DescriptorValue& value, bool written = false) { if (source >= program.descriptor_sources.size()) { return false; } if (active.empty() || active[source]) { - return walker.EvaluateDescriptor(source, value); + return (written ? clean : walker).EvaluateDescriptor(source, value); } value = {}; value.dword_count = program.descriptor_sources[source].dword_count; @@ -992,19 +1009,11 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime }; snapshot.buffers.resize(program.info.buffers.size()); for (uint32_t i = 0; i < program.info.buffers.size(); ++i) { - if (!evaluate(program.info.buffers[i].source, snapshot.buffers[i])) { + if (!evaluate(program.info.buffers[i].source, snapshot.buffers[i], + program.info.buffers[i].written)) { return false; } } - if (capture_reads) { - for (uint32_t i = 0; i < program.info.buffers.size(); ++i) { - const auto& buffer = program.info.buffers[i]; - if (!buffer.written || (!active.empty() && !active[buffer.source])) continue; - DescriptorValue strict; - if (!clean.EvaluateDescriptor(buffer.source, strict) || - strict != snapshot.buffers[i]) return false; - } - } snapshot.images.resize(program.info.images.size()); specialization.images.resize(program.info.images.size()); for (uint32_t i = 0; i < program.info.images.size(); ++i) { @@ -1040,7 +1049,7 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime return false; } } else { - if (!evaluate(image.source, snapshot.images[i])) { + if (!evaluate(image.source, snapshot.images[i], image.written)) { return false; } if (!ValidImageDescriptor(snapshot.images[i], image.r128)) { @@ -1054,7 +1063,6 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime return false; } } - if (capture_reads && !WrittenBuffersDisjoint(program, snapshot, reads)) return false; snapshot.user_data.assign(runtime.user_data.begin(), runtime.user_data.end()); return BuildResourceSpecialization(program, snapshot, specialization); } diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp index 34fdc08b0..2d684a97a 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp @@ -795,18 +795,26 @@ bool SrtWalker::EvaluateInst(const Inst& inst, uint64_t& result) { return true; } return false; - case ValueOpcode::LogicalAnd: - if (binary()) { - result = (a != 0u) && (b != 0u); + case ValueOpcode::LogicalAnd: { + const bool left = Arg(inst, 0, a); + if (left && a == 0u) { + result = 0u; return true; } - return false; - case ValueOpcode::LogicalOr: - if (binary()) { - result = (a != 0u) || (b != 0u); + if (!Arg(inst, 1, b) || (b != 0u && !left)) return false; + result = b != 0u; + return true; + } + case ValueOpcode::LogicalOr: { + const bool left = Arg(inst, 0, a); + if (left && a != 0u) { + result = 1u; return true; } - return false; + if (!Arg(inst, 1, b) || (b == 0u && !left)) return false; + result = b != 0u; + return true; + } case ValueOpcode::LogicalXor: if (binary()) { result = (a != 0u) != (b != 0u); @@ -844,16 +852,31 @@ bool SrtWalker::EvaluateDescriptor(uint32_t source, DescriptorValue& result) { return true; } -std::span SrtWalker::FindActiveSources() { - if (m_program.control_flow.empty()) { - return {}; - } +bool SrtWalker::RefreshFlatBuffer(std::vector& flat) { + if (!m_program.srt_plan_complete) return false; + const auto refresh = [&](uint32_t slot) { + if (slot >= m_program.srt_reads.size()) return false; + const auto& read = m_program.srt_reads[slot]; + const bool clean = read.flat_offset < m_clean_flat_slots.size() && + m_clean_flat_slots[read.flat_offset] != 0u; + if (clean && (m_clean_evaluator == nullptr || m_runtime.read_specialization_memory == nullptr)) + return false; + auto& evaluator = clean ? *m_clean_evaluator : *this; + return read.flat_offset < flat.size() && evaluator.Evaluate(read.value, flat[read.flat_offset]); + }; auto& active = m_program.active_sources; + if (m_program.control_flow.empty()) { + active.clear(); + flat.resize(m_program.srt_reads.size()); + for (uint32_t slot = 0; slot < m_program.srt_reads.size(); ++slot) { + if (!refresh(slot)) return false; + } + return true; + } + flat.assign(m_program.srt_reads.size(), 0u); active.assign(m_program.descriptor_sources.size(), 1u); for (const auto& block: m_program.control_flow) { - for (const auto source: block.sources) { - active.at(source) = 0u; - } + for (const auto source: block.sources) active.at(source) = 0u; } auto& visited = m_program.visited_blocks; auto& pending = m_program.pending_blocks; @@ -863,41 +886,22 @@ std::span SrtWalker::FindActiveSources() { while (!pending.empty()) { const auto index = pending.back(); pending.pop_back(); - if (visited.at(index)) { - continue; - } + if (visited.at(index)) continue; visited[index] = 1u; const auto& block = m_program.control_flow[index]; - for (const auto source: block.sources) { - active[source] = 1u; + for (const auto source: block.sources) active[source] = 1u; + for (const auto slot: block.srt_reads) { + if (!refresh(slot)) return false; } uint32_t condition = 0; + auto& predicate = m_clean_evaluator != nullptr ? *m_clean_evaluator : *this; if (!block.condition.IsEmpty() && m_runtime.read_specialization_memory != nullptr && - Evaluate(block.condition, condition)) { + predicate.Evaluate(block.condition, condition)) { pending.push_back(block.successors[condition != 0u ? 0u : 1u]); } else { pending.insert(pending.end(), block.successors.begin(), block.successors.end()); } } - return active; -} - -bool SrtWalker::RefreshFlatBuffer(std::vector& flat) { - if (!m_program.srt_plan_complete) { - return false; - } - flat.resize(m_program.srt_reads.size()); - for (const auto& read: m_program.srt_reads) { - const bool clean = read.flat_offset < m_clean_flat_slots.size() && - m_clean_flat_slots[read.flat_offset] != 0u; - if (clean && (m_clean_evaluator == nullptr || m_runtime.read_specialization_memory == nullptr)) { - return false; - } - auto& evaluator = clean ? *m_clean_evaluator : *this; - if (read.flat_offset >= flat.size() || !evaluator.Evaluate(read.value, flat[read.flat_offset])) { - return false; - } - } return true; } diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.h b/src/graphics/shader/recompiler/ir/passes/SrtWalker.h index 9e160f41b..4903d11bb 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.h +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.h @@ -38,8 +38,7 @@ public: bool Evaluate(Value value, uint32_t& result); bool EvaluateDescriptor(uint32_t source, DescriptorValue& result); - // An empty span means that all sources are active. - std::span FindActiveSources(); + // Refreshes reachable scalar reads and active descriptor sources in one walk. bool RefreshFlatBuffer(std::vector& flat); private: diff --git a/tests/ResourceMaterializationTests.cpp b/tests/ResourceMaterializationTests.cpp index 2d27302c3..8380178e9 100644 --- a/tests/ResourceMaterializationTests.cpp +++ b/tests/ResourceMaterializationTests.cpp @@ -219,6 +219,56 @@ void TestUnbasedFlatCacheHitMaterializes() { "unbased FLAT plan produced unexpected descriptors"); } +void TestWrittenDescriptorUsesStrictReaderOnce() { + using namespace Libs::Graphics::ShaderRecompiler::IR; + Program program; + program.stage = Libs::Graphics::ShaderType::Compute; + program.srt_plan_complete = true; + program.resource_tracking_complete = true; + auto &block = AddValueBlock(program); + program.memory_info.push_back({.kind = ResourceKind::ScalarAddress}); + auto &handle = block.AppendNewInst(ValueOpcode::GetAddressResource, + {Value(0x1000u), Value(0u)}); + auto &offset = block.AppendNewInst(ValueOpcode::GetUserData, + {Value(static_cast(0))}); + auto &read = block.AppendNewInst(ValueOpcode::LoadAddressU32, + {Value(&handle), Value(&offset), Value(0u), Value(false)}); + read.SetFlags(MemoryFlags{.index = 0}); + DescriptorSource source; + source.dwords = {Value(&read), Value(0u), Value(4u), Value(0u)}; + source.dword_count = 4; + program.descriptor_sources.push_back(source); + program.info.buffers.push_back({.source = 0, .written = true}); + auto plan = ExtractResourcePlan(program); + struct Reads { uint32_t ordinary = 0; uint32_t strict = 0; bool clean = false; } reads; + const std::array user_data{4u}; + const SrtRuntime runtime{ + .user_data = user_data, + .read_memory = +[](void *data, uint64_t, std::span words) { + ++static_cast(data)->ordinary; + words[0] = 0x8000u; + return true; + }, + .userdata = &reads, + .read_specialization_memory = +[](void *data, uint64_t address, std::span words) { + auto &reads = *static_cast(data); + ++reads.strict; + if (!reads.clean || address != 0x1004u) return false; + words[0] = 0x8000u; + return true; + }}; + ResourceSnapshot snapshot; + ResourceSpecialization specialization; + Check(!MaterializeResources(plan, runtime, snapshot, specialization) && + reads.ordinary == 0 && reads.strict == 1, + "GPU-dirty dynamic writable descriptor bypassed strict provenance"); + reads.clean = true; + reads.strict = 0; + Check(MaterializeResources(plan, runtime, snapshot, specialization) && + reads.ordinary == 0 && reads.strict == 1 && snapshot.buffers[0].dwords[0] == 0x8000u, + "writable descriptor was evaluated twice or scalar EXEC suppressed its read"); +} + void TestFailedMaterializationRejectsStage() { using namespace Libs::Graphics::ShaderRecompiler::IR; auto plan = UserDataBufferPlan(); @@ -262,6 +312,7 @@ int main() { TestMappedSrtUsesDirectReaderByDefault(); TestIntegerRuntimeValueFollowsSrtReads(); TestUnbasedFlatCacheHitMaterializes(); + TestWrittenDescriptorUsesStrictReaderOnce(); TestFailedMaterializationRejectsStage(); TestMixedSamplerDuplicatesTheCorrectSnapshot(); std::puts("ResourceMaterializationTests: all cases passed"); diff --git a/tests/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index 484b49fab..13dfe758c 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -1386,11 +1386,17 @@ void TestUniformizedMaterialImageKeys() { specialization.images[0].indirect_mapping_offset + 3u] == 4096u, "material mask did not limit sparse descriptor reads"); user_data[8] = first_material; - Check(!MaterializeResources(plan, runtime, snapshot, specialization), - "written buffer alias with a material key was accepted"); + Check(MaterializeResources(plan, runtime, snapshot, specialization) && + std::ranges::find(snapshot.specialization_reads, + std::pair{first_material, 4}) != + snapshot.specialization_reads.end(), + "material key read was omitted from the renderer write-overlap check"); user_data[8] = first_table; - Check(!MaterializeResources(plan, runtime, snapshot, specialization), - "written buffer alias with an image record was accepted"); + Check(MaterializeResources(plan, runtime, snapshot, specialization) && + std::ranges::find(snapshot.specialization_reads, + std::pair{first_table, 32}) != + snapshot.specialization_reads.end(), + "image descriptor read was omitted from the renderer write-overlap check"); CheckFatal([&] { make_plan(Variant::WrongUpdate); }, "not a valid runtime value", "non-clearing material mask was accepted"); CheckFatal([&] { make_plan(Variant::WrongEquality); }, "not a valid runtime value", @@ -2785,6 +2791,108 @@ void TestConditionalBufferMaterialization() { CheckActive(); } +void TestGuardedScalarDescriptorReads() { + namespace CFG = Libs::Graphics::ShaderRecompiler::CFG; + for (const bool shared : {false, true}) { + for (const bool exec : {false, true}) { + Fixture fixture; + auto *entry = fixture.block; + auto *optional = fixture.AddBlock(); + auto *done = fixture.AddBlock(); + entry->AddBranch(optional); + entry->AddBranch(done); + optional->AddBranch(done); + fixture.program.block_info[0].terminator = { + .kind = CFG::TerminatorKind::ConditionalBranch, .true_block = 1, .false_block = 2}; + fixture.program.block_info[1].terminator = { + .kind = CFG::TerminatorKind::Branch, .true_block = 2}; + fixture.program.block_info[2].terminator.kind = CFG::TerminatorKind::Return; + const auto control = fixture.Buffer({fixture.UserData(0), fixture.UserData(1), + fixture.UserData(2), fixture.UserData(3)}); + MemoryInfo scalar; + scalar.kind = ResourceKind::ScalarBuffer; + scalar.offset = 28; + const auto flag = fixture.Emit(ValueOpcode::ReadConstBuffer, {control, Value(0u)}, + fixture.AddMemory(scalar, 4)); + const auto lane = fixture.Emit(ValueOpcode::GetBuiltin, + {Value(static_cast(StageInputKind::LocalInvocationId)), Value(0u)}); + const auto varying = fixture.Emit(ValueOpcode::INotEqual32, {lane, Value(0u)}); + const auto enabled = fixture.Emit(ValueOpcode::INotEqual32, {flag, Value(0u)}); + fixture.program.block_info[0].condition = + fixture.Emit(ValueOpcode::LogicalAnd, {varying, enabled}); + const auto root = fixture.Address(fixture.UserData(4), Value(0u)); + const auto Load = [&](Block *block) { + fixture.block = block; + MemoryInfo address; + address.kind = ResourceKind::ScalarAddress; + const auto pointer = fixture.Emit(ValueOpcode::LoadAddressU32, + {root, Value(0u), Value(0u), Value(exec)}, fixture.AddMemory(address, 8)); + const auto table = fixture.Address(pointer, Value(0u)); + std::array words; + for (uint32_t word = 0; word < words.size(); ++word) { + address.offset = 40 + word * 4; + words[word] = fixture.Emit(ValueOpcode::LoadAddressU32, + {table, Value(0u), Value(0u), Value(exec)}, fixture.AddMemory(address, 12)); + } + scalar.offset = 0; + fixture.Emit(ValueOpcode::ReadConstBuffer, {fixture.Buffer(words), Value(0u)}, + fixture.AddMemory(scalar, 16)); + }; + Load(optional); + if (shared) Load(done); + fixture.block = done; + MemoryInfo store; + store.kind = ResourceKind::Buffer; + fixture.Emit(ValueOpcode::StoreBufferU32, + {fixture.Buffer({Value(0x9000u), Value(0u), Value(4u), Value(0u)}), + Value(0u), Value(0u), Value(0u), Value(1u), Value(true)}, fixture.AddMemory(store, 20)); + fixture.PlanAndTrack(); + auto plan = ExtractResourcePlan(fixture.program); + Check(!exec || (!plan.srt_reads.empty() && !plan.control_flow[1].srt_reads.empty() && + (!shared || !plan.control_flow[2].srt_reads.empty())), + "guarded shared-read fixture lost its flattened execution sites"); + struct Memory { LinearTestMemory data; uint32_t null_reads = 0; } memory; + const auto Read = +[](void *data, uint64_t address, std::span words) { + auto &memory = *static_cast(data); + if (address == 40) ++memory.null_reads; + return ReadLinearTestMemory(&memory.data, address, words); + }; + const std::array user_data{0x1000u, 0u, 64u, 0u, 0x1040u}; + const SrtRuntime runtime{.user_data = user_data, .read_memory = Read, + .userdata = &memory, .read_specialization_memory = Read}; + memory.data.words[0xa8 / 4] = 0x2000u; + memory.data.words[0xb0 / 4] = 4u; + ResourceSnapshot snapshot; + ResourceSpecialization specialization; + if (!shared) { + Check(MaterializeResources(plan, runtime, snapshot, specialization) && memory.null_reads == 0 && + std::ranges::all_of(snapshot.flattened_srt, [](auto word) { return word == 0; }), + "disabled feature speculatively dereferenced its null BVH table"); + memory.data.words[7] = 1; + Check(!MaterializeResources(plan, runtime, snapshot, specialization) && memory.null_reads == 1, + "active feature accepted an unreadable BVH table"); + } + memory.data.words[0x40 / 4] = 0x1080u; + memory.data.watched_address = 0x10a8u; + memory.data.watched_reads = 0; + Check(MaterializeResources(plan, runtime, snapshot, specialization) && + std::ranges::any_of(snapshot.buffers, [](const auto& descriptor) { + return descriptor.dwords[0] == 0x2000u; + }) && + (!exec || (memory.data.watched_reads == 1 && + std::ranges::find(snapshot.flattened_srt, 0x2000u) != snapshot.flattened_srt.end())), + "active or shared descriptor read was omitted after refreshing the cached plan"); + if (!shared) { + memory.data.words[7] = 0; + memory.data.words[0x40 / 4] = 0; + Check(MaterializeResources(plan, runtime, snapshot, specialization) && memory.null_reads == 1 && + std::ranges::all_of(snapshot.flattened_srt, [](auto word) { return word == 0; }), + "cached plan retained reads after the feature was disabled"); + } + } + } +} + void TestConservativeBufferReachability() { std::array user_data{ 0x1000, 16u << 16u, 1, 0x4dfac, @@ -2797,11 +2905,14 @@ void TestConservativeBufferReachability() { auto plan = ConditionalBufferPlan(use); ResourceSnapshot snapshot; ResourceSpecialization specialization; + std::array expected{}; + if (use != ConditionalBufferUse::Writable) + std::copy_n(user_data.begin() + 4, expected.size(), expected.begin()); Check(MaterializeResources(plan, runtime, snapshot, specialization) && snapshot.buffers.size() == 2 && - std::equal(user_data.begin() + 4, user_data.end(), + std::equal(expected.begin(), expected.end(), snapshot.buffers[1].dwords.begin()), - "shared, loop-dependent, or writable-alias resource was pruned"); + "shared or loop-dependent resource was pruned, or inactive scalar branch retained"); } } @@ -3187,6 +3298,7 @@ int main() { Run("dynamic FLAT address", TestDynamicFlatAddressesUseDma); Run("buffer swizzle specialization", TestBufferSwizzleSpecialization); Run("conditional buffer materialization", TestConditionalBufferMaterialization); + Run("guarded scalar descriptor reads", TestGuardedScalarDescriptorReads); Run("conservative buffer reachability", TestConservativeBufferReachability); Run("conditional indirect image", TestConditionalIndirectImageMaterialization); Run("shader info and bindings", TestShaderInfoAndBindingLayout); diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 2311cc744..541c77875 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -1497,6 +1497,7 @@ CompiledShader CompileCase(const TestCase &test, u32 host_subgroup_size = 64) { .shader_base = reinterpret_cast(test.code.data()), .read_memory = ReadTestMemory, .userdata = const_cast *>(&test.initial), + .read_specialization_memory = ReadTestMemory, }; Require(test.name, "resource materialization", ShaderRecompiler::IR::MaterializeResources( @@ -33928,7 +33929,8 @@ void CheckResourcePlanHandoff() { ResourceSpecialization specialization; const SrtRuntime runtime{.user_data = user_data, .read_memory = ReadTestMemory, - .userdata = &memory}; + .userdata = &memory, + .read_specialization_memory = ReadTestMemory}; Require(name, "initial descriptors", MaterializeResources(plan, runtime, snapshot, specialization) && snapshot.buffers.size() == 1 && snapshot.buffers[0].dwords[0] == 0x1000u, diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 9a35cb16c..1b5049711 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -8584,11 +8584,17 @@ void TestNestedSelectionPreservesDescriptorSources() { "nested descriptor selection lost its structured resource bindings"); for (size_t i = 0; i < result.program.info.buffers.size(); ++i) { const auto pc = result.program.info.buffers[i].first_use_pc; - const uint32_t expected = pc == 8u * 4u ? table[0] - : pc == 14u * 4u ? table[4] - : pc == direct_arm * 4u ? user_data[8] - : pc == join * 4u ? user_data[4] : 0u; - Check(expected != 0u && result.resources.buffers[i].dwords[0] == expected, + Check(pc == 8u * 4u || pc == 14u * 4u || pc == direct_arm * 4u || pc == join * 4u, + "nested descriptor selection lost a native source location"); + const auto selected_pc = (mode == 0u ? 8u : mode == 1u ? 14u : direct_arm) * 4u; + const auto *expected = pc == 8u * 4u ? table.data() + : pc == 14u * 4u ? table.data() + 4 + : pc == direct_arm * 4u ? user_data.data() + 8 : user_data.data() + 4; + const auto &descriptor = result.resources.buffers[i]; + const bool unused = pc != selected_pc && pc != join * 4u; + Check(descriptor.dword_count == 4 && + (std::equal(expected, expected + 4, descriptor.dwords.begin()) || + (unused && std::ranges::all_of(descriptor.dwords, [](auto word) { return word == 0; }))), "nested descriptor selection chose another arm's descriptor"); } CheckSpirvBinaryValidates(result.spirv); @@ -8696,10 +8702,10 @@ void TestNativeScalarReadDescriptorPlanning() { auto options = MakeCompileOptions(ShaderType::Pixel); options.user_data = user_data; const auto result = RecompileForTest(shader, options, ReadHostTestMemory); - Check(!result.program.dispatcher_fallback && result.resources.buffers.size() == 1u && - std::ranges::equal(result.resources.flattened_srt, table), - "scalar descriptor planning retained an unrelated native execution mask"); const auto expected = mode == 2u ? table : std::array{}; + Check(!result.program.dispatcher_fallback && result.resources.buffers.size() == 1u && + std::ranges::equal(result.resources.flattened_srt, expected), + "scalar descriptor planning retained an unrelated native execution mask"); Check(std::equal(expected.begin(), expected.end(), result.resources.buffers[0].dwords.begin()), "scalar descriptor planning selected the wrong native resource"); CheckSpirvBinaryValidates(result.spirv);