Skip to content

Fix abort extension name#11716

Open
jkwak-work wants to merge 3 commits into
shader-slang:masterfrom
jkwak-work:fix-abort-extension-name
Open

Fix abort extension name#11716
jkwak-work wants to merge 3 commits into
shader-slang:masterfrom
jkwak-work:fix-abort-extension-name

Conversation

@jkwak-work

@jkwak-work jkwak-work commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Motivation

The abort intrinsic emits the SPIR-V OpAbortKHR instruction. That instruction is provided by the Khronos SPIR-V extension named SPV_KHR_abort, but this PR previously emitted and documented the non-registry spelling SPV_KHR_shader_abort. A concrete failure mode is tests/spirv/abort.slang:24: the test expects OpCapability AbortKHR and an OpExtension declaration before the OpAbortKHR instructions, and the validator only recognizes the canonical SPV_KHR_abort extension name.

The same spelling mismatch also existed in Slang's capability source of truth, so source/slang/slang-capabilities.capdef exposed a SPIR-V extension atom named SPV_KHR_shader_abort while emitAbort needed to emit SPV_KHR_abort. That left two names for one SPIR-V extension.

Proposed solution

Use the Khronos extension name everywhere Slang models the SPIR-V extension, and keep the SPIR-V capability as a separate capability atom:

  • source/slang/slang-emit-spirv.cpp:4796 declares OpExtension "SPV_KHR_abort" when emitting OpAbortKHR.
  • source/slang/slang-capabilities.capdef:580 defines the SPIR-V extension atom as SPV_KHR_abort.
  • source/slang/slang-capabilities.capdef:701 adds spvAbort for the SPIR-V AbortKHR capability, inheriting from SPV_KHR_abort.
  • source/slang/slang-capabilities.capdef:1059 maps GL_EXT_shader_abort to GLSL's _GL_EXT_shader_abort atom or SPIR-V's spvAbort atom, preserving the existing abort = GL_EXT_shader_abort compound capability at source/slang/slang-capabilities.capdef:2423.

With the extension name corrected, the abort-related SPIR-V tests no longer need to bypass SPIR-V validation.

Change summary

source/slang/slang-emit-spirv.cpp: emitAbort now emits SPV_KHR_abort while still requiring SpvCapabilityAbortKHR and emitting OpAbortKHR from the already-legalized abort message payload.

source/slang/slang-capabilities.capdef: renames the SPIR-V extension atom to SPV_KHR_abort, adds spvAbort for AbortKHR, updates GL_EXT_shader_abort to use spvAbort for SPIR-V, and keeps the public abort compound capability unchanged.

docs/user-guide/a3-02-reference-capability-atoms.md and docs/command-line-slangc-reference.md: regenerate the capability and command-line references so they list SPV_KHR_abort and spvAbort, and no longer document SPV_KHR_shader_abort.

tests/spirv/abort.slang: updates the FileCheck expectation to OpExtension "SPV_KHR_abort".

Abort-related tests under tests/spirv/, tests/diagnostics/, and tests/autodiff/: remove -skip-spirv-validation now that the emitted extension string is accepted by the bundled SPIR-V validator. tests/diagnostics/abort-unsupported-target.slang:2 also updates its explanatory capability comment.

Concepts and vocabulary

SPIR-V extension atom: a SPV_* capability definition in slang-capabilities.capdef that names a SPIR-V extension such as SPV_KHR_abort.

SPIR-V capability atom: a lower-camel spv* capability definition that models a SPIR-V OpCapability, such as spvAbort for AbortKHR.

abort compound capability: the user-facing capability used by the abort intrinsic. It remains an alias for GL_EXT_shader_abort, which now expands to the GLSL extension atom or the SPIR-V spvAbort atom depending on target.

Process report

The first issue was the emitted SPIR-V module shape. source/slang/slang-emit-spirv.cpp:4794 handles an Abort IR instruction after SPIR-V legalization has already packed the abort message into a payload struct. The input shape is correct: emitAbort receives a valid Abort instruction and still requires SpvCapabilityAbortKHR at source/slang/slang-emit-spirv.cpp:4797. The bug was not the IR producer or payload representation; it was the extension declaration string emitted beside a valid OpAbortKHR. Updating ensureExtensionDeclaration to SPV_KHR_abort fixes the backend mapping at the point where SPIR-V syntax is emitted.

That fix exposed a second source-of-truth issue in the capability model. Leaving SPV_KHR_shader_abort in slang-capabilities.capdef would make Slang document and accept a SPIR-V extension atom whose name does not match the emitted extension. The principled split is to use SPV_KHR_abort for the extension atom and spvAbort for the SPIR-V AbortKHR capability. GL_EXT_shader_abort remains the cross-target capability bridge, now mapping to _GL_EXT_shader_abort for GLSL and spvAbort for SPIR-V.

The test changes follow from the corrected output. tests/spirv/abort.slang:25 now checks the canonical extension string, and the abort tests that previously passed -skip-spirv-validation can use normal SPIR-V validation because the bundled validator recognizes SPV_KHR_abort. These are not guards for malformed input; they remove a validation bypass that existed only because Slang was emitting a non-registry extension spelling.

…r_abort

