[PR #3327] [MERGED] shader_recompiler: Rework sharp tracking for robustness #3393

New issue

Closed

opened 2026-02-27 22:03:32 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 22:03:32 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/shadps4-emu/shadPS4/pull/3327
Author: @raphaelthegreat
Created: 7/26/2025
Status: ✅ Merged
Merged: 7/28/2025
Merged by: @squidbus

Base: main ← Head: sharp-track

📝 Commits (8)

d7e5c2e shader_recompiler: Remove remnants of old discard
9fc3d68 resource_tracking_pass: Rework sharp tracking for robustness
fe57b5a resource_tracking_pass: Add source dominance analysis
c024904 resource_tracking_pass: Fix immediate check
c0755e7 resource_tracking_pass: Remove unused template type
45e666a readlane_elimination_pass: Don't add phi when all args are the same
ad2cb2d resource_tracking_pass: Allow phi in disable aniso pattern
e9317b9 resource_tracking_pass: Handle not valid buffer sharp and more phi in aniso pattern

📊 Changes

20 files changed (+303 additions, -242 deletions)

View changed files

📝 CMakeLists.txt (+1 -0)
📝 src/shader_recompiler/backend/spirv/spirv_emit_context.cpp (+5 -5)
📝 src/shader_recompiler/frontend/control_flow_graph.cpp (+1 -20)
📝 src/shader_recompiler/frontend/control_flow_graph.h (+0 -1)
📝 src/shader_recompiler/frontend/structured_control_flow.cpp (+5 -24)
📝 src/shader_recompiler/frontend/translate/scalar_flow.cpp (+3 -5)
📝 src/shader_recompiler/frontend/translate/translate.cpp (+6 -6)
📝 src/shader_recompiler/frontend/translate/translate.h (+4 -3)
📝 src/shader_recompiler/frontend/translate/vector_memory.cpp (+11 -10)
📝 src/shader_recompiler/info.h (+14 -16)
📝 src/shader_recompiler/ir/basic_block.cpp (+2 -2)
📝 src/shader_recompiler/ir/basic_block.h (+8 -0)
📝 src/shader_recompiler/ir/ir_emitter.cpp (+5 -5)
📝 src/shader_recompiler/ir/ir_emitter.h (+3 -3)
📝 src/shader_recompiler/ir/opcodes.inc (+2 -2)
📝 src/shader_recompiler/ir/passes/constant_propagation_pass.cpp (+14 -0)
📝 src/shader_recompiler/ir/passes/readlane_elimination_pass.cpp (+11 -1)
📝 src/shader_recompiler/ir/passes/resource_tracking_pass.cpp (+203 -139)
📝 src/shader_recompiler/ir/reg.h (+1 -0)
📝 src/video_core/amdgpu/resource.h (+4 -0)

📄 Description

On main image+sampler sharp tracking was kinda broken in a silly way. PatchImageSharp would call BreadthFirstSearch on the image instruction itself. Before https://github.com/shadps4-emu/shadPS4/pull/3015 that was fine because there was a single handle argument and image address components are very unlikely to come from user data or read const instruction.

However that PR added a sampler handle argument at the end of the instruction, which caused the image tracking to search in the sampler instruction tree first (because BreadthFirstSearch iterates instruction arguments in reverse order). Thankfully the sampler argument is a composite so it wouldn't get expanded at the first level of the tree. If the image handle was a read const/user data instruction then it would get picked. But if it was a phi, then next iteration the sampler tree would be searched next, leading to the tracking sometimes picking sampler sharps instead of image sharps.

Fix this by reworking sharp tracking for robustness. Instead of searching a single source, traverse the phi tree (only the image handle this time) and find all possible sources. Then attempt to prune these sources, based on dominance analysis. If a source is dominated by another one then the former can be removed as control flow is guaranteed to pass the dominator. This also implicitly performs reachability analysis as the function will also return true if no path from source to resource access block exists in general.

This fixes the rear mirror in Driveclub without reverting the PR mentioned above.

main	PR

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/shadps4-emu/shadPS4/pull/3327 **Author:** [@raphaelthegreat](https://github.com/raphaelthegreat) **Created:** 7/26/2025 **Status:** ✅ Merged **Merged:** 7/28/2025 **Merged by:** [@squidbus](https://github.com/squidbus) **Base:** `main` ← **Head:** `sharp-track` --- ### 📝 Commits (8) - [`d7e5c2e`](https://github.com/shadps4-emu/shadPS4/commit/d7e5c2e09f1056eabdfd9b44790627a174c9ed4d) shader_recompiler: Remove remnants of old discard - [`9fc3d68`](https://github.com/shadps4-emu/shadPS4/commit/9fc3d6837a938a56267757ddcd5ffced2a30e6cd) resource_tracking_pass: Rework sharp tracking for robustness - [`fe57b5a`](https://github.com/shadps4-emu/shadPS4/commit/fe57b5a66e544982f5017953d999806182e69541) resource_tracking_pass: Add source dominance analysis - [`c024904`](https://github.com/shadps4-emu/shadPS4/commit/c024904a63318c881fd0ab66e1f8978c12f2a25d) resource_tracking_pass: Fix immediate check - [`c0755e7`](https://github.com/shadps4-emu/shadPS4/commit/c0755e7222ec5fdaeacdae849cc2ef99a9859746) resource_tracking_pass: Remove unused template type - [`45e666a`](https://github.com/shadps4-emu/shadPS4/commit/45e666a79eee77179c19c98d3bc75605496f4260) readlane_elimination_pass: Don't add phi when all args are the same - [`ad2cb2d`](https://github.com/shadps4-emu/shadPS4/commit/ad2cb2d9862ceafe5b9c271dcb5d4cfea9e30798) resource_tracking_pass: Allow phi in disable aniso pattern - [`e9317b9`](https://github.com/shadps4-emu/shadPS4/commit/e9317b9bfc4b6dab00375fa999027f8020b894e1) resource_tracking_pass: Handle not valid buffer sharp and more phi in aniso pattern ### 📊 Changes **20 files changed** (+303 additions, -242 deletions) <details> <summary>View changed files</summary> 📝 `CMakeLists.txt` (+1 -0) 📝 `src/shader_recompiler/backend/spirv/spirv_emit_context.cpp` (+5 -5) 📝 `src/shader_recompiler/frontend/control_flow_graph.cpp` (+1 -20) 📝 `src/shader_recompiler/frontend/control_flow_graph.h` (+0 -1) 📝 `src/shader_recompiler/frontend/structured_control_flow.cpp` (+5 -24) 📝 `src/shader_recompiler/frontend/translate/scalar_flow.cpp` (+3 -5) 📝 `src/shader_recompiler/frontend/translate/translate.cpp` (+6 -6) 📝 `src/shader_recompiler/frontend/translate/translate.h` (+4 -3) 📝 `src/shader_recompiler/frontend/translate/vector_memory.cpp` (+11 -10) 📝 `src/shader_recompiler/info.h` (+14 -16) 📝 `src/shader_recompiler/ir/basic_block.cpp` (+2 -2) 📝 `src/shader_recompiler/ir/basic_block.h` (+8 -0) 📝 `src/shader_recompiler/ir/ir_emitter.cpp` (+5 -5) 📝 `src/shader_recompiler/ir/ir_emitter.h` (+3 -3) 📝 `src/shader_recompiler/ir/opcodes.inc` (+2 -2) 📝 `src/shader_recompiler/ir/passes/constant_propagation_pass.cpp` (+14 -0) 📝 `src/shader_recompiler/ir/passes/readlane_elimination_pass.cpp` (+11 -1) 📝 `src/shader_recompiler/ir/passes/resource_tracking_pass.cpp` (+203 -139) 📝 `src/shader_recompiler/ir/reg.h` (+1 -0) 📝 `src/video_core/amdgpu/resource.h` (+4 -0) </details> ### 📄 Description On main image+sampler sharp tracking was kinda broken in a silly way. PatchImageSharp would call BreadthFirstSearch on the image instruction itself. Before https://github.com/shadps4-emu/shadPS4/pull/3015 that was fine because there was a single handle argument and image address components are very unlikely to come from user data or read const instruction. However that PR added a sampler handle argument at the end of the instruction, which caused the image tracking to search in the sampler instruction tree first (because BreadthFirstSearch iterates instruction arguments in reverse order). Thankfully the sampler argument is a composite so it wouldn't get expanded at the first level of the tree. If the image handle was a read const/user data instruction then it would get picked. But if it was a phi, then next iteration the sampler tree would be searched next, leading to the tracking sometimes picking sampler sharps instead of image sharps. Fix this by reworking sharp tracking for robustness. Instead of searching a single source, traverse the phi tree (only the image handle this time) and find all possible sources. Then attempt to prune these sources, based on dominance analysis. If a source is dominated by another one then the former can be removed as control flow is guaranteed to pass the dominator. This also implicitly performs reachability analysis as the function will also return true if no path from source to resource access block exists in general. This fixes the rear mirror in Driveclub without reverting the PR mentioned above. | main | PR | | ------------- | ------------- | | <img width="1282" height="719" alt="before" src="https://github.com/user-attachments/assets/d7a03e91-9fe9-49c2-92c4-ab833ed9f575" /> | <img width="1279" height="720" alt="after" src="https://github.com/user-attachments/assets/c8a31ac6-bd67-47ce-98fe-07b6bbccae3d" /> | --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>