[SPIR-V] Follow-up to #8616: exclude entry functions from function-target decoration - #8719
Open
mmoult wants to merge 3 commits into
Open
[SPIR-V] Follow-up to #8616: exclude entry functions from function-target decoration#8719mmoult wants to merge 3 commits into
mmoult wants to merge 3 commits into
Conversation
…nction-target decoration Resolve some issues raised on the last pull request. Most notably, fix the interaction between decorations on entry points and decorations on regular functions. The pre-existing test for entry points was not specific enough to catch introduced bug. This has been amended.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents entry-point inline SPIR-V decorations from incorrectly targeting source OpFunctions.
Changes:
- Moves ordinary-function attribute handling into
SpirvEmitter. - Excludes entry functions from function-target decorations.
- Strengthens entry-point and diagnostic tests; updates release notes.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/clang/lib/SPIRV/SpirvEmitter.cpp |
Applies attributes based on entry-point status. |
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp |
Removes unconditional function decoration. |
tools/clang/test/CodeGenSPIRV/inline-spirv/spv.intrinsicDecorate.hlsl |
Rejects accidental source-function decoration. |
tools/clang/test/CodeGenSPIRV/inline-spirv/spv.intrinsicDecorate.function.error.hlsl |
Checks both unsupported function attributes. |
tools/clang/test/CodeGenSPIRV/inline-spirv/spv.intrinsicDecorate.entry.hlsl |
Adds focused entry-point coverage. |
docs/ReleaseNotes.md |
Documents function decoration support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update code comments for accuracy and brevity.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/clang/lib/SPIRV/SpirvEmitter.cpp:1651
- Ray-tracing entries are not covered by
processInlineSpirvAttributes:emitEntryFunctionWrapperreturns throughemitEntryFunctionWrapperForRayTracingat line 15141 before the call at line 15226. Consequently, this new entry-point guard removes the only registration of[[vk::ext_capability]]and[[vk::ext_extension]]for ray-tracing entry functions, so those attributes are silently dropped. Keep decoration excluded, but register capabilities/extensions for ray entries as well.
if (!isEntry) {
declIdMapper.decorateWithIntrinsicAttrs(decl, func);
declIdMapper.registerCapabilitiesAndExtensionsForDecl(decl);
}
damyanp
reviewed
Jul 30, 2026
Comment on lines
+71
to
+74
| - Functions can now be decorated with inline SPIR-V, and | ||
| `[[vk::ext_capability]]`/`[[vk::ext_extension]]` are honored on ordinary | ||
| functions | ||
| [#8616](https://github.com/microsoft/DirectXShaderCompiler/pull/8616). |
Member
There was a problem hiding this comment.
This needs to go in the upcoming release section, not the just-release 1.9.2607 one.
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.
Resolve some issues raised on the last pull request. Most notably, fix the interaction between decorations on entry points and decorations on regular functions. The pre-existing test for entry points was not specific enough to catch introduced bug. This has been amended.