Implement SPV_KHR_abort extension by vmustya · Pull Request #3691 · KhronosGroup/SPIRV-LLVM-Translator

vmustya · 2026-04-20T17:16:40Z

The SPV_KHR_abort extension introduces the OpAbortKHR instruction,
allowing shaders to terminate execution early.

Assisted-by: Claude Opus 4.7 noreply@anthropic.com

The SPV_KHR_abort extension introduces the `OpAbortKHR` instruction, allowing shaders to terminate execution early. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>

CLAassistant · 2026-04-20T17:16:46Z

All committers have signed the CLA.

michalpaszkowski

LGTM from my side!

vmaksimo

SPV_KHR_constant_data is listed as a dependency, but I don't see it used anywhere in the code - maybe spirv-val will catch this as well.
Please also address clang-tidy failure

vmustya · 2026-04-22T15:26:50Z

Please also address clang-tidy failure

The clang-tidy fails as follows:

/home/runner/work/SPIRV-LLVM-Translator/SPIRV-LLVM-Translator/lib/SPIRV/libSPIRV/SPIRVInstruction.h:903:3: error: visibility of function 'decode' is changed from public in class 'SPIRVEntry' to protected [misc-override-with-different-visibility,-warnings-as-errors]
  903 |   _SPIRV_DEF_ENCDEC2(MessageTypeId, MessageId)
      |   ^
/home/runner/work/SPIRV-LLVM-Translator/SPIRV-LLVM-Translator/lib/SPIRV/libSPIRV/SPIRVEntry.h:151:8: note: expanded from macro '_SPIRV_DEF_ENCDEC2'
  151 |   void decode(std::istream &I) override { getDecoder(I) >> (x) >> (y); }
      |        ^
/home/runner/work/SPIRV-LLVM-Translator/SPIRV-LLVM-Translator/lib/SPIRV/libSPIRV/SPIRVEntry.h:402:16: note: function declared here as public
  402 |   virtual void decode(std::istream &I);
      |                ^

The reported issue is that the encode and decode methods generated by the _SPIRV_DEF_ENCDEC2 macro are protected. However, the base SPIRVEntry class defines these methods as public. I've checked other instruction implementations, like OpReturn and OpUnreachable, and they also define the encode and decode methods as protected. So, this clang-tidy issue is applicable to the whole translator code.

I don't really want to deviate the style other instructions are implemented. So, I believe, that either all the instructions should be changed by a separate commit, or the clang-tidy error should be suppressed. @vmaksimo, what would you suggest?

maarquitos14

I was looking at SPV_KHR_abort separately, but @MrSidims brought this PR to my attention. I have a few extra tests that I'm more than happy to share if you're interested @vmustya.

maarquitos14 · 2026-04-24T09:27:09Z

-  // llvm.trap and llvm.debugtrap intrinsics are not implemented. But for now
-  // don't crash. This change is pending the trap/abort intrinsic
-  // implementation.
+  // TODO: lower llvm.trap / llvm.ubsantrap / llvm.debugtrap to OpAbortKHR


Could we do this in this patch too? Seems very related.

@maarquitos14 Maybe not to overload this PR, we can follow-up in a separate one (together with Reader part update for default/non-friendly IR path). What do you think?

I won't block this PR for this. If you want to follow-up in a separate PR, go for it.

maarquitos14 · 2026-04-24T09:53:50Z

+  if (isa<UnreachableInst>(V)) {
+    // SPV_KHR_abort: OpAbortKHR is itself a block terminator. If the LLVM IR
+    // appends a trailing 'unreachable' after the abort call (the canonical
+    // pattern for noreturn calls), do not emit a second SPIR-V terminator.
+    if (auto *Last = BB->getTerminateInstr())
+      if (Last->getOpCode() == OpAbortKHR)
+        return mapValue(V, const_cast<SPIRVInstruction *>(Last));
    return mapValue(V, BM->addUnreachableInst(BB));
+  }

  if (auto *RI = dyn_cast<ReturnInst>(V)) {
+    // SPV_KHR_abort: OpAbortKHR is itself a SPIR-V block terminator. If the
+    // LLVM IR appends a trailing `ret void` after the abort call, do not emit
+    // a second SPIR-V terminator. We deliberately limit this suppression to
+    // `ret void`: if a non-void function ends with `ret <value>` after
+    // OpAbortKHR, fall through so that the value is still translated and the
+    // user sees a normal SPIR-V validation error rather than silently
+    // dropping the return value.
+    if (auto *Last = BB->getTerminateInstr();
+        !RI->getReturnValue() && Last && Last->getOpCode() == OpAbortKHR)
+      return mapValue(V, const_cast<SPIRVInstruction *>(Last));


I am not sure this is enough to handle all the cases. For example:

call void @llvm.trap()/__spirv_AbortKHR cal void @llvm.lifetime.end() ret void

This would be invalid, because we would have an instruction after a terminator. As far as I can tell, nothing would prevent translating the call to @llvm.lifetime.end() after the OpAbortKHR, which is a terminator. Am I right?

@maarquitos14, I've moved the tests you've shared from the llvm.trap intrinsic to the SPIR-V friendly IR, and adjusted the writer code. Now all the instructions in the basic block, that follow the __spirv_AbortKHR function call, are ignored by the writer. Could you please provide your feedback?

LGTM! Thanks :)

maarquitos14 · 2026-04-24T09:55:38Z

  }

