From 0977d859a131806b7f39e612e53a4d214819beea Mon Sep 17 00:00:00 2001 From: nmzik Date: Mon, 28 Sep 2026 12:58:22 +0400 Subject: [PATCH] shader: preserve scalar buffer offset alignment and carry --- .../backend/spirv/spirvEmitterMemory.cpp | 9 +-- .../ir/passes/ResourceMaterialization.cpp | 24 ++++---- .../recompiler/ir/passes/ResourceTracking.cpp | 6 +- .../shader/recompiler/ir/passes/SrtWalker.cpp | 7 +-- tests/ResourceTrackingTests.cpp | 46 ++++++++++++---- tests/ScalarProvenanceTests.cpp | 55 +++++++++---------- tests/ShaderRecompilerComputeTests.cpp | 30 ++++++++++ tests/shaderCfgTests.cpp | 6 +- 8 files changed, 118 insertions(+), 65 deletions(-) diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp index 8e10b4266..3d21e0f4b 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp @@ -1187,10 +1187,11 @@ void EmitReadConstBuffer(ValueEmitContext& ctx, const IR::Inst& inst) { if (mem.planning_only) return; auto& state = ctx.state; mem.kind = IR::ResourceKind::ScalarBuffer; - const auto address = Binary(state, spv::OpIAdd, TypeU32(state), ctx.Arg(inst, 1), - ConstantU32(state, mem.offset)); - const auto index = - Binary(state, spv::OpShiftRightLogical, TypeU32(state), address, ConstantU32(state, 2)); + auto index = Binary(state, spv::OpShiftRightLogical, TypeU32(state), ctx.Arg(inst, 1), + ConstantU32(state, 2)); + if (mem.offset >= 4u) { + index = Binary(state, spv::OpIAdd, TypeU32(state), index, ConstantU32(state, mem.offset >> 2u)); + } const auto access = PrepareMemoryResourceAccess(state, mem); const auto element = EmitMemoryElementIndex(state, access, index); const auto condition = EmitMemoryElementInBounds(state, access, element); diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index ebe2da6c0..57b30d2f4 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -225,9 +225,9 @@ void MarkCleanFlatSlots(const ResourcePlan& program, const DescriptorSource* sou } } -bool ReadScalarTable(uint64_t base, uint64_t size, uint32_t dynamic_offset, +bool ReadScalarTable(uint64_t base, uint64_t size, uint64_t dynamic_offset, const SrtRuntime& runtime, std::span words) { - const auto offset = static_cast(dynamic_offset) & ~uint64_t {3}; + const auto offset = dynamic_offset & ~uint64_t {3}; const auto count = std::min(words.size(), offset < size ? (size - offset) / 4u : 0u); std::ranges::fill(words.subspan(count), 0u); if (count == 0u) { @@ -306,26 +306,24 @@ bool MaterializeIndirectImage(const ResourcePlan& program, material.Stride() != indirect.selector_stride) { return false; } - // Enumerate every wrapped scalar-buffer offset that can pass the descriptor bounds. - const auto step = std::gcd(indirect.selector_stride, uint64_t {1} << 32u); - const auto residue = static_cast(indirect.selector_offset) % step; - const auto limit = std::min(UINT32_MAX, material.GetSize() + 3u); - const auto probe_count = residue <= limit ? (limit - residue) / step + 1u : 0u; + // The first aligned offset includes the immediate added after shader U32 arithmetic. + const auto step = std::max(4u, + std::gcd(indirect.selector_stride, uint64_t {1} << 32u)); + const uint64_t first = indirect.selector_offset; + const auto size = material.GetSize(); + const auto limit = std::min(first + (uint64_t {1} << 32u) - step, size >= 4u ? size - 4u : 0u); + const auto probe_count = size >= 4u && first <= limit ? (limit - first) / step + 1u : 0u; if (probe_count > MaxIndirectImageProbes) { return false; } keys.reserve(static_cast(probe_count) + 1u); keys.push_back(0u); - for (uint64_t offset = residue; offset <= limit && probe_count != 0u; offset += step) { + for (uint64_t probe = 0, offset = first; probe < probe_count; ++probe, offset += step) { uint32_t key = 0; - if (!ReadScalarTable(material.Base48(), material.GetSize(), - static_cast(offset), runtime, {&key, 1})) { + if (!ReadScalarTable(material.Base48(), size, offset, runtime, {&key, 1})) { return false; } keys.push_back(key); - if (limit - offset < step) { - break; - } } std::ranges::sort(keys); keys.erase(std::unique(keys.begin(), keys.end()), keys.end()); diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index 17a0bb0e5..4be82a638 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -1204,7 +1205,7 @@ private: const auto* memory = material_read != nullptr ? ScalarReadMemory(*material_read, material_memory_index) : nullptr; if (table_offset != 0u || memory == nullptr || memory->kind != ResourceKind::ScalarBuffer || - memory->offset != 0u || !MemoryIndexBelongsTo(material_memory_index, *material_read)) { + memory->offset > INT32_MAX || !MemoryIndexBelongsTo(material_memory_index, *material_read)) { return false; } Value selector; @@ -1212,6 +1213,9 @@ private: indirect.selector_offset)) { return false; } + const auto step = std::gcd(indirect.selector_stride, uint64_t {1} << 32u); + indirect.selector_offset = + (static_cast(indirect.selector_offset % step) & ~3u) + (memory->offset & ~3u); const auto* shift = plan.reads[0]->Arg(1).Resolve().TryInstruction(); const std::array material_users {shift}; if (!UsesOnly(*material_read, material_users) || !UsesOnly(*shift, plan.reads)) { diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp index 953051551..34fdc08b0 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp @@ -456,16 +456,15 @@ bool SrtWalker::EvaluateRawRead(const Inst& inst, uint64_t& result) { return false; } const auto byte_offset = - static_cast(immediate) + static_cast(offset); - const auto aligned = byte_offset & ~uint64_t {3}; + (static_cast(immediate) & ~uint64_t {3}) + (static_cast(offset) & ~3u); const auto stride = (static_cast(high) >> 16u) & 0x3fffu; const auto size = stride == 0u ? static_cast(static_cast(records)) : static_cast(stride) * static_cast(records); - if (aligned > size || size - aligned < sizeof(uint32_t)) { + if (byte_offset > size || size - byte_offset < sizeof(uint32_t)) { return false; } - address = ((base & ~uint64_t {3}) + byte_offset) & ~uint64_t {3}; + 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/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index 283426027..ea99186de 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -206,8 +206,9 @@ bool ReadLinearTestMemory(void *userdata, uint64_t address, std::span } std::unique_ptr -MakeIndirectImageFixture(bool malformed, uint32_t material_immediate = 0, - bool memory_backed_material = false) { +MakeIndirectImageFixture(bool malformed, uint32_t material_immediate = 4, + bool memory_backed_material = false, uint32_t member_offset = 0, + uint32_t material_stride = 224) { auto fixture = std::make_unique(); std::array material_words; std::array heap_words; @@ -249,9 +250,9 @@ MakeIndirectImageFixture(bool malformed, uint32_t material_immediate = 0, const auto selector = fixture->Emit(ValueOpcode::ReadFirstLane, {invocation, Value(true)}); const auto record = - fixture->Emit(ValueOpcode::IMul32, {selector, Value(224u)}); - const auto member = fixture->Emit(ValueOpcode::IAdd32, {record, Value(4u)}); - fixture->Emit(ValueOpcode::ReferenceU32, {record}); + fixture->Emit(ValueOpcode::IMul32, {selector, Value(material_stride)}); + const auto member = member_offset == 0 ? record : + fixture->Emit(ValueOpcode::IAdd32, {record, Value(member_offset)}); fixture->Emit(ValueOpcode::ReferenceU32, {member}); MemoryInfo material_scalar; material_scalar.kind = ResourceKind::ScalarBuffer; @@ -497,7 +498,7 @@ void TestInvariantIndirectImageMaterialization() { rebound_specialization != collapsed_specialization, "larger indirect candidate topology reused the old specialization"); - auto memory_backed = MakeIndirectImageFixture(false, 0u, true); + auto memory_backed = MakeIndirectImageFixture(false, 4u, true); memory_backed->PlanAndTrack(); auto memory_backed_plan = ExtractResourcePlan(memory_backed->program); EliminateDeadCode(memory_backed->program.blocks); @@ -550,13 +551,34 @@ void TestInvariantIndirectImageMaterialization() { malformed->program.descriptor_sources.empty(), "malformed indirect image pattern was partially accepted"); - auto wrapped_immediate = MakeIndirectImageFixture(false, 4u); - - CheckFatal([&] { wrapped_immediate->PlanAndTrack(); }, + auto negative_immediate = MakeIndirectImageFixture(false, 0xfffffffcu); + CheckFatal([&] { negative_immediate->PlanAndTrack(); }, "not a valid runtime value", - "wrapped scalar immediate entered the invariant image proof"); - Check(!wrapped_immediate->program.resource_tracking_complete, - "wrapped scalar immediate entered the invariant image proof"); + "negative scalar immediate entered the indirect image proof"); + + for (const auto [immediate, member, first, stride] : + {std::array{0u, 4u, 4u, 224u}, std::array{4u, 4u, 8u, 224u}, + std::array{36u, 0u, 36u, 224u}, std::array{1u, 3u, 0u, 224u}, + std::array{1u, 3u, 0u, 1u}, std::array{1u, 3u, 0u, 2u}}) { + auto split_offset = MakeIndirectImageFixture(false, immediate, false, member, stride); + split_offset->PlanAndTrack(); + const auto split_plan = ExtractResourcePlan(split_offset->program); + LinearTestMemory split_memory; + std::copy(image_descriptor.begin(), image_descriptor.end(), + split_memory.words.begin() + 0x1000u / 4u); + split_memory.fail_address = first == 36u ? 0x1004u : UINT64_MAX; + split_memory.watched_address = split_memory.base + first; + user_data[1] = stride << 16u; + user_data[2] = stride < 4u ? 8u / stride : 1u; + SrtRuntime split_runtime{.user_data = user_data, + .userdata = &split_memory, + .read_specialization_memory = ReadLinearTestMemory}; + Check(MaterializeResources(split_plan, split_runtime, snapshot, specialization) && + snapshot.images.size() == 1 && + snapshot.images[0].dwords == image_descriptor && + split_memory.watched_reads == 1u, + "indirect scalar offsets did not align and bound their components independently"); + } } void TestGuardedDirectImageTable() { diff --git a/tests/ScalarProvenanceTests.cpp b/tests/ScalarProvenanceTests.cpp index f24c3b377..0144fdb50 100644 --- a/tests/ScalarProvenanceTests.cpp +++ b/tests/ScalarProvenanceTests.cpp @@ -488,35 +488,34 @@ void TestSharedIntegerRuntimeDependencies() { } void TestConstantBufferBounds() { - Fixture fixture; - const auto memory = fixture.AddMemory(ResourceKind::ScalarBuffer); - const auto buffer = - fixture.Emit(ValueOpcode::GetBufferResource, - {Value(0x3000u), Value(0u), Value(16u), Value(0u)}); - const auto read = fixture.EmitMemory(ValueOpcode::ReadConstBuffer, - {buffer, Value(12u)}, memory); - LoadBuffer(fixture, {read, Value(0u), Value(16u), Value(0u)}); - fixture.Plan(); + struct Case { + uint32_t offset; + uint32_t immediate; + bool valid; + uint32_t expected; + }; + for (const auto &test : {Case{12u, 0u, true, 0xa5a5a5a5u}, + Case{3u, 1u, true, 0x12345678u}, + Case{0xfffffffcu, 4u, false, 0u}, + Case{16u, 0u, false, 0u}}) { + Fixture fixture; + const auto memory = fixture.AddMemory(ResourceKind::ScalarBuffer, test.immediate); + const auto buffer = + fixture.Emit(ValueOpcode::GetBufferResource, + {Value(0x3000u), Value(0u), Value(16u), Value(0u)}); + const auto read = fixture.EmitMemory(ValueOpcode::ReadConstBuffer, + {buffer, Value(test.offset)}, memory); + LoadBuffer(fixture, {read, Value(0u), Value(16u), Value(0u)}); + fixture.Plan(); - TestMemory memory_image{{{0x300cu, 0xa5a5a5a5u}}}; - SrtRuntime runtime{.read_memory = ReadMemory, .userdata = &memory_image}; - std::vector flat; - Check(SrtWalker(fixture.program, runtime).RefreshFlatBuffer(flat) && - flat == std::vector{0xa5a5a5a5u}, - "constant-buffer SRT walk failed"); - - Fixture overflow; - const auto overflow_memory = overflow.AddMemory(ResourceKind::ScalarBuffer); - const auto overflow_buffer = - overflow.Emit(ValueOpcode::GetBufferResource, - {Value(0x3000u), Value(0u), Value(16u), Value(0u)}); - const auto overflow_read = - overflow.EmitMemory(ValueOpcode::ReadConstBuffer, - {overflow_buffer, Value(16u)}, overflow_memory); - LoadBuffer(overflow, {overflow_read, Value(0u), Value(16u), Value(0u)}); - overflow.Plan(); - Check(!SrtWalker(overflow.program, runtime).RefreshFlatBuffer(flat), - "out-of-bounds constant-buffer walk was accepted"); + TestMemory memory_image{{{0x3000u, 0x12345678u}, {0x300cu, 0xa5a5a5a5u}}}; + SrtRuntime runtime{.read_memory = ReadMemory, .userdata = &memory_image}; + std::vector flat; + Check(SrtWalker(fixture.program, runtime).RefreshFlatBuffer(flat) == test.valid, + "constant-buffer walk misaligned or wrapped its offset components"); + Check(test.valid ? flat == std::vector{test.expected} : memory_image.reads == 0, + "constant-buffer walk read the wrong word or accessed an out-of-bounds address"); + } } void TestReadLaneElimination() { diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 47f2ef8b4..b2f64353a 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -23772,6 +23772,35 @@ TestCase ScalarMemoryLoadVariants() { O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; } +TestCase ScalarBufferOffsetAlignmentAndCarry() { + using O = ShaderOpcode; + std::vector code; + AppendSMovLiteral(&code, 12, 0xfffffffcu); + code.push_back(EncodeSop2(0x00, 12, 12, 4)); + code.push_back(EncodeSmem0(0x08, 20)); + code.push_back(EncodeSmem1(4, 12)); + AppendSMovLiteral(&code, 12, 3u); + code.push_back(EncodeSop2(0x00, 12, 12, 4)); + code.push_back(EncodeSmem0(0x08, 21)); + code.push_back(EncodeSmem1(1, 12)); + AppendStoreSgpr(&code, 20, 0); + AppendStoreSgpr(&code, 21, 1); + AppendEnd(&code); + + TestCase test{"ScalarBufferOffsetAlignmentAndCarry", code, + {0x11111111u, 0x22222222u}, {0u, 0x11111111u}, + {O::S_MOV_B32, O::S_ADD_U32, O::S_BUFFER_LOAD_DWORD, + O::V_MOV_B32, O::BUFFER_STORE_DWORD, O::S_ENDPGM}}; + test.forbidden_spirv = {"flattened_srt"}; + test.compute_info.threads_num[0] = 1; + test.compute_info.threads_num[1] = 1; + test.compute_info.threads_num[2] = 1; + test.compute_info.group_id[0] = true; + test.compute_info.workgroup_register = 4; + test.has_compute_info = true; + return test; +} + TestCase ScalarLoadSignedImmediateOffsetAddsSoffset() { using O = ShaderOpcode; @@ -29888,6 +29917,7 @@ std::vector MakeCases() { AddCase(BranchVccnzUsesCarryProducedWaveMask); AddCase(ScalarMemRealtimeCapturedPlaceholder); AddCase(ScalarMemoryLoadVariants); + AddCase(ScalarBufferOffsetAlignmentAndCarry); AddCase(ScalarLoadSignedImmediateOffsetAddsSoffset); AddCase(ScalarLoadAlignsComponentsAndMasksAddress); AddCase(ScalarLoadAlignsDynamicBase); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 745c85d8c..782081916 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -12822,10 +12822,10 @@ void TestSrtWalkerRealSBufferTranslation() { const auto walked = ShaderRecompiler::IR::SrtWalker(ir, runtime).RefreshFlatBuffer(flat); Check(walked, "SRT walk failed"); Check(flat.size() == 4 && - std::equal(flat.begin(), flat.end(), table.begin() + 1), - "real S_BUFFER_LOAD walk used the wrong final alignment"); + std::equal(flat.begin(), flat.end(), table.begin()), + "real S_BUFFER_LOAD walk did not align offset components independently"); - user_data[10] = 4 * sizeof(uint32_t); + user_data[10] = 3 * sizeof(uint32_t); const auto bounds_walked = ShaderRecompiler::IR::SrtWalker(ir, runtime).RefreshFlatBuffer(flat); Check(!bounds_walked, "real S_BUFFER_LOAD walk ignored descriptor bounds"); CheckFlattenedReadSlots(