fix(recompiler): throw for descriptor web leaves that are not SRT descriptors

A path whose T# or S# dwords hold a program-counter, EXEC or compare-mask
value is no longer folded onto the first descriptor: the web split gives up
and the read throws as on main. Cache the invariant-Phi resolution per web,
lower the state limit to 512 and test the program-counter, EXEC and
compare-mask rows, unwritten registers, and the arm and state limits.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp

@@ -511,18 +511,62 @@ void verifyDescriptorPhis() {
     expectFailure([&] { static_cast<void>(GetResourcePlan(dynamic)); }, "GetSamplerResource dword 0 is not a valid runtime value", "descriptor Phi: an edge without an SRT slot was accepted");
 
     const std::vector<std::uint32_t> loopCode{0xf4080500u, 0xfa000020u, 0xf4080600u, 0xfa000040u, 0xf40c0200u, 0xfa000000u, 0xbe910380u, 0xbf8cc07fu, 0xf09c0f08u, 0x00a20000u, 0xbf8c3f70u, 0xe0700000u, 0x80060000u, 0xbf068011u, 0xbf850003u, 0xf40c0200u, 0xfa000000u, 0xbf820002u, 0xf40c0200u, 0xfa000060u, 0xbf8cc07fu, 0x80118111u, 0xbf0a8211u, 0xbf85fff0u, 0xbf810000u};
-    auto programCounterCode = loopCode;
-    programCounterCode.insert(programCounterCode.begin() + 8, 0xbe8e1f00u);
-    for (const auto* code : std::array<const std::vector<std::uint32_t>*, 2>{&loopCode, &programCounterCode}) {
-        const auto loopCapture = compile(*code, 2u, 1u);
-        const auto& loopImages = loopCapture->snapshot.images;
-        const auto holdsLoopImage = [&](const std::array<std::uint32_t, 8>& words) {
-            return std::ranges::any_of(loopImages, [&](const DescriptorValue& value) {
-                return std::equal(words.begin(), words.end(), value.dwords.begin());
-            });
-        };
-        require(loopImages.size() == 2u && holdsLoopImage(first) && holdsLoopImage(second), "descriptor Phi: the loop's chained T# Phis were not split into the two SRT T#s");
+    const auto loopCapture = compile(loopCode, 2u, 1u);
+    const auto& loopImages = loopCapture->snapshot.images;
+    const auto holdsLoopImage = [&](const std::array<std::uint32_t, 8>& words) {
+        return std::ranges::any_of(loopImages, [&](const DescriptorValue& value) {
+            return std::equal(words.begin(), words.end(), value.dwords.begin());
+        });
+    };
+    require(loopImages.size() == 2u && holdsLoopImage(first) && holdsLoopImage(second), "descriptor Phi: the loop's chained T# Phis were not split into the two SRT T#s");
+
+    const auto entryWrites = [&](std::initializer_list<std::uint32_t> words) {
+        auto code = loopCode;
+        code.insert(code.begin() + 8, words);
+        return code;
+    };
+    const std::array<std::pair<std::vector<std::uint32_t>, const char*>, 3> invalidEntries{{
+        {entryWrites({0xbe8e1f00u}), "descriptor Phi: a T# holding the program counter was accepted"},
+        {entryWrites({0xbe8e037eu}), "descriptor Phi: a T# holding EXEC was accepted"},
+        {entryWrites({0x7d840080u, 0xbe8e036au}), "descriptor Phi: a T# holding a compare mask was accepted"},
+    }};
+    for (const auto& [code, message] : invalidEntries) {
+        auto request = makeRequest(code);
+        expectFailure([&] { static_cast<void>(GetResourcePlan(request)); }, "GetImageResource dword 0 is not a valid runtime value", message);
     }
+    auto unwritten = loopCode;
+    unwritten[4] = 0xf4080200u;
+    auto unwrittenRequest = makeRequest(unwritten);
+    require(GetResourcePlan(unwrittenRequest)->info.images.size() == 3u, "descriptor Phi: a T# whose unwritten registers read as 0 was not planned as its own image");
+
+    const std::array<std::uint32_t, 7> prologue{0xf4080500u, 0xfa000020u, 0xf4080600u, 0xfa000040u, 0xf40c0200u, 0xfa000000u, 0xbf8cc07fu};
+    const std::array<std::uint32_t, 7> epilogue{0xbf8cc07fu, 0xf09c0f08u, 0x00a20000u, 0xbf8c3f70u, 0xe0700000u, 0x80060000u, 0xbf810000u};
+    const auto planImages = [&](const std::vector<std::uint32_t>& body) {
+        std::vector<std::uint32_t> code(prologue.begin(), prologue.end());
+        code.insert(code.end(), body.begin(), body.end());
+        code.insert(code.end(), epilogue.begin(), epilogue.end());
+        auto request = makeRequest(code);
+        return GetResourcePlan(request)->info.images.size();
+    };
+    const auto distinctLoads = [](std::uint32_t loads) {
+        std::vector<std::uint32_t> body;
+        for (std::uint32_t load = 0; load < loads; load++) {
+            body.insert(body.end(), {0xbf068014u, 0xbf850002u, 0xf40c0200u, 0xfa000060u + load * 0x20u});
+        }
+        return body;
+    };
+    require(planImages(distinctLoads(63u)) == 64u, "descriptor Phi: 64 distinct T#s were not given one image each");
+    expectFailure([&] { static_cast<void>(planImages(distinctLoads(64u))); }, "GetImageResource dword 0 is not a valid runtime value", "descriptor Phi: more than 64 distinct T#s were accepted");
+
+    const auto splitReloads = [](std::uint32_t joins) {
+        std::vector<std::uint32_t> body;
+        for (std::uint32_t join = 0; join < joins; join++) {
+            body.insert(body.end(), {0xbf068014u, 0xbf850003u, 0xf40003c0u, join % 2u == 0u ? 0xfa000088u : 0xfa00008cu, 0xbf820006u, 0xf4080200u, 0xfa000060u, 0xf4040300u, 0xfa000070u, 0xf4000380u, 0xfa000078u});
+        }
+        return body;
+    };
+    require(planImages(splitReloads(8u)) == 4u, "descriptor Phi: separate reloads of T# dword 7 and dwords 0-6 were not split into their 4 T#s");
+    expectFailure([&] { static_cast<void>(planImages(splitReloads(260u))); }, "GetImageResource dword 0 is not a valid runtime value", "descriptor Phi: a web over more than 512 descriptor tuples was accepted");
 }
 
 void verifyProgramCounterRelativeData() {

core/shader/recompiler/Optimization/src/ResourceTracker.cpp

@@ -13,6 +13,7 @@
 #include <map>
 #include <span>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -741,25 +742,16 @@ private:
         inst.Parent()->RemoveInstruction(&inst);
     }
 
-    static bool CarriesControlValue(IrValue* value, std::uint32_t depth) {
+    static bool HoldsProgramCounter(IrValue* value, std::uint32_t depth) {
         value = value->Resolve();
-        if (value->HasImmediate() || value->IsPhi()) {
-            return false;
+        if (value->Opcode() == IrOpcode::GetShaderBase) {
+            return true;
         }
-        switch (value->Opcode()) {
-            case IrOpcode::GetShaderBase:
-            case IrOpcode::GetExec:
-            case IrOpcode::GetExecLo:
-            case IrOpcode::GetExecHi:
-            case IrOpcode::Ballot:
-            case IrOpcode::UndefU32: return true;
-            default: break;
-        }
-        if (depth == 0u || (!Detail::IsRuntimeUniformOp(value->Opcode()) && value->Opcode() != IrOpcode::CompositeExtractU32x4)) {
+        if (value->HasImmediate() || depth == 0u || !Detail::IsRuntimeUniformOp(value->Opcode())) {
             return false;
         }
         for (std::size_t index = 0; index < value->ArgumentCount(); index++) {
-            if (CarriesControlValue(value->Argument(index), depth - 1u)) {
+            if (HoldsProgramCounter(value->Argument(index), depth - 1u)) {
                 return true;
             }
         }
@@ -767,34 +759,34 @@ private:
     }
 
     struct DescriptorWeb {
-        static constexpr std::size_t maxStates = 4096u;
+        static constexpr std::size_t maxStates = 512u;
         static constexpr std::size_t maxArms = 64u;
         std::vector<std::vector<IrValue*>> arms;
         std::map<std::vector<IrValue*>, IrValue*> selectors;
         std::vector<std::pair<IrBlock*, IrValue*>> phis;
+        std::unordered_map<IrValue*, IrValue*> resolved;
     };
 
     IrValue* WebSelector(std::vector<IrValue*> tuple, DescriptorWeb& web, IrValue& position) {
         for (IrValue*& value : tuple) {
             value = value->Resolve();
-            while (value->IsPhi()) {
-                IrValue* invariant = ResolveInvariantPhi(m_program.Resources(), value);
-                if (invariant == nullptr || invariant == value) {
-                    break;
-                }
-                value = invariant->Resolve();
+            if (!value->IsPhi()) {
+                continue;
             }
+            const auto [entry, inserted] = web.resolved.try_emplace(value, value);
+            if (inserted) {
+                IrValue* invariant = ResolveInvariantPhi(m_program.Resources(), value);
+                entry->second = invariant == nullptr ? value : invariant->Resolve();
+            }
+            value = entry->second;
         }
         if (const auto known = web.selectors.find(tuple); known != web.selectors.end()) {
             return known->second;
         }
         const auto phi = std::ranges::find_if(tuple, [](const IrValue* value) { return value->IsPhi(); });
         if (phi == tuple.end()) {
-            if (std::ranges::any_of(tuple, [](IrValue* value) { return CarriesControlValue(value, phiSearchDepth); })) {
-                return &m_builder.Constant(0u);
-            }
             for (IrValue* value : tuple) {
-                if (Rematerialize(value, position, false, phiSearchDepth) == nullptr) {
+                if (HoldsProgramCounter(value, phiSearchDepth) || Rematerialize(value, position, false, phiSearchDepth) == nullptr) {
                     return nullptr;
                 }
             }

docs/dev/TechnicalDebt.md

@@ -233,7 +233,6 @@ Throughout the project, every function at every stage either **does exactly what
 - The shader recompiler [ignores `s_setreg_b32`](../../core/shader/recompiler/Translation/src/ScalarInstructions.cpp): writes to MODE (float rounding and denormal controls) and the other hardware registers have no effect, and `s_getreg_b32` reads of the MODE round fields report round to nearest even even after `s_setreg_b32` wrote another mode. The initial round mode from the shader's program registers is not read either.
 - On a host subgroup narrower than 32 lanes, [`threadBit`](../../core/shader/recompiler/Translation/src/TranslationContext/ExecMask.cpp) reads an EXEC written as a value (`s_mov_b32`/`s_mov_b64`) at bit (subgroup invocation id) of EXEC_LO, so lanes at or above the subgroup size read a lower lane's bit.
 - [`v_readlane_b32`](../../core/shader/recompiler/SpirvBackend/src/SpirvAlu/SpirvAluEmitterMath.cpp) and fetch-inactive `v_permlane16_b32`/`v_permlanex16_b32` read undefined data from a lane that the host subgroup does not hold or whose host invocation is inactive; the console reads the lane's VGPR.
-- [Image reads through a web of descriptor Phis](../../core/shader/recompiler/Optimization/src/ResourceTracker.cpp) are split into one read per distinct SRT descriptor. A path whose T# or S# dwords hold a program-counter, EXEC or ballot value, or an undefined register, is folded onto the first descriptor: no real descriptor holds such a value, so the path is assumed not to reach the read. If a title does read through it, the hardware would fetch from a garbage descriptor, and the read here returns the first descriptor's result instead.
 - Without fetch-inactive, [`v_permlane16_b32` and `v_permlanex16_b32`](../../core/shader/recompiler/SpirvBackend/src/SpirvAlu/SpirvAluEmitterMath.cpp) take a source lane that the host subgroup does not hold as inactive and keep the old value (0 with bound_ctrl); the console reads that lane's VGPR.
 - [Shader recompilation](../../core/shader/recompiler/Recompiler.cpp) currently occurs right before it was transferred to Vulkan with caching, but should be moved to the [relinker](../../core/relinker/main.cpp) stage. For this purpose, [shader/recompiler](../../core/shader/recompiler) was written completely independently from [libs/prx](../../core/libs/prx).
 - [v_fma_f64](../../core/shader/recompiler/SpirvBackend/src/SpirvAlu/SpirvAluEmitterFloat64.cpp) is fused exactly only for normal inputs: with a subnormal input it is a separate multiply and add, which rounds twice.