From c52321fab5ddd64ba09ff75759336ceee8a0749f Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Tue, 13 Oct 2026 13:18:51 +1300 Subject: [PATCH] fix(shader): move the primitive class to request version 19 and compose the ancillary word with render target index 0 main reads target.vertexSubgroups at request version 18 since #4470, so reading the pixel primitive class at 18 as well read a version 18 request written by main one byte short. The class is now written and read at version 19. agc_shader_memory loads a version 18 request as main writes it, and it fails with "truncated data" when the class is read at 18. The composed word no longer reads the Layer builtin for bits 16-28. Draws render to a single layer and guest layer exports are ignored, so no stage writes Layer, and AMD's Windows driver drops the writes of a fragment shader that reads it unwritten (#4800). The field is the constant 0 and pixel_ancillary checks that the composed program has no Layer read. The debt entry names the bits that have no source and are assumed zero (2-7, 12-15, 29-31), and says that a change which makes draws export a layer must restore the read. --- .../libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp | 13 +++++++++++-- core/libs/tests/PixelAncillary.cpp | 12 ++++++++---- .../ControlFlow/src/RequestSerializer.cpp | 6 +++--- .../recompiler/Optimization/src/ConstantFolder.cpp | 4 +--- docs/dev/debt/shader.md | 2 +- 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp b/core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp index 6ccfb4be3..4290c4a85 100644 --- a/core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp @@ -1809,15 +1809,24 @@ void verifyPixelRequestSerialization() { minimal.context.pixel = ShaderPixelStageInfo{}; minimal.context.pixel->rasterPrimitive = RasterPrimitive::Rectangle; const auto encoded = serializer.Serialize(minimal); - require(requestPrefix(encoded, 8u) == "NVNQQRIAAAA=", "new requests did not use serialization version 18"); + require(requestPrefix(encoded, 8u) == "NVNQQRMAAAA=", "new requests did not use serialization version 19"); require(serializer.Deserialize(encoded).request.context.pixel->rasterPrimitive == RasterPrimitive::Rectangle, "the pixel primitive class did not round-trip"); constexpr std::size_t mappingOffset = 8u + 37u + 18u + 163u; for (std::size_t bytes = 0; bytes < 20u; ++bytes) { expectFailure([&] { static_cast(serializer.Deserialize(requestPrefix(encoded, mappingOffset + bytes))); }, "truncated data", "a truncated pixel mapping, packing, dual-source, alpha-to-coverage, stencil reference export flag or primitive class was accepted"); } - for (const auto unsupported : {"NVNQQQAAAAA=", "NVNQQRMAAAA="}) { + for (const auto unsupported : {"NVNQQQAAAAA=", "NVNQQRQAAAA="}) { expectFailure([&] { static_cast(serializer.Deserialize(unsupported)); }, "serialization version", "an unsupported request version was accepted"); } + static constexpr std::string_view version18Request{ + "NVNQQRIAAAAFAAADAAAAAAABAAAAAAAAAAAAgb8AAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAA" + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAB0gQAAAE="}; + const auto version18 = serializer.Deserialize(version18Request); + require(version18.request.context.pixel.has_value() && version18.request.context.pixel->rasterPrimitive == RasterPrimitive::Triangle, "a version 18 request read a primitive class"); + require(version18.request.target.registrationModules && version18.request.target.registrationWordLimit == 1234u && version18.request.target.vertexSubgroups, "a version 18 request trailer was misread"); } void verifyLegacyPixelRequests() { diff --git a/core/libs/tests/PixelAncillary.cpp b/core/libs/tests/PixelAncillary.cpp index d1529fbc2..491526ba6 100644 --- a/core/libs/tests/PixelAncillary.cpp +++ b/core/libs/tests/PixelAncillary.cpp @@ -432,14 +432,17 @@ static void HelperLoopRefused(LoopRead read) { } Require(false); } -static bool AncillaryRemains(const IrProgram& program) { +static bool BuiltinRemains(const IrProgram& program, StageInputKind kind) { for (const IrBlock* block : program.BlockOrder()) { for (const IrValue* inst : block->Instructions()) { - if (inst->Opcode() == IrOpcode::GetBuiltin && static_cast(inst->Argument(0)->Resolve()->ImmediateU32()) == StageInputKind::PackedAncillary) return true; + if (inst->Opcode() == IrOpcode::GetBuiltin && static_cast(inst->Argument(0)->Resolve()->ImmediateU32()) == kind) return true; } } return false; } +static bool AncillaryRemains(const IrProgram& program) { + return BuiltinRemains(program, StageInputKind::PackedAncillary); +} static void ExportedWholeWord() { IrProgram program; auto& entry = Begin(program); @@ -473,10 +476,11 @@ static void ExportedWholeWord() { static_cast(builder.Emit(IrOpcode::Return, IrType::Void, {})); LowerMasked(program, true); Require(!AncillaryRemains(program) && program.Info().rasterPrimitive); + Require(!BuiltinRemains(program, StageInputKind::Layer)); const IrValue* word = carried.Argument(0)->Resolve(); constexpr std::array words {{{0u, 0u, false, RasterPrimitive::Triangle}, {0u, 0u, false, RasterPrimitive::Rectangle}, {0u, 0u, false, RasterPrimitive::Line}, {15u, 0x1FFFu, false, RasterPrimitive::Point}, {3u, 0x7FFu, false, RasterPrimitive::Triangle}}}; for (const Row& row : words) { - Require(Evaluate(word, row) == (static_cast(row.primitive) | (row.sample << 8u) | (row.layer << 16u))); + Require(Evaluate(word, row) == (static_cast(row.primitive) | (row.sample << 8u))); } } static void WholeWordReplacesRefusal(HelperRead read) { @@ -486,7 +490,7 @@ static void WholeWordReplacesRefusal(HelperRead read) { Require(!AncillaryRemains(program) && program.Info().rasterPrimitive); Require(built.written->Parent() != nullptr); const Row row {9u, 0x123u, false, RasterPrimitive::Rectangle}; - Require(Evaluate(built.written->Argument(2), row) == (3u | (9u << 8u) | (0x123u << 16u))); + Require(Evaluate(built.written->Argument(2), row) == (3u | (9u << 8u))); } enum class StartingQuad { Kept, BallotAfterBranch, BallotOfOtherLanes, ComputeStage }; diff --git a/core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp b/core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp index 821df18ff..5202b9d20 100644 --- a/core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp +++ b/core/shader/recompiler/ControlFlow/src/RequestSerializer.cpp @@ -384,7 +384,7 @@ ShaderPixelStageInfo readPixelInfo(Reader& reader, std::uint32_t version) { if (version >= 15u) info.dualSourceBlend = reader.ReadBool(); if (version >= 16u) info.alphaToCoverage = reader.ReadBool(); if (version >= 17u) info.stencilRefExportEnable = reader.ReadBool(); - if (version >= 18u) info.rasterPrimitive = readRasterPrimitive(reader); + if (version >= 19u) info.rasterPrimitive = readRasterPrimitive(reader); if (version < 5u) { const auto input = [](PixelInput value, bool present) { return present ? PixelInputBit(value) : 0u; }; info.inputAddr = input(PixelInput::PerspectiveSample, info.hasPerspectiveCenterVgpr && inputAddrOrCenterVgpr == 2u) | input(PixelInput::PerspectiveCenter, info.hasPerspectiveCenterVgpr) | @@ -746,7 +746,7 @@ std::string RequestSerializer::Serialize(const RecompileRequest& request) const std::string buffer; Writer writer(buffer); writer.WriteU32(0x41505335u); - writer.WriteU32(18u); + writer.WriteU32(19u); writeShaderBinary(writer, request.shader); writeGuestContext(writer, request.context); writeSpirvTarget(writer, request.target); @@ -777,7 +777,7 @@ DeserializedRequest RequestSerializer::Deserialize(std::string_view text) const Reader reader(decoded); if (reader.ReadU32() != 0x41505335u) throw std::runtime_error("invalid recompile request signature"); const auto version = reader.ReadU32(); - if (version < 1u || version > 18u) throw std::runtime_error("unsupported recompile request serialization version"); + if (version < 1u || version > 19u) throw std::runtime_error("unsupported recompile request serialization version"); DeserializedRequest result{}; result.request.shader = readShaderBinary(reader, result.shaderCode, result.shaderHeader); result.request.context = readGuestContext(reader, result, version); diff --git a/core/shader/recompiler/Optimization/src/ConstantFolder.cpp b/core/shader/recompiler/Optimization/src/ConstantFolder.cpp index 61679e1f5..079df35fb 100644 --- a/core/shader/recompiler/Optimization/src/ConstantFolder.cpp +++ b/core/shader/recompiler/Optimization/src/ConstantFolder.cpp @@ -866,9 +866,7 @@ bool ConstantFolder::ComposePackedAncillary(IrProgram& program) const { for (IrValue* word : words) { IrValue& primitive = insertBuiltin(program, builder, *word, StageInputKind::RasterPrimitive); IrValue& sample = insertBefore(program, *word, IrOpcode::ShiftLeftLogical32, insertBuiltin(program, builder, *word, StageInputKind::SampleId), builder.Constant(8u)); - IrValue& layer = insertBefore(program, *word, IrOpcode::ShiftLeftLogical32, insertBuiltin(program, builder, *word, StageInputKind::Layer), builder.Constant(16u)); - IrValue& low = insertBefore(program, *word, IrOpcode::BitwiseOr32, primitive, sample); - word->ReplaceAllUsesWith(&insertBefore(program, *word, IrOpcode::BitwiseOr32, low, layer)); + word->ReplaceAllUsesWith(&insertBefore(program, *word, IrOpcode::BitwiseOr32, primitive, sample)); } return !words.empty(); } diff --git a/docs/dev/debt/shader.md b/docs/dev/debt/shader.md index ce1f09951..c424adcaf 100644 --- a/docs/dev/debt/shader.md +++ b/docs/dev/debt/shader.md @@ -2,7 +2,7 @@ ### Unknown function info -- [Packed pixel ancillary word](../../../core/shader/recompiler/Optimization/src/ConstantFolder.cpp) (shader recompiler) - the sample index at bits 8-11 and the render target index at bits 16-28 follow #295, and their equality with the SampleId and Layer builtins is inferred, not measured. Mesa reads both fields from those bits, with the layer 14 bits wide only from GFX12 on ([ac_nir_lower_intrinsics_to_args.c](https://gitlab.freedesktop.org/mesa/mesa/-/blob/bffd91947efc1932da99aca69fc540d6d4201b0c/src/amd/common/nir/ac_nir_lower_intrinsics_to_args.c#L342-L368)). A value passed through a select, move or phi whose every read is an extract inside one of those fields is rebuilt from the two fields alone. Any other read the masked-select pass cannot drop gets the whole word: the primitive class at bits 0-1 (0 point, 1 line, 2 triangle, 3 rectangle), which the draw sets as a pipeline specialization constant ([RasterPrimitiveOf](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp)) in the shaders that read it, plus the sample and layer fields, with every other bit zero. That layout comes from the RDNA 3 and 3.5 ISA guides (prim type 1:0, VRS rates 3:2 and 5:4, eye id 7, sample 11:8, render target index 28:16). The GCN register guides give the same prim type and sample fields and put the render target index at 26:16, so the two sources agree for layers below 2048. AMD's LLPC reads the primitive class from bits 1:0 with the same encoding ([LowerGlCompatibility.cpp](https://github.com/GPUOpen-Drivers/llpc/blob/40cb8d95ad8d6f7f1652e3fd47d39667594cce08/llpc/lowering/LowerGlCompatibility.cpp#L830-L836)) and without a GFX-level check ([getPrimType](https://github.com/GPUOpen-Drivers/llpc/blob/40cb8d95ad8d6f7f1652e3fd47d39667594cce08/lgc/lowering/LowerInOut.cpp#L5307-L5315)). Nobody has checked it on gfx1013. The VRS rates and the eye id are zero because the driver supports neither variable-rate shading nor multiview; a title that enables either would read other bits on the console. The sample field reads SampleId, which is always 0 here because the driver rejects MSAA and per-sample shading (`PA_SC_AA_CONFIG`, `PS_ITER_SAMPLE`), and the layer field reads Layer, which is 0 while the driver ignores vertex layer exports +- [Packed pixel ancillary word](../../../core/shader/recompiler/Optimization/src/ConstantFolder.cpp) (shader recompiler) - the sample index at bits 8-11 and the render target index at bits 16-28 follow #295, and their equality with the SampleId and Layer builtins is inferred, not measured. Mesa reads both fields from those bits, with the layer 14 bits wide only from GFX12 on ([ac_nir_lower_intrinsics_to_args.c](https://gitlab.freedesktop.org/mesa/mesa/-/blob/bffd91947efc1932da99aca69fc540d6d4201b0c/src/amd/common/nir/ac_nir_lower_intrinsics_to_args.c#L342-L368)). A value passed through a select, move or phi whose every read is an extract inside one of those fields is rebuilt from the two fields alone. Any other read the masked-select pass cannot drop gets the whole word: the primitive class at bits 0-1 (0 point, 1 line, 2 triangle, 3 rectangle), which the draw sets as a pipeline specialization constant ([RasterPrimitiveOf](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp)) in the shaders that read it, plus the sample field, with every other bit zero. That layout comes from the RDNA 3 and 3.5 ISA guides (prim type 1:0, VRS rates 3:2 and 5:4, eye id 7, sample 11:8, render target index 28:16). The GCN register guides give the same prim type and sample fields and put the render target index at 26:16, so the two sources agree for layers below 2048. AMD's LLPC reads the primitive class from bits 1:0 with the same encoding ([LowerGlCompatibility.cpp](https://github.com/GPUOpen-Drivers/llpc/blob/40cb8d95ad8d6f7f1652e3fd47d39667594cce08/llpc/lowering/LowerGlCompatibility.cpp#L830-L836)) and without a GFX-level check ([getPrimType](https://github.com/GPUOpen-Drivers/llpc/blob/40cb8d95ad8d6f7f1652e3fd47d39667594cce08/lgc/lowering/LowerInOut.cpp#L5307-L5315)). Nobody has checked it on gfx1013. In the whole word only bits 0-1 and 8-11 have a source. Bits 2-7, 12-15 and 29-31 are assumed zero, not measured: the VRS rates (2-5) and the eye id (7) because the driver supports neither variable-rate shading nor multiview, so a title that enables either would read other bits on the console, and bits 6, 12-15 and 29-31 because no source gives them a meaning. The sample field reads SampleId, which is always 0 here because the driver rejects MSAA and per-sample shading (`PA_SC_AA_CONFIG`, `PS_ITER_SAMPLE`). The render target index (bits 16-28) of the whole word is the constant 0 and not the Layer builtin: draws render to a single layer and guest layer exports are ignored, so no stage writes Layer, and a fragment shader that reads the unwritten builtin loses its writes on AMD's Windows driver (#4800). A change that makes draws export a layer must restore the read - [Color comparison descriptor forms](../../../core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp): emulated color comparison supports UNORM, SNORM and float formats for 2D, 2D-array and cube views. 8-bit sRGB and UNORM (`Format8Srgb`, `Format8_8Srgb`, `Format8_8_8_8Srgb`, `Format8UNorm`, `Format8_8UNorm`, `Format8_8_8_8UNorm`) return 0 for every compare function, measured with `image_sample_c_lz` on gfx1035 over references -0.5..1.5 and texel codes 0..255. Anisotropic filters are refused, except on those 8-bit formats, where every texel comparison is 0 (not measured with an anisotropic S#). Bilinear cube comparisons of float, snorm and the other unorm formats without `DISABLE_CUBE_WRAP` are refused; on that hardware a 4x4 R32 float cube blends across faces (46 of 90 edge rows differ from a per-face clamp; face 0, ref 0.1, u=3.9 v=2.5 is 0.6016 and u=1.5 v=0.1 is 0.3984), and with `DISABLE_CUBE_WRAP` the per-face clamp matches. An 8-bit sRGB or UNORM bilinear cube without that bit returns 0, since every texel comparison is already 0 for every compare function and the selected face cannot change the result. Cube-edge sampling behavior and console coverage on PS5's gfx1013 remain unverified. - [s_setkill](../../../core/shader/recompiler/RdnaDecoder/src/RdnaScalarOpDecoder.cpp) (core/shader/recompiler) - decoded as `s_endpgm` for any immediate: measured on RDNA2 hardware (gfx1035), a store before it is visible and nothing after it runs ([#771](https://github.com/boykopovar/AnyPS5/pull/771)); whether the KILL status bit it sets is observable differently from `s_endpgm` is unverified - [s_sethalt](../../../core/shader/recompiler/Translation/src/ScalarInstructions.cpp) (core/shader/recompiler) - with SIMM16[0] clear it is a no-op: the RDNA 2 ISA guide describes that value as resuming a halted wave, and a wave that executes it is not halted; measured on a gfx1035 (RDNA 2) wave: `s_sethalt 0` followed by a VALU op completes and gives the normal result. Only SIMM16[0] is read; with it set the instruction throws