The SPIRV grammar (spirv.core.grammar.json) registers the AbortKHR capability
under the extension name "SPV_KHR_abort", but the emitter was declaring
"SPV_KHR_shader_abort". The validator's IsEnabledByExtension check looks up
the capability's associated extensions from the grammar table, so the mismatch
caused validation to fail even though the extension string was present.

Fix the emitted extension string and update the test to remove the now-unnecessary
-skip-spirv-validation workaround and stale comment.
Now that the emitter uses the correct extension name SPV_KHR_abort (matching
the SPIRV grammar), spirv-val passes without skipping validation.
@jkwak-work jkwak-work self-assigned this Jun 23, 2026
@jkwak-work jkwak-work requested a review from a team as a code owner June 23, 2026 20:17
@jkwak-work jkwak-work added the pr: non-breaking PRs without breaking changes label Jun 23, 2026
@jkwak-work jkwak-work requested review from bmillsNV and removed request for a team June 23, 2026 20:17
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46bb4860-e041-40db-be45-199e88d73b1a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR corrects the SPIR-V extension name from SPV_KHR_shader_abort to SPV_KHR_abort throughout the codebase. Capability definitions are updated to introduce SPV_KHR_abort and spvAbort atoms and map GL_EXT_shader_abort to the new capability. Documentation is refreshed to reflect these changes. The emitter is fixed to declare the correct extension name. All abort-related test directives then remove the -skip-spirv-validation workaround and FileCheck expectations are updated to match.

Changes

SPV_KHR_abort Extension Name Fix and Validation Re-enablement

Layer / File(s) Summary
Define SPV_KHR_abort and spvAbort capability atoms
source/slang/slang-capabilities.capdef
Replaces SPV_KHR_shader_abort with SPV_KHR_abort atom definition, introduces spvAbort capability for OpAbortKHR, and updates GL_EXT_shader_abort alias to use the new spvAbort capability.
Document SPV_KHR_abort and spvAbort capabilities
docs/command-line-slangc-reference.md, docs/user-guide/a3-02-reference-capability-atoms.md
Adds SPV_KHR_abort and spvAbort capability atoms to documentation, removes outdated SPV_KHR_shader_abort entry, and updates GL_EXT_shader_abort documentation to reference spvAbort for SPIR-V targets.
Correct SPV_KHR_abort extension name in emitter
source/slang/slang-emit-spirv.cpp
emitAbort now declares "SPV_KHR_abort" instead of "SPV_KHR_shader_abort" via ensureExtensionDeclaration.
Remove -skip-spirv-validation and update FileCheck expectations
tests/spirv/abort*.slang, tests/diagnostics/abort*.slang, tests/autodiff/abort-in-differentiable*.slang
All abort test directives remove the -skip-spirv-validation flag; OpExtension FileCheck expectation in tests/spirv/abort.slang is updated from "SPV_KHR_shader_abort" to "SPV_KHR_abort"; test documentation comments are cleaned up to remove references to SPV_KHR_shader_abort.

Suggested reviewers

  • bmillsNV
  • expipiplus1
  • jkiviluoto-nv
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: correcting the SPIR-V abort extension name from SPV_KHR_shader_abort to SPV_KHR_abort.
Description check ✅ Passed The description provides clear context about the fix, references the official SPIRV-Registry specification, and explains the related capability definition updates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

github-actions[bot]

This comment was marked as outdated.

@jkwak-work

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 🟡 Has issues — 1 question

PR renames the SPIR-V extension from the non-registry SPV_KHR_shader_abort to the canonical SPV_KHR_abort, splits out a spvAbort capability atom for AbortKHR, regenerates docs, and removes -skip-spirv-validation from abort tests. The bundled spirv-tools (external/spirv-tools-generated/core_tables_header.inc:154: kSPV_KHR_abort,) recognizes the canonical name, so dropping the validation skip is supported. Five VK_KHR_shader_abort references in adjacent doc comments are not updated — flagged for author intent.

Changes Overview

SPIR-V extension rename (source/slang/slang-emit-spirv.cpp, source/slang/slang-capabilities.capdef)

  • emitAbort now emits OpExtension "SPV_KHR_abort" (the Khronos registry-canonical name) instead of the non-registry SPV_KHR_shader_abort.
  • The capdef extension atom is renamed to SPV_KHR_abort. A new spvAbort capability atom for AbortKHR is added, and GL_EXT_shader_abort now ORs _GL_EXT_shader_abort | spvAbort instead of ... | SPV_KHR_shader_abort. The user-facing abort capability alias is unchanged.

Docs (docs/command-line-slangc-reference.md, docs/user-guide/a3-02-reference-capability-atoms.md)

  • Regenerated to list SPV_KHR_abort and spvAbort and to drop SPV_KHR_shader_abort.

Test directives (tests/spirv/abort*.slang, tests/diagnostics/abort-*.slang, tests/autodiff/abort-in-differentiable*.slang)

  • -skip-spirv-validation removed from 12 abort tests; tests/spirv/abort.slang CHECK updated to OpExtension "SPV_KHR_abort"; explanatory notes about the validator-not-knowing-the-extension are dropped.
Findings (1 total)
Severity Location Finding
🔵 Question source/slang/slang-capabilities.capdef:1049 (and 4 sibling sites) VK_KHR_shader_abort doc-comment references not renamed alongside the SPIR-V atom

Comment thread source/slang/slang-capabilities.capdef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant