diff --git a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp index 35940134a..e31510d5b 100644 --- a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp +++ b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp @@ -934,43 +934,6 @@ 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->module->info.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[id]; - if (!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->module->info; - for (const auto& resource: program.info.buffers) { - if (!resource.written || resource.descriptor_index == UINT32_MAX) continue; - const auto& written = writer->buffer_sources[resource.descriptor_index]; - // ADD_TID and swizzle move fixed-offset writes past write_extent. - const bool linear = (resource.packed_stride & ((1u << 14u) | (1u << 20u))) == 0u; - const auto written_size = - linear ? std::min(written.size, resource.write_extent) : written.size; - if (written_size != 0 && ImageRangeOverlaps(address, size, - written.address, written_size)) { - EXIT("scalar resource reads overlap a shader buffer write\n"); - } - } - } - } - } if (pipeline.uses_image_heap) { m_context.GetImageHeap().Commit(buffer, pipeline_bind_point, pipeline.pipeline_layout); } diff --git a/src/graphics/shader/recompiler/ir/Serialize.h b/src/graphics/shader/recompiler/ir/Serialize.h index 95044f323..21cc78141 100644 --- a/src/graphics/shader/recompiler/ir/Serialize.h +++ b/src/graphics/shader/recompiler/ir/Serialize.h @@ -148,7 +148,7 @@ template void Io(S& io, T& value) { IoAll(io, __VA_ARGS__); \ } KYTY_IO_FIELDS(MemoryInfo, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y) -KYTY_IO_FIELDS(BufferResource, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s) +KYTY_IO_FIELDS(BufferResource, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r) KYTY_IO_FIELDS(ImageResource, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w) KYTY_IO_FIELDS(SamplerResource, a, b, c, d, e, f, g, h, i, j, k) KYTY_IO_FIELDS(SampledResourcePair, a, b, c) diff --git a/src/graphics/shader/recompiler/ir/ShaderIR.h b/src/graphics/shader/recompiler/ir/ShaderIR.h index 4fce2eb53..09f6b03b6 100644 --- a/src/graphics/shader/recompiler/ir/ShaderIR.h +++ b/src/graphics/shader/recompiler/ir/ShaderIR.h @@ -122,7 +122,6 @@ struct BufferResource { uint32_t descriptor_index = UINT32_MAX; uint32_t first_use_pc = 0; uint32_t max_byte_extent = 0; - uint32_t write_extent = 0; // UINT32_MAX once a write is runtime-addressed or formatted uint32_t packed_stride = 0; Prospero::BufferFormat descriptor_format = Prospero::BufferFormat::kInvalid; uint32_t descriptor_swizzle = DstSel(4, 5, 6, 7); diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index 152400b19..6bfafa839 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -1167,7 +1167,7 @@ bool BuildSamplerPlan(const ShaderInfo& base, SamplerPlan& plan) { template static std::vector ResourceControlFlow(const Program& program, const Predicate& predicate) { - // The renderer checks captured scalar reads against final buffer and image write ranges. + // Address writes can change the values control flow reads. if (program.has_address_writes) { return {}; } diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index 6c4f16d2b..c8ebff0e6 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -4022,10 +4022,10 @@ private: } } - uint32_t AddBuffer(uint32_t source, const MemoryInfo& memory, const Inst& inst, uint32_t pc) { + uint32_t AddBuffer(uint32_t source, const MemoryInfo& memory, ValueOpcode op, uint32_t pc) { for (uint32_t i = 0; i < m_info.buffers.size(); i++) { if (m_info.buffers[i].source == source) { - Merge(m_info.buffers[i], memory, inst, pc); + Merge(m_info.buffers[i], memory, op, pc); return i; } } @@ -4035,14 +4035,13 @@ private: BufferResource resource; resource.source = source; resource.first_use_pc = pc; - Merge(resource, memory, inst, pc); + Merge(resource, memory, op, pc); m_info.buffers.push_back(resource); return static_cast(m_info.buffers.size() - 1); } - static void Merge(BufferResource& resource, const MemoryInfo& memory, const Inst& inst, + static void Merge(BufferResource& resource, const MemoryInfo& memory, ValueOpcode op, uint32_t pc) { - const auto op = inst.GetOpcode(); const auto access = BufferAccessOf(op); const bool atomic = access == BufferAccess::Atomic; const bool write = access == BufferAccess::Write || atomic; @@ -4050,13 +4049,6 @@ private: resource.max_byte_extent = std::max(resource.max_byte_extent, ByteExtent(memory)); resource.read = resource.read || !write || atomic; resource.written = resource.written || write; - if (write) { - // Formatted stores write the whole format; otherwise zero index, VGPR and SGPR offsets - // leave only the immediate offset. - const bool fixed = !memory.formatted && inst.Arg(1).Resolve() == Value(0u) && - inst.Arg(2).Resolve() == Value(0u) && inst.Arg(3).Resolve() == Value(0u); - resource.write_extent = std::max(resource.write_extent, fixed ? ByteExtent(memory) : UINT32_MAX); - } resource.atomic = resource.atomic || atomic; resource.formatted = resource.formatted || memory.formatted; resource.scalar = resource.scalar || op == ValueOpcode::ReadConstBuffer || @@ -4226,7 +4218,7 @@ private: m_info.uses_dma = true; return; } - resource = AddBuffer(source, memory, inst, flags.pc); + resource = AddBuffer(source, memory, op, flags.pc); if (resource == UINT32_MAX) { Fail(flags.pc, "buffer resource limit exceeded"); } diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 2578645b4..26e6d5e55 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -2331,6 +2331,10 @@ public: vertex.shader_data_buffer.offset == 0, "repeated shader binding preparation allocated or retained stale state"); + // Scalar resource reads are captured before the draw, so a stage that writes through + // computed addresses commits alongside them. + vertex_module.info.has_address_writes = true; + pixel_snapshot.specialization_reads = {{0x1000u, 4u}}; const auto pipeline = RenderExecutorTestAccess::CommitBindings( context.GetRenderExecutor(), scheduler.Current(), vertex, pixel); Require(name, "shared push data",