Add DebugBuildIdentifier to the live worklist during initialization#6626
Merged
s-perron merged 4 commits intoKhronosGroup:mainfrom Apr 20, 2026
Merged
Conversation
DebugBuildIdentifier was missing from the set of NonSemantic debug instructions added to the worklist in InitializeModuleScopeLiveInstructions. Instead it was handled in ProcessGlobalValues (after the worklist is exhausted) using live_insts_.Set() on its direct operands. live_insts_.Set() marks an instruction live but does not enqueue it, so transitive operand dependencies are never visited. In shaders where the only use of a type (e.g. OpTypeInt) is through a constant that is itself only referenced by DebugBuildIdentifier's flags argument, the type instruction is not marked live and is incorrectly killed by ADCE. The surviving constant then references a deleted type, producing invalid SPIR-V. Any subsequent pass that rebuilds the DefUseManager (e.g. CCP) will fail with "Definition is not registered." Fix: include NonSemanticShaderDebugInfo100DebugBuildIdentifier in the worklist loop alongside the other always-live debug instructions. The worklist's transitive closure correctly marks all operand dependencies live before global-value cleanup runs. The existing special-case code in ProcessGlobalValues becomes unreachable for this opcode (IsLive returns true, so the early continue fires) and is now dead; it is left in place for safety.
Adds AggressiveDCETest.KeepDebugBuildIdentifier to verify that DebugBuildIdentifier and its transitive operands (OpTypeInt, OpConstant) are preserved and the output remains valid after ADCE runs. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Contributor
Author
|
@SteveUrquhart , can you re-review this PR? |
Contributor
Author
|
Fixes #6619 |
jkwak-work
added a commit
to jkwak-work/slang
that referenced
this pull request
Apr 3, 2026
This PR includes the latest SPIRV-Tools and the matching SPIRV-Headers. This PR also includes a spirv-opt fix that is currently in a review process: KhronosGroup/SPIRV-Tools#6626
github-merge-queue Bot
pushed a commit
to shader-slang/slang
that referenced
this pull request
Apr 3, 2026
This PR includes the latest SPIRV-Tools and the matching SPIRV-Headers. This PR also includes a spirv-opt fix that is currently in a review process: KhronosGroup/SPIRV-Tools#6626
github-merge-queue Bot
pushed a commit
to shader-slang/slang
that referenced
this pull request
Apr 3, 2026
This PR includes the latest SPIRV-Tools and the matching SPIRV-Headers. This PR also includes a spirv-opt fix that is currently in a review process: KhronosGroup/SPIRV-Tools#6626
s-perron
approved these changes
Apr 8, 2026
Contributor
Author
|
@s-perron , I resolved the merge conflict. |
auto-merge was automatically disabled
April 20, 2026 21:47
Head branch was pushed to by a user without write access
Contributor
Author
|
@s-perron , sorry, I had to re-resolve the merge-conflict. |
Contributor
Author
|
@s-perron , I am not sure what is happening. |
2 tasks
kaizhangNV
added a commit
to kaizhangNV/slang
that referenced
this pull request
Apr 23, 2026
Update SPIRV-Tools to ff5c5033 (origin/main) and SPIRV-Headers to ad9184e (origin/main). The previous spirv-tools pointed to a fork with custom patches for DebugBuildIdentifier in ADCE; those fixes have since been merged upstream (KhronosGroup/SPIRV-Tools#6626), so we can now point directly to upstream main. Notable upstream changes: - spirv-val: Add validation for 4-bit integer types - spirv-val: Handle OpSpecConstantDataKHR / OpConstantDataKHR - spirv-opt: Fix DebugValue placements - spirv-as: Fix OpConstantDataKHR new grammar - Add support for SPV_KHR_abort and SPV_KHR_constant_data - Add validation for NSDI 101 Made-with: Cursor
kaizhangNV
added a commit
to kaizhangNV/slang
that referenced
this pull request
Apr 24, 2026
Update SPIRV-Tools to ff5c5033 (origin/main) and SPIRV-Headers to ad9184e (origin/main). The previous spirv-tools pointed to a fork with custom patches for DebugBuildIdentifier in ADCE; those fixes have since been merged upstream (KhronosGroup/SPIRV-Tools#6626), so we can now point directly to upstream main. Notable upstream changes: - spirv-val: Add validation for 4-bit integer types - spirv-val: Handle OpSpecConstantDataKHR / OpConstantDataKHR - spirv-opt: Fix DebugValue placements - spirv-as: Fix OpConstantDataKHR new grammar - Add support for SPV_KHR_abort and SPV_KHR_constant_data - Add validation for NSDI 101 Made-with: Cursor
jkwak-work
pushed a commit
to jkwak-work/slang
that referenced
this pull request
Apr 24, 2026
…#10929) ## Summary - Update SPIRV-Tools submodule to `ff5c5033` (upstream `main`) and SPIRV-Headers to `ad9184e` (upstream `main`) - The previous SPIRV-Tools pointed to a fork with custom patches for DebugBuildIdentifier in ADCE; those fixes have since been merged upstream ([KhronosGroup/SPIRV-Tools#6626](KhronosGroup/SPIRV-Tools#6626)), so we now point directly to upstream main - Regenerated `external/spirv-tools-generated/` files to match ### Notable upstream SPIRV-Tools changes (21 commits): - spirv-val: Add validation for 4-bit integer types (shader-slang#6644) - spirv-val: Handle OpSpecConstantDataKHR (shader-slang#6646) - spirv-val: Validate OpConstantDataKHR (shader-slang#6643) - spirv-opt: Fix DebugValue placements (shader-slang#6599) - spirv-as: Fix OpConstantDataKHR new grammar (shader-slang#6642) - Add support for SPV_KHR_abort and SPV_KHR_constant_data (shader-slang#6625) - Add validation for NSDI 101 (shader-slang#6635) - Fix crash in shuffle validation (shader-slang#6645) - spirv-opt: add missing OpConstantCompositeReplicateEXT support to constant manager (shader-slang#6616) ### Notable upstream SPIRV-Headers changes (4 commits): - Fix constant data operand types (shader-slang#581) - Add NSDI 101 instructions and version bump (shader-slang#579) - Add version-unified grammar and header for NonSemantic.Shader.DebugInfo (shader-slang#578) - [Bazel] Add NonSemanticDebugBreak.h to the common headers (shader-slang#577) ## Test plan - [x] Builds successfully on Linux (Release) - [x] CI passes Made with [Cursor](https://cursor.com)
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
3 participants
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.
DebugBuildIdentifier was missing from the set of NonSemantic debug instructions added to the worklist in InitializeModuleScopeLiveInstructions. Instead it was handled in ProcessGlobalValues (after the worklist is exhausted) using live_insts_.Set() on its direct operands.
live_insts_.Set() marks an instruction live but does not enqueue it, so transitive operand dependencies are never visited. In shaders where the only use of a type (e.g. OpTypeInt) is through a constant that is itself only referenced by DebugBuildIdentifier's flags argument, the type instruction is not marked live and is incorrectly killed by ADCE.
The surviving constant then references a deleted type, producing invalid SPIR-V. Any subsequent pass that rebuilds the DefUseManager (e.g. CCP) will fail with "Definition is not registered."
Fix: include NonSemanticShaderDebugInfo100DebugBuildIdentifier in the worklist loop alongside the other always-live debug instructions. The worklist's transitive closure correctly marks all operand dependencies live before global-value cleanup runs. The existing special-case code in ProcessGlobalValues becomes unreachable for this opcode (IsLive returns true, so the early continue fires) and is now dead; it is left in placefor safety.