perf(shader): avoid redundancy traversal map copies

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

@@ -378,6 +378,77 @@ OpFunctionEnd
     }
 }
 
+void testLongControlFlow(bool optimizationEnabled, bool large) {
+    const char* mode = std::getenv("APS5_SPIRV_OPT");
+    const bool full = mode != nullptr && std::string(mode) == "full";
+    const std::size_t blocks = large && optimizationEnabled && !full ? 512 : 8;
+    std::string source = R"(OpCapability Shader
+OpCapability StorageBuffer8BitAccess
+OpExtension "SPV_KHR_8bit_storage"
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %array ArrayStride 4
+OpMemberDecorate %buffer 0 Offset 0
+OpDecorate %buffer Block
+OpDecorate %data DescriptorSet 0
+OpDecorate %data Binding 0
+OpDecorate %byteArray ArrayStride 1
+OpMemberDecorate %byteBuffer 0 Offset 0
+OpDecorate %byteBuffer Block
+OpDecorate %bytes DescriptorSet 0
+OpDecorate %bytes Binding 1
+%void = OpTypeVoid
+%uint = OpTypeInt 32 0
+%boolean = OpTypeBool
+%byte = OpTypeInt 8 0
+%byteArray = OpTypeRuntimeArray %byte
+%byteBuffer = OpTypeStruct %byteArray
+%byteBufferPointer = OpTypePointer StorageBuffer %byteBuffer
+%bytePointer = OpTypePointer StorageBuffer %byte
+%array = OpTypeRuntimeArray %uint
+%buffer = OpTypeStruct %array
+%bufferPointer = OpTypePointer StorageBuffer %buffer
+%valuePointer = OpTypePointer StorageBuffer %uint
+%function = OpTypeFunction %void
+%data = OpVariable %bufferPointer StorageBuffer
+%bytes = OpVariable %byteBufferPointer StorageBuffer
+%zero = OpConstant %uint 0
+%one = OpConstant %uint 1
+%two = OpConstant %uint 2
+)";
+    for (std::size_t i = 0; i < blocks * 4; ++i) source += "%index" + std::to_string(i) + " = OpConstant %uint " + std::to_string(i + 1) + "\n";
+    source += "%main = OpFunction %void None %function\n%entry = OpLabel\n%input = OpAccessChain %valuePointer %data %zero %zero\n%seed = OpLoad %uint %input Volatile\n%condition = OpINotEqual %boolean %seed %zero\nOpBranch %block0\n";
+    for (std::size_t i = 0; i < blocks; ++i) {
+        const auto number = std::to_string(i);
+        const auto previous = i == 0 ? "%seed" : "%next" + std::to_string(i - 1);
+        source += "%block" + number + " = OpLabel\n%sum" + number + " = OpIAdd %uint " + previous + " %one\n";
+        source += "%duplicate" + number + " = OpIAdd %uint " + previous + " %one\n%shared" + number + " = OpIAdd %uint %seed %one\n";
+        for (std::size_t j = 0; j < 3; ++j) {
+            const auto index = std::to_string(i * 4 + j);
+            source += "%slot" + index + " = OpAccessChain %valuePointer %data %zero %index" + index + "\n";
+            source += "OpStore %slot" + index + " %" + (j == 0 ? "sum" : j == 1 ? "duplicate" : "shared") + number + " Volatile\n";
+        }
+        const auto branchIndex = std::to_string(i * 4 + 3);
+        source += "%branchSlot" + number + " = OpAccessChain %valuePointer %data %zero %index" + branchIndex + "\n";
+        source += "OpSelectionMerge %join" + number + " None\nOpBranchConditional %condition %left" + number + " %right" + number + "\n";
+        source += "%left" + number + " = OpLabel\n%leftValue" + number + " = OpIAdd %uint %sum" + number + " %one\nOpStore %branchSlot" + number + " %leftValue" + number + " Volatile\nOpBranch %join" + number + "\n";
+        source += "%right" + number + " = OpLabel\n%rightCopy" + number + " = OpIAdd %uint %sum" + number + " %one\nOpStore %branchSlot" + number + " %rightCopy" + number + " Volatile\n";
+        source += "%rightValue" + number + " = OpIAdd %uint %sum" + number + " %two\nOpStore %branchSlot" + number + " %rightValue" + number + " Volatile\nOpBranch %join" + number + "\n";
+        source += "%join" + number + " = OpLabel\n%next" + number + " = OpPhi %uint %leftValue" + number + " %left" + number + " %rightValue" + number + " %right" + number + "\n";
+        if (i + 1 == blocks) source += "%byteSlot = OpAccessChain %bytePointer %bytes %zero %zero\n%truncated = OpUConvert %byte %next" + number + "\nOpStore %byteSlot %truncated Volatile\nOpReturn\n";
+        else source += "OpBranch %block" + std::to_string(i + 1) + "\n";
+    }
+    source += "OpFunctionEnd\n";
+    const auto input = assemble(source);
+    const auto output = ShaderRecompiler::ValidateAndOptimizeSpirv(input, Vulkan11, Spirv13, false);
+    const auto stores = opcodeCount(output, spv::OpStore);
+    check(stores >= blocks * 4 + 1 && stores <= blocks * 6 + 1, "long control-flow optimization discarded observable stores");
+    const auto adds = opcodeCount(output, spv::OpIAdd);
+    if (!optimizationEnabled) check(output == input, "disabled optimization changed the long control-flow module");
+    else check(adds >= blocks && adds <= blocks * 4, "control-flow optimization lost arithmetic or retained redundant values");
+}
+
 }
 
 int main() {
@@ -388,6 +459,8 @@ int main() {
     testInvalidInputIsRejected();
     testSpecializedSelectionExit();
     testSpecializedDeadContinue();
+    testLongControlFlow(optimizationEnabled, false);
+    testLongControlFlow(optimizationEnabled, true);
     if (failures != 0) {
         std::fprintf(stderr, "%d optimizer check(s) failed\n", failures);
         return 1;

core/shader/recompiler/PatchSpirvTools.cmake

@@ -3,31 +3,101 @@ set(spirvToolsRedundancySource "${spirvToolsSourceDir}/redundancy_elimination.cp
 set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${spirvToolsRedundancySource}")
 file(READ "${spirvToolsRedundancySource}" spirvToolsRedundancy)
 
+set(spirvToolsLocalRedundancySource "${spirvToolsSourceDir}/local_redundancy_elimination.cpp")
+set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${spirvToolsLocalRedundancySource}")
+file(READ "${spirvToolsLocalRedundancySource}" spirvToolsLocalRedundancy)
+set(spirvToolsExpectedElimination [=[
+  auto func = [this, &vnTable, &modified, value_to_ids](Instruction* inst) {
+    if (inst->result_id() == 0) {
+      return;
+    }
+
+    uint32_t value = vnTable.GetValueNumber(inst);
+
+    if (value == 0) {
+      return;
+    }
+
+    auto candidate = value_to_ids->insert({value, inst->result_id()});
+    if (!candidate.second) {
+      context()->KillNamesAndDecorates(inst);
+      context()->ReplaceAllUsesWith(inst->result_id(), candidate.first->second);
+      context()->KillInst(inst);
+      modified = true;
+    }
+  };
+  block->ForEachInst(func);
+]=])
+string(FIND "${spirvToolsLocalRedundancy}" "${spirvToolsExpectedElimination}" spirvToolsEliminationPosition)
+if(spirvToolsEliminationPosition EQUAL -1)
+    message(FATAL_ERROR "SPIRV-Tools value elimination has changed; review the local traversal fix")
+endif()
+
 set(spirvToolsOldTraversal [=[
+  struct State {
+    DominatorTreeNode* node;
+    std::map<uint32_t, uint32_t> value_to_id_map;
+  };
+  std::vector<State> todo;
+  todo.push_back({bb, std::map<uint32_t, uint32_t>()});
+  bool modified = false;
+  for (size_t next_node = 0; next_node < todo.size(); next_node++) {
+    modified |= EliminateRedundanciesInBB(todo[next_node].node->bb_, vnTable,
+                                          &todo[next_node].value_to_id_map);
     for (DominatorTreeNode* child : todo[next_node].node->children_) {
       todo.push_back({child, todo[next_node].value_to_id_map});
     }
+  }
 ]=])
 set(spirvToolsNewTraversal [=[
-    const auto& children = todo[next_node].node->children_;
-    for (size_t child_index = 0; child_index < children.size(); ++child_index) {
-      if (child_index + 1 == children.size()) {
-        todo.push_back({children[child_index],
-                        std::move(todo[next_node].value_to_id_map)});
-      } else {
-        todo.push_back({children[child_index], todo[next_node].value_to_id_map});
-      }
+  struct State {
+    DominatorTreeNode* node;
+    size_t next_child;
+    size_t added_begin;
+    bool entered;
+  };
+  std::map<uint32_t, uint32_t> value_to_id_map;
+  std::vector<uint32_t> added_values;
+  std::vector<State> todo;
+  todo.push_back({bb, 0, 0, false});
+  bool modified = false;
+  while (!todo.empty()) {
+    auto& state = todo.back();
+    if (!state.entered) {
+      state.entered = true;
+      state.node->bb_->ForEachInst([&](Instruction* inst) {
+        if (inst->result_id() == 0) return;
+        const uint32_t value = vnTable.GetValueNumber(inst);
+        if (value == 0) return;
+        auto candidate = value_to_id_map.insert({value, inst->result_id()});
+        if (candidate.second) {
+          added_values.push_back(value);
+        } else {
+          context()->KillNamesAndDecorates(inst);
+          context()->ReplaceAllUsesWith(inst->result_id(), candidate.first->second);
+          context()->KillInst(inst);
+          modified = true;
+        }
+      });
     }
-    todo[next_node].value_to_id_map.clear();
+    const auto& children = state.node->children_;
+    if (state.next_child != children.size()) {
+      auto* child = children[state.next_child++];
+      todo.push_back({child, 0, added_values.size(), false});
+      continue;
+    }
+    while (added_values.size() != state.added_begin) {
+      value_to_id_map.erase(added_values.back());
+      added_values.pop_back();
+    }
+    todo.pop_back();
+  }
 ]=])
 string(FIND "${spirvToolsRedundancy}" "${spirvToolsOldTraversal}" spirvToolsTraversalPosition)
 if(spirvToolsTraversalPosition EQUAL -1)
     message(FATAL_ERROR "SPIRV-Tools redundancy traversal has changed; review the local memory fix")
 endif()
 string(REPLACE "${spirvToolsOldTraversal}" "${spirvToolsNewTraversal}" spirvToolsRedundancy "${spirvToolsRedundancy}")
-string(REPLACE "#include \"source/opt/redundancy_elimination.h\"\n"
-        "#include \"source/opt/redundancy_elimination.h\"\n\n#include <utility>\n"
-        spirvToolsRedundancy "${spirvToolsRedundancy}")
 
 set(spirvToolsPatchedDir "${CMAKE_BINARY_DIR}/3rdparty/SPIRV-Tools/patched")
 set(spirvToolsPatchedSource "${spirvToolsPatchedDir}/redundancy_elimination.cpp")

docs/dev/TechnicalDebt.md

@@ -4,7 +4,7 @@ Entries about one library or component are in [debt](debt/), one file per librar
 
 ### Build
 
-- The bundled [SPIRV-Tools build](../../core/shader/recompiler/PatchSpirvTools.cmake) replaces the redundancy-elimination source with a build-directory copy that releases processed traversal maps. Remove this patch when the pinned revision includes [SPIRV-Tools #6962](https://github.com/KhronosGroup/SPIRV-Tools/pull/6962). System-dependency builds need the fix in their installed SPIRV-Tools package.
+- The bundled [SPIRV-Tools build](../../core/shader/recompiler/PatchSpirvTools.cmake) replaces the redundancy-elimination source with a build-directory copy. A depth-first traversal keeps one value map and removes each subtree's added values on return, avoiding growing map copies and preventing sibling values from leaking into one another. This extends the processed-map release fix in [SPIRV-Tools #6962](https://github.com/KhronosGroup/SPIRV-Tools/pull/6962). Remove the patch when the pinned dependency has equivalent copy-free traversal; #6962 alone still copies maps for sibling branches. System-dependency builds need an equivalent fix in their installed package. Optimization passes and validation remain unchanged; wider shader and game performance measurement is needed.
 - Building on Windows requires a specific version of mingw - MinGW-w64 GCC 15.2.0 (`winlibs-gcc15`, `x86_64-ucrt-posix-seh`)
 - Even compiled prx libraries on Windows require nearby (static linking of these dependencies causes conflicts):
   - libgcc_s_seh-1.dll