From c83eb1d1f5c14d0842a165de431ac6367dff2909 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:19:46 -0500 Subject: shader: tell handle-program textures apart when adding descriptors Every texture whose handle is computed per draw gets the same placeholder constant buffer address (index 0, offset 0), and Descriptors::Add did not compare handle_program. Two such textures of the same type and depth in one shader therefore shared a descriptor, so the second sampled the first, and one could also merge with a real texture bound at constant buffer 0 offset 0. Compare handle_program too. The pixel-format-integer lookup cached its last answer by (index, offset), which has the same problem for handle programs; skip that cache for them. Follow-up to 561f87abec. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/shader_recompiler/ir_opt/texture_pass.cpp') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index 8a72f565dc..514cd9b2b6 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -1036,7 +1036,8 @@ public: desc.secondary_cbuf_index == existing.secondary_cbuf_index && desc.secondary_cbuf_offset == existing.secondary_cbuf_offset && desc.secondary_shift_left == existing.secondary_shift_left && - desc.count == existing.count && desc.size_shift == existing.size_shift; + desc.count == existing.count && desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; // TODO: Read this from TIC texture_descriptors[index].is_multisample |= desc.is_multisample; @@ -1171,6 +1172,10 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo u32 last_cbuf_offset{}; bool last_is_integer{false}; const auto is_texture_pixel_format_integer{[&](const ConstBufferAddr& cbuf_addr) { + if (cbuf_addr.handle_program != ~0U) { + // Every handle program is at cbuf 0 offset 0: the cache below can't tell them apart. + return IsTexturePixelFormatIntegerCached(env, cbuf_addr); + } if (has_last_is_integer && last_cbuf_index == cbuf_addr.index && last_cbuf_offset == cbuf_addr.offset) { return last_is_integer; -- cgit v1.2.3 From bda49e810b5955bff8474e7b8f2b09eab51e9be2 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:28:52 -0500 Subject: shader: evaluate handle programs for image and texture buffer descriptors 561f87abec only gave TextureDescriptor a handle_program. Image load/store/atomics and texture/image buffers whose handle comes from a storage buffer still got the placeholder constant buffer address (index 0, offset 0), so the pipelines read cbuf 0 word 0 as the handle (and asserted if cbuf 0 was unbound). Add handle_program to TextureBufferDescriptor, ImageBufferDescriptor and ImageDescriptor, compare it when merging, and let all four pipelines evaluate it for every descriptor kind instead of only TextureDescriptor. ImageQueryDimensions on such a handle was always declared Color2D, so array, cube and 3D views got a 2D declaration. The instruction carries no type, so take it from another instruction using the same handle program, falling back to Color2D when there is none. Follow-up to 561f87abec. The OpenGL pipeline edits are the same change as the Vulkan ones but were not compiled here. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 34 ++++++++++++++++++---- src/shader_recompiler/shader_info.h | 9 ++++++ .../renderer_opengl/gl_compute_pipeline.cpp | 25 ++++++++-------- .../renderer_opengl/gl_graphics_pipeline.cpp | 22 +++++++------- .../renderer_vulkan/vk_compute_pipeline.cpp | 25 ++++++++-------- .../renderer_vulkan/vk_graphics_pipeline.cpp | 22 +++++++------- 6 files changed, 82 insertions(+), 55 deletions(-) (limited to 'src/shader_recompiler/ir_opt/texture_pass.cpp') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index 514cd9b2b6..db8f1ac716 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -345,7 +345,8 @@ static inline u32 GetTextureHandleCached(Environment& env, const ConstBufferAddr // Cached variants of existing helpers // A handle computed per draw has no descriptor to read at translation time: assume an ordinary -// floating-point 2D texture, the instruction's own type deciding the rest. +// floating-point 2D texture, the instruction's own type deciding the rest (see the TexturePass +// ImageQueryDimensions case, which has none). static inline TextureType ReadTextureTypeCached(Environment& env, const ConstBufferAddr& cbuf) { if (cbuf.handle_program != ~0U) return TextureType::Color2D; return env.ReadTextureType(GetTextureHandleCached(env, cbuf)); @@ -1010,7 +1011,8 @@ public: desc.secondary_cbuf_offset == existing.secondary_cbuf_offset && desc.secondary_shift_left == existing.secondary_shift_left && desc.count == existing.count && desc.size_shift == existing.size_shift && - desc.has_secondary == existing.has_secondary; + desc.has_secondary == existing.has_secondary && + desc.handle_program == existing.handle_program; }); } @@ -1018,7 +1020,8 @@ public: const u32 index{Add(image_buffer_descriptors, desc, [&desc](const auto& existing) { return desc.format == existing.format && desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && desc.count == existing.count && - desc.size_shift == existing.size_shift; + desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; image_buffer_descriptors[index].is_written |= desc.is_written; image_buffer_descriptors[index].is_read |= desc.is_read; @@ -1050,7 +1053,8 @@ public: return desc.type == existing.type && desc.format == existing.format && desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && desc.count == existing.count && - desc.size_shift == existing.size_shift; + desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; image_descriptors[index].is_written |= desc.is_written; image_descriptors[index].is_read |= desc.is_read; @@ -1186,6 +1190,18 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo has_last_is_integer = true; return last_is_integer; }}; + // A handle program has no descriptor to read a type from, and ImageQueryDimensions carries + // none: it takes the type of another instruction using the same handle. + std::unordered_map handle_program_types; + for (const TextureInst& texture_inst : to_replace) { + const IR::Opcode opcode{IndexedInstruction(*texture_inst.inst)}; + if (texture_inst.cbuf.handle_program != ~0U && opcode != IR::Opcode::ImageQueryDimensions && + !IsStorageImageOpcode(opcode)) { + handle_program_types.try_emplace( + texture_inst.cbuf.handle_program, + texture_inst.inst->Flags().type.Value()); + } + } for (TextureInst& texture_inst : to_replace) { // TODO: Handle arrays IR::Inst* const inst{texture_inst.inst}; @@ -1196,7 +1212,12 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo bool is_multisample{false}; switch (inst->GetOpcode()) { case IR::Opcode::ImageQueryDimensions: - flags.type.Assign(ReadTextureTypeCached(env, cbuf)); + if (const auto it{handle_program_types.find(cbuf.handle_program)}; + it != handle_program_types.end()) { + flags.type.Assign(it->second); + } else { + flags.type.Assign(ReadTextureTypeCached(env, cbuf)); + } inst->SetFlags(flags); break; case IR::Opcode::ImageSampleImplicitLod: @@ -1261,6 +1282,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .cbuf_offset = cbuf.offset, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } else { index = descriptors.Add(ImageDescriptor{ @@ -1273,6 +1295,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .cbuf_offset = cbuf.offset, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } break; @@ -1289,6 +1312,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .secondary_shift_left = cbuf.secondary_shift_left, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } else { count = std::min(count, sampled_dynamic_cap); diff --git a/src/shader_recompiler/shader_info.h b/src/shader_recompiler/shader_info.h index eeb8d768d7..e2b341b0c3 100644 --- a/src/shader_recompiler/shader_info.h +++ b/src/shader_recompiler/shader_info.h @@ -189,6 +189,9 @@ struct TextureBufferDescriptor { u32 secondary_shift_left; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const TextureBufferDescriptor&) const = default; }; @@ -203,6 +206,9 @@ struct ImageBufferDescriptor { u32 cbuf_offset; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const ImageBufferDescriptor&) const = default; }; @@ -240,6 +246,9 @@ struct ImageDescriptor { u32 cbuf_offset; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const ImageDescriptor&) const = default; }; diff --git a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp index eb134e86b5..5fc10cfce2 100644 --- a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp +++ b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp @@ -106,19 +106,18 @@ void ComputePipeline::Configure() { const auto& cbufs{qmd.const_buffer_config}; const bool via_header_index{qmd.linked_tsc != 0}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this dispatch. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && - ((qmd.const_buffer_enable_mask >> cbuf) & 1) != 0 - ? cbufs[cbuf].Address() - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this dispatch. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && + ((qmd.const_buffer_enable_mask >> cbuf) & + 1) != 0 + ? cbufs[cbuf].Address() + : GPUVAddr{}; + }), + via_header_index); } ASSERT(((qmd.const_buffer_enable_mask >> desc.cbuf_index) & 1) != 0); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp index 81950279d5..4e02f0569b 100644 --- a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp +++ b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp @@ -309,18 +309,16 @@ bool GraphicsPipeline::ConfigureImpl(bool is_indexed) { } const auto& cbufs{maxwell3d->state.shader_stages[stage].const_buffers}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this draw. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && cbufs[cbuf].enabled - ? cbufs[cbuf].address - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this draw. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && cbufs[cbuf].enabled + ? cbufs[cbuf].address + : GPUVAddr{}; + }), + via_header_index); } ASSERT(cbufs[desc.cbuf_index].enabled); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp index 1cfe391ba6..3a3861930c 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp @@ -176,19 +176,18 @@ bool ComputePipeline::Configure(Tegra::Engines::KeplerCompute& kepler_compute, const auto& cbufs{qmd.const_buffer_config}; const bool via_header_index{qmd.linked_tsc != 0}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this dispatch. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && - ((qmd.const_buffer_enable_mask >> cbuf) & 1) != 0 - ? cbufs[cbuf].Address() - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this dispatch. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && + ((qmd.const_buffer_enable_mask >> cbuf) & + 1) != 0 + ? cbufs[cbuf].Address() + : GPUVAddr{}; + }), + via_header_index); } ASSERT(((qmd.const_buffer_enable_mask >> desc.cbuf_index) & 1) != 0); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp index fa273f6ef9..4bd2e2735e 100644 --- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp @@ -400,18 +400,16 @@ bool GraphicsPipeline::ConfigureImpl(bool is_indexed) { } const auto& cbufs{maxwell3d->state.shader_stages[stage].const_buffers}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this draw. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && cbufs[cbuf].enabled - ? cbufs[cbuf].address - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this draw. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && cbufs[cbuf].enabled + ? cbufs[cbuf].address + : GPUVAddr{}; + }), + via_header_index); } ASSERT(cbufs[desc.cbuf_index].enabled); const u32 index_offset{index << desc.size_shift}; -- cgit v1.2.3 From 8b0666b74868da8221d9e3ef3995f3a862ee24b6 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:29:14 -0500 Subject: shader: restore the handle build depth when a build throws BuildHandleProgram incremented g_handle_depth and decremented it at the end, so a NotImplementedException from BuildHandleNode left the thread-local depth above zero. That thread never cleared g_handle_slots again, and stale IR::Inst* keys from an earlier program could match instructions of a later one (a freed instruction's address can be reused). Do the decrement and the outermost clear in SCOPE_EXIT. Follow-up to e23b0f2fab. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src/shader_recompiler/ir_opt/texture_pass.cpp') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index db8f1ac716..b33700cabd 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -16,6 +16,7 @@ #include #include +#include "common/scope_exit.h" #include "common/settings.h" #include "shader_recompiler/environment.h" #include "shader_recompiler/frontend/ir/basic_block.h" @@ -455,6 +456,12 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud if (outermost) { g_handle_slots.clear(); } + SCOPE_EXIT { + --g_handle_depth; + if (outermost) { + g_handle_slots.clear(); + } + }; const IR::Inst* const inst{value.InstRecursive()}; bool result; if (const auto it = g_handle_slots.find(inst); it != g_handle_slots.end()) { @@ -484,10 +491,6 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud code.push_back(slot); } } - --g_handle_depth; - if (outermost) { - g_handle_slots.clear(); - } return result; } -- cgit v1.2.3 From 0ed4a3a9e795896a4751d0330c78958a309d1aa0 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:29:42 -0500 Subject: shader: drop the slots of a failed handle subtree with its code When BuildHandleProgram failed on a subtree it truncated the code back to where the subtree started, which removed the Tee ops of the values the subtree had kept, but their entries stayed in g_handle_slots. A Phi tries its other inputs after the first one fails; if one of them reused a value from the dropped subtree, it emitted a Load of a slot that nothing wrote, so the handle read 0. Number slots with a counter instead of counting map entries, and on failure erase the entries numbered from the subtree's first slot on and rewind the counter, so such a value is built again. Follow-up to 561f87abec. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/shader_recompiler/ir_opt/texture_pass.cpp') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index b33700cabd..b6aca0a118 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -438,9 +438,10 @@ bool IsUndefined(IR::Opcode opcode) { opcode == IR::Opcode::UndefU64; } -// Values already emitted by the program being built, by the slot that keeps them, and how deep -// in nested BuildHandleProgram calls the build is. +// Values already emitted by the program being built, by the slot that keeps them, how many slots +// are in use, and how deep in nested BuildHandleProgram calls the build is. thread_local std::unordered_map g_handle_slots; +thread_local u32 g_handle_slot_count{}; thread_local int g_handle_depth{}; bool BuildHandleNode(const IR::Value& value, std::vector& code, int& budget, @@ -455,11 +456,13 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud const bool outermost = g_handle_depth++ == 0; if (outermost) { g_handle_slots.clear(); + g_handle_slot_count = 0; } SCOPE_EXIT { --g_handle_depth; if (outermost) { g_handle_slots.clear(); + g_handle_slot_count = 0; } }; const IR::Inst* const inst{value.InstRecursive()}; @@ -478,14 +481,20 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud } else { g_handle_slots.emplace(inst, ~0U); const size_t start = code.size(); + const u32 first_slot = g_handle_slot_count; result = BuildHandleNode(value, code, budget, block); g_handle_slots.erase(inst); if (!result) { + // The Tee of every value the dropped code kept goes with it: a later use of one of + // them (another Phi input) must build it again, not load a slot nothing wrote. code.resize(start); + std::erase_if(g_handle_slots, [first_slot](const auto& entry) { + return entry.second != ~0U && entry.second >= first_slot; + }); + g_handle_slot_count = first_slot; } if (result && inst->UseCount() > 1 && g_handle_slots.size() < 256) { - const u32 slot = static_cast( - std::ranges::count_if(g_handle_slots, [](const auto& entry) { return entry.second != ~0U; })); + const u32 slot = g_handle_slot_count++; g_handle_slots.emplace(inst, slot); code.push_back(static_cast(Op::Tee)); code.push_back(slot); -- cgit v1.2.3