From 054f8ff4de13a9bcd8329e762d8f38a609d74fc4 Mon Sep 17 00:00:00 2001 From: thuanquang Date: Wed, 21 Oct 2026 11:16:03 +0700 Subject: [PATCH 1/2] fix(shader): index tessellation attributes at a constant address with constants TessellationPointer built the vertex, attribute and component indices of an attribute access from its address with OpUDiv, OpUMod, OpShiftRightLogical and OpBitwiseAnd even when the address was an immediate, and without SPIRV-Tools nothing folded them. AMD's Windows driver (32.0.21045.5002 on an RX 6700 XT, 32.0.21042.62 on an RDNA 2 iGPU) reads a per-vertex hull output indexed that way wrongly: a merged hull program's level read came back wrong, and agc_driver_tessellation drew a different wrong patch on each run. A read of the program's own record through a constant index failed the same way, and the test passes once the indices are OpConstants, as glslang emits them. The merged hull case now also checks that no access chain index of the hull stage is arithmetic on constants, so the shape is caught on every device. Fixes #8861 Co-Authored-By: Claude --- .../tests/execution/Tessellation.cpp | 24 +++++++++++++++++++ .../SpirvBackend/src/SpirvModuleEmitter.cpp | 14 +++++++++++ 2 files changed, 38 insertions(+) diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/Tessellation.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/Tessellation.cpp index 87c935f0d248..8eee7b840bec 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/Tessellation.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/Tessellation.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -946,6 +947,28 @@ void RequireMergedHullWithoutSubgroups(const ShaderRecompiler::SpirvTarget& targ } } +void RequireMergedHullConstantIndices(const ShaderRecompiler::SpirvTarget& target) { + std::uint32_t cursor = 0; + const auto hull = Compile(target, ShaderStage::TessellationControl, MergedHullCode, cursor, MergedTriangles); + const auto words = hull.spirv.Words(); + std::unordered_set constants; + std::unordered_set folded; + for (std::size_t at = 5; at + 1u < words.size(); at += words[at] >> 16u) { + const auto count = words[at] >> 16u; + Require(count != 0u && at + count <= words.size(), "the merged hull program compiled to a malformed SPIR-V instruction"); + const auto opcode = words[at] & 0xffffu; + if (opcode == spv::OpConstant && count >= 3u) constants.insert(words[at + 2u]); + const bool integer = opcode == spv::OpIAdd || opcode == spv::OpIMul || opcode == spv::OpUDiv || opcode == spv::OpUMod || opcode == spv::OpShiftRightLogical || opcode == spv::OpBitwiseAnd; + const auto operand = [&](std::uint32_t id) { return constants.contains(id) || folded.contains(id); }; + if (integer && count == 5u && operand(words[at + 3u]) && operand(words[at + 4u])) folded.insert(words[at + 2u]); + if (opcode == spv::OpAccessChain) { + for (std::uint32_t index = 4u; index < count; ++index) { + Require(!folded.contains(words[at + index]), "the merged hull program indexes a tessellation attribute with arithmetic on constants instead of a constant, which AMD's Windows driver reads wrongly"); + } + } + } +} + void RequireRegisteredMergedStages() { const auto low = [](const auto& code) { return static_cast(reinterpret_cast(code.data()) >> 8u); }; const auto high = [](const auto& code) { return static_cast(reinterpret_cast(code.data()) >> 40u); }; @@ -1003,6 +1026,7 @@ int main() { ExpectRefusal(target, ShaderStage::TessellationControl, MergedHullOutsidePatch, "reads LDS outside its patch's control point records", MergedTriangles); ExpectRefusal(target, ShaderStage::TessellationControl, MergedHullReadLane, "another lane's register", MergedTriangles); RequireMergedHullWithoutSubgroups(target); + RequireMergedHullConstantIndices(target); RequirePatches(*device, target, MergedHullCode, "merged hull program", MergedTriangles, MergedHullCode); RequirePatches(*device, target, MergedHullRegisters, "merged hull program storing its local part's registers", MergedTriangles, MergedHullRegisters); RequireCounterClockwise(*device, target); diff --git a/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp b/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp index 8449663a77e6..0ef03a1698ac 100644 --- a/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp +++ b/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp @@ -811,6 +811,20 @@ std::uint32_t TessellationPointer(SpirvValueEmitContext& ctx, const IrValue& ins state.module.AddFunction(spv::OpAccessChain, type, pointer, variable, vertex, ConstantU32(state, constant % stride / 16u), ConstantU32(state, constant % stride % 16u / 4u)); return pointer; } + if (inst.Argument(1)->HasImmediate()) { + const auto value = inst.Argument(1)->ImmediateU32(); + const auto offset = patch ? value : value % stride; + const auto attribute = ConstantU32(state, offset >> 4u); + const auto component = ConstantU32(state, (offset >> 2u) & 3u); + if (kind == TessellationAttribute::LocalOutput || patch) { + state.module.AddFunction(spv::OpAccessChain, type, pointer, variable, attribute, component); + return pointer; + } + const bool own = kind == TessellationAttribute::ControlOutput || (kind == TessellationAttribute::ControlShared && !sharedRead); + const auto controlPoint = kind == TessellationAttribute::ControlInput ? std::min(value / stride, state.inputInfo.vertex->tess.inputControlPoints - 1u) : value / stride; + state.module.AddFunction(spv::OpAccessChain, type, pointer, variable, own ? EmitLaneId(state) : ConstantU32(state, controlPoint), attribute, component); + return pointer; + } const auto offset = patch ? address : EmitBinaryU32(state, spv::OpUMod, address, ConstantU32(state, stride)); const auto attribute = EmitBinaryU32(state, spv::OpShiftRightLogical, offset, ConstantU32(state, 4u)); const auto component = EmitBinaryU32(state, spv::OpBitwiseAnd, EmitBinaryU32(state, spv::OpShiftRightLogical, offset, ConstantU32(state, 2u)), ConstantU32(state, 3u)); From e4de56f3ce528ea30a3e42794282d883c1f81114 Mon Sep 17 00:00:00 2001 From: thuanquang Date: Thu, 22 Oct 2026 10:39:26 +0700 Subject: [PATCH 2/2] fix(shader): clamp a domain read at an immediate address to the last control point The dynamic path of TessellationPointer passes the control point through inPatch(), which clamps it with UMin to inputControlPoints - 1 for hull inputs and to outputControlPoints - 1 for domain inputs. The immediate path clamped only hull inputs, so a domain read at an immediate address past the last control point indexed the input array past its end. It now clamps domain inputs to outputControlPoints - 1 too. Co-Authored-By: Claude --- .../tests/TessellationLowering.cpp | 55 +++++++++++++++++++ .../SpirvBackend/src/SpirvModuleEmitter.cpp | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/TessellationLowering.cpp b/core/libs/prx/libSceAgcDriver/tests/TessellationLowering.cpp index 1b1ec2d0dac5..2ccb9129acdd 100644 --- a/core/libs/prx/libSceAgcDriver/tests/TessellationLowering.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/TessellationLowering.cpp @@ -1,11 +1,16 @@ #include "IntermediateRepresentation/IrBuilder.hpp" #include "IntermediateRepresentation/IrProgram.hpp" +#include "Optimization/ShaderStageInputInfo.hpp" #include "Optimization/TessellationLowering.hpp" +#include "SpirvBackend/SpirvEmitter.hpp" +#include #include #include #include #include +#include #include +#include using namespace ShaderRecompiler; @@ -135,6 +140,52 @@ bool RefusedBecause(IrProgram& program, const char* reason) { return !outcome.lowered && outcome.reason.find(reason) != std::string::npos; } +std::uint32_t EmittedEvaluationControlPoint(std::uint32_t address) { + constexpr std::uint32_t ControlStride = 16u; + IrProgram program; + IrBuilder builder{program}; + program.SetWaveSize(64); + program.Resources().stage = IrShaderStage::TessellationEvaluation; + program.Resources().srtPlanComplete = true; + program.Resources().resourceTrackingComplete = true; + program.Metadata().shaderInfoComplete = true; + program.Metadata().bindingLayoutComplete = true; + program.Metadata().tessellationControlStride = ControlStride; + auto& block = program.CreateBlock(); + program.SetEntryBlock(block); + program.BlockOrder().push_back(&block); + BlockInfo info; + info.id = block.Id(); + program.Metadata().blockInfo.push_back(info); + builder.SetInsertionPoint(block); + auto& read = builder.Emit(IrOpcode::GetTessellationAttribute, IrType::U32, {&builder.Constant(static_cast(TessellationAttribute::EvaluationInput)), &builder.Constant(address), &builder.ConstantBool(true)}); + static_cast(builder.Emit(IrOpcode::ReferenceU32, IrType::Void, {&read})); + ShaderVertexInputInfo vertex; + vertex.tess.inputControlPoints = InputControlPoints; + vertex.tess.outputControlPoints = InputControlPoints; + vertex.tess.domain = 1u; + vertex.tess.partitioning = 2u; + vertex.tess.outputTopology = 2u; + ShaderStageInputInfo inputs; + inputs.vertex = &vertex; + CompiledBindingLayout bindings; + bindings.layout = program.Metadata().bindings; + SpirvTargetOptions target{}; + target.vulkanVersion = 0x00401000; + target.spirvVersion = 0x00010300; + target.subgroupSize = 64; + const auto words = SpirvEmitter().Emit(program, inputs, bindings, target); + std::unordered_map constants; + for (std::size_t at = 5; at < words.size(); at += words[at] >> 16u) { + const auto count = words[at] >> 16u; + if (count == 0u || at + count > words.size()) throw std::runtime_error("the domain program compiled to a malformed SPIR-V instruction"); + const auto opcode = words[at] & 0xffffu; + if (opcode == spv::OpConstant && count == 4u) constants[words[at + 2u]] = words[at + 3u]; + if (opcode == spv::OpAccessChain && count == 7u) return constants.at(words[at + 4u]); + } + throw std::runtime_error("the domain program reads no control point attribute"); +} + bool run(const char* name, const std::function& body) { bool passed = false; try { @@ -206,5 +257,9 @@ int main() { return patch->Opcode() == IrOpcode::IMul32 && patch->Argument(1)->Resolve()->ImmediateU32() == PatchStride; }); + passed &= run("a domain read at an immediate address past the last control point reads the last control point", [] { + return EmittedEvaluationControlPoint(InputControlPoints * 16u) == InputControlPoints - 1u; + }); + return passed ? 0 : 1; } diff --git a/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp b/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp index 0ef03a1698ac..87ed5233c6e4 100644 --- a/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp +++ b/core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp @@ -821,7 +821,7 @@ std::uint32_t TessellationPointer(SpirvValueEmitContext& ctx, const IrValue& ins return pointer; } const bool own = kind == TessellationAttribute::ControlOutput || (kind == TessellationAttribute::ControlShared && !sharedRead); - const auto controlPoint = kind == TessellationAttribute::ControlInput ? std::min(value / stride, state.inputInfo.vertex->tess.inputControlPoints - 1u) : value / stride; + const auto controlPoint = kind == TessellationAttribute::ControlInput ? std::min(value / stride, state.inputInfo.vertex->tess.inputControlPoints - 1u) : kind == TessellationAttribute::EvaluationInput ? std::min(value / stride, state.inputInfo.vertex->tess.outputControlPoints - 1u) : value / stride; state.module.AddFunction(spv::OpAccessChain, type, pointer, variable, own ? EmitLaneId(state) : ConstantU32(state, controlPoint), attribute, component); return pointer; }