+  case OpAbortKHR: {
+    // OpAbortKHR is a SPIR-V block terminator. In LLVM IR, model it as a call


Should we split in two paths? This one for spirv-friendly IR, and another one for non-friendly which would translate into... llvm.trap maybe?

vmustya · 2026-04-24T12:59:10Z

I have a few extra tests that I'm more than happy to share if you're interested @vmustya.

@maarquitos14, sure! I'd really appreciate that.

maarquitos14 · 2026-04-24T15:16:59Z

I have a few extra tests that I'm more than happy to share if you're interested @vmustya.

@maarquitos14, sure! I'd really appreciate that.

You can find them in maarquitos14@fe51642. I think all of them assume llvm.trap gets translated to OpAbortKHR and back, but still I think they can be useful.

vmaksimo · 2026-04-24T16:14:10Z

I don't really want to deviate the style other instructions are implemented. So, I believe, that either all the instructions should be changed by a separate commit, or the clang-tidy error should be suppressed.

@vmustya If can - we should not introduce new errors, and I believe this is the case. So please just put encode/decode to public - we'll take a look at other instructions separately.

…R call

maarquitos14

LGTM.

The CI environment does not provide the split-file utility, causing the abort-invalid-arg-count.ll test to fail. Split it into two standalone tests that exercise the no-args and two-args negative cases independently, removing the split-file requirement.

vmaksimo

The change with the test coverege looks great, thanks for the work done!

I have the nits about the tests that'd better to be addressed, after that I guess we're perfectly good to go 🙂

vmaksimo

LGTM

vmaksimo · 2026-05-04T14:37:03Z

@svenvh would you like to review this one prior merging?

vmustya · 2026-05-04T20:36:15Z

/backport llvm_release_220

github-actions · 2026-05-04T20:38:40Z

Attempting to create backport to llvm_release_220...

github-actions · 2026-05-04T20:38:45Z

Success. Backport PR created: #3730

vmustya · 2026-05-04T20:39:23Z

/backport llvm_release_210

github-actions · 2026-05-04T21:17:56Z

Attempting to create backport to llvm_release_210...

github-actions · 2026-05-04T21:18:00Z

Success. Backport PR created: #3733

vmustya · 2026-05-04T21:24:57Z

/backport llvm_release_200

github-actions · 2026-05-04T21:28:42Z

Attempting to create backport to llvm_release_200...

github-actions · 2026-05-04T21:28:44Z

Backport to llvm_release_200 failed due to conflicts on commit c5dbe5662a56e57c6592e4f84ac5e087e9306580. Please backport manually.

Implement SPV_KHR_abort extension

d209681

The SPV_KHR_abort extension introduces the `OpAbortKHR` instruction, allowing shaders to terminate execution early. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>

Fix dyn_cast -> isa

9493cdf

michalpaszkowski approved these changes Apr 21, 2026

View reviewed changes

michalpaszkowski requested a review from vmaksimo April 21, 2026 13:43

vmaksimo reviewed Apr 21, 2026

View reviewed changes

Comment thread lib/SPIRV/SPIRVWriter.cpp Outdated

Comment thread test/extensions/KHR/SPV_KHR_abort/abort.ll

Comment thread lib/SPIRV/SPIRVWriter.cpp Outdated

Comment thread lib/SPIRV/SPIRVWriter.cpp

Comment thread lib/SPIRV/libSPIRV/SPIRVInstruction.h Outdated

vmustya added 2 commits April 22, 2026 09:34

Address review comments

0b02b53

Restore the getOperands() method

f541009

maarquitos14 reviewed Apr 24, 2026

View reviewed changes

maarquitos14 mentioned this pull request Apr 24, 2026

[SPIRV] Add support for SPV_KHR_abort extension llvm/llvm-project#193037

Merged

Add more tests; Ignore LLVM IR instructions after the __spirv_AbortKH…

c5de6a4

…R call

maarquitos14 approved these changes Apr 29, 2026

View reviewed changes

vmaksimo reviewed Apr 30, 2026

View reviewed changes

vmaksimo requested a review from svenvh April 30, 2026 12:55

Apply suggestions from the review

0360938

vmaksimo approved these changes May 4, 2026

View reviewed changes

svenvh approved these changes May 4, 2026

View reviewed changes

Merge branch 'main' into spv-khr-abort

e15f699

michalpaszkowski merged commit c5dbe56 into KhronosGroup:main May 4, 2026
9 checks passed

vmustya deleted the spv-khr-abort branch May 4, 2026 19:47

github-actions Bot mentioned this pull request May 4, 2026

[Backport to llvm_release_220] Implement SPV_KHR_abort extension #3730

Open

vmustya mentioned this pull request May 4, 2026

Translate llvm.trap and llvm.ubsantrap to OpAbortKHR #3719

Open

github-actions Bot mentioned this pull request May 4, 2026

[Backport to llvm_release_210] Implement SPV_KHR_abort extension #3733

Open

Uh oh!

Conversation

vmustya commented Apr 20, 2026

Uh oh!

CLAassistant commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michalpaszkowski left a comment

Choose a reason for hiding this comment

Uh oh!

vmaksimo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vmustya commented Apr 22, 2026

Uh oh!

maarquitos14 left a comment

Choose a reason for hiding this comment

Uh oh!

maarquitos14 Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

vmaksimo Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

maarquitos14 Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maarquitos14 Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

vmustya Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

maarquitos14 Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

maarquitos14 Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

vmustya commented Apr 24, 2026

Uh oh!

maarquitos14 commented Apr 24, 2026

Uh oh!

vmaksimo commented Apr 24, 2026

Uh oh!

maarquitos14 left a comment

Choose a reason for hiding this comment

Uh oh!

vmaksimo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vmaksimo left a comment

Choose a reason for hiding this comment

Uh oh!

vmaksimo commented May 4, 2026

Uh oh!

Uh oh!

vmustya commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

vmustya commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

CLAassistant commented Apr 20, 2026 •

edited

Loading

vmaksimo left a comment •

edited

Loading