Repository navigation
Preserve ViewID dependencies across retained helper calls - #9114
Open
Sandro Trianni (5andr0) wants to merge 1 commit into
Open
Sandro Trianni (5andr0) wants to merge 1 commit into
Sandro Trianni (5andr0) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unchecked calls to GetDxilOpFuncCallInst on non-DXIL instructions can abort assertions-enabled builds or misinterpret constant arguments as DXIL opcodes when assertions are disabled.
2 open issues
What changed in this PR
This PR updates DXC’s ViewID analysis to preserve input and ViewID dependencies across retained helper calls, including writes through output pointers.
Changes:
- Replaces recursive dependency tracing with reusable function summaries and fixed-point memory analysis.
- Tracks reachable blocks, PHI selection control, and recognized DXIL pointer operations.
- Adds regression tests for helper calls, aliasing, memory operations, unsupported calls, and amplification payloads.
| File | Description |
|---|---|
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/unreachable-out-store.ll | Tests exclusion of unreachable writes. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/unreachable-opaque-call.ll | Tests ignoring unreachable opaque calls. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/unknown-out-call.ll | Tests rejection of unknown memory-writing calls. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/summary-alias-feedback.ll | Tests alias feedback and private allocation isolation. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/shared-call-dag.hlsl | Tests shared helper call graphs without expansion. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/scalar-phi-out-dependencies.ll | Tests PHI dependencies through output pointers. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/scalar-phi-dependencies.ll | Tests PHI dependencies through return values. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/recursive-out-call.ll | Tests rejection of recursive calls. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/recognized-pointer-memory.ll | Tests DXIL pointer load/store dependencies. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/pointer-call-dependencies.ll | Tests nested pointer calls, casts, and PHI cycles. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/out-argument-dependencies.hlsl | Tests call-specific output argument dependencies. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/opaque-readonly-call.ll | Tests rejection of opaque readonly calls. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/nested-out-dependencies.hlsl | Tests nested inout and conditional writes. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/memory-intrinsic-dependencies.ll | Tests memcpy, memset, and atomic updates. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/memmove-cmpxchg-dependencies.ll | Tests memmove and compare-exchange dependencies. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/indirect-out-call.ll | Tests rejection of indirect calls. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/identical-pointer-phi.ll | Tests independence of identical pointer selections. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/identical-phi-out-actuals.ll | Tests identical arguments selected into output pointers. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/identical-phi-actuals.ll | Tests identical arguments selected into return values. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/distinct-pointer-phi.ll | Tests control dependence of distinct pointer selections. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/caller-control-dependencies.ll | Tests caller control over helper writes. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/callee-viewid-dependencies.ll | Tests callee ViewID reads and shared global writes. |
| tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_viewid/amplification-payload.hlsl | Tests amplification payload compilation. |
| lib/HLSL/ComputeViewIdStateBuilder.cpp | Implements function summaries and fixed-point dependency propagation. |
Review effort: Balanced
Sandro Trianni (5andr0)
force-pushed
the
codex/fix-callee-output-dependencies
branch
from
November 11, 2026 16:41
8130d4e to
2d9b25d
Compare
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
8 open issues
Dalready contains guarded dependencies… · NewDependencytuple is difficult to reason about and makes it easy to misuse field positions… · NewSize()(which walks all maps/sets) every iteration, turning… · NewSize()(which walks all maps/sets) every iteration, turning… · Newerror, which is broad and risks matching… · New
Review effort: Lite
Sandro Trianni (5andr0)
force-pushed
the
codex/fix-callee-output-dependencies
branch
from
November 11, 2026 18:02
2d9b25d to
0d3e2c6
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Reverse-ordered dependency and memory-copy chains still cause quadratic rescanning, requiring change-driven scheduling and regression coverage.
3 open issues
7 resolved since last review
error, which is broad and risks matching…Size()(which walks all maps/sets) every iteration, turning…Size()(which walks all maps/sets) every iteration, turning…Dependencytuple is difficult to reason about and makes it easy to misuse field positions…
Review effort: Balanced
Compute completed symbolic function summaries for return values, memory effects and signature outputs, and propagate them into callers. Revisit pointer, value and memory readers when their sets grow, including control consumers and alias feedback, instead of repeatedly sweeping instructions. Preserve reachable-CFG filtering, selection precision and empty effects. Add reverse-layout and loop-carried coverage and a release note. An assertions-enabled build passes 50 focused files / 58 RUN commands. The original retained-IR case and four normal source configurations restore the exact 82 missing input-output dependency edges.
Sandro Trianni (5andr0)
force-pushed
the
codex/fix-callee-output-dependencies
branch
from
November 11, 2026 18:48
0d3e2c6 to
da86a8b
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ViewID analysis can miss input dependencies when a retained helper writes through an output pointer. Summarize each reachable function's return, memory effects and signature outputs, then substitute the used arguments at each call site. Resolve pointer and memory dependencies to a fixed point without expanding call paths.
Preserve PHI selection control and independent identical selections, exclude unreachable blocks, and account for recognized DXIL pointer operations while retaining amplification shader support.
Validation: 43 focused ViewID test files with 51 RUN commands, including helper calls, alias feedback, PHIs, shared call graphs, memory intrinsics and amplification payloads. The existing 29 files and 34 RUN commands retain their assertions; the official amplification compilation and FileCheck check also pass.