Device compatibility for exception throwing code using assertions by EkremA · Pull Request #2038 · AdaptiveCpp/AdaptiveCpp

EkremA · 2026-04-09T20:25:27Z

Added compatibility for code possibly throwing exceptions on device. In LLVM C++ thrown exceptions are handled with the functions __cxa_throw and __cxa_allocate_exception in LLVM IR. However if we run such code on GPU devices, we may not have access to appropriate implementations of these exception functions.

This PR suggests a compilation pass, which parses device IR for calls and invokes to __cxa_throw and replacing them with an assertion __acpp_sscp_assert_fail. Then, the function __cxa_allocate_exception is removed altogether as any allocation of exception structures would no longer be required.

Additionally, the fix of a typo in one of the CI workflow files is suggested.

…tead of __cxa_allocate_exception call

illuhad

I've only read the beginning of the pass -- looks like this needs some general cleanups first :)
Let me know once it's ready to review.

illuhad · 2026-04-10T12:44:25Z

+ */
+// SPDX-License-Identifier: BSD-2-Clause
+#ifndef ACPP_REFLECTION_EXCEPTION_TO_ASSERTION_PASS_HPP
+#define ACPP_REFLECTION_EXCEPTION_TO_ASSERTION_PASS_HPP


Why is it framed as part of reflection and in reflection subdirectory? I don't think it has anything to do with those. Probably it should go into compiler/sscp?

illuhad · 2026-04-10T12:44:53Z

+
+  static const char* CXAThrow = "__cxa_throw";
+  static const char* ACPPSSCPAssertFail = "__acpp_sscp_assert_fail";
+  //static const char* GlibcxxAssertFailBuiltinName = "__acpp_sscp_glibcxx_assert_fail";


I don't think we need this -> remove

illuhad · 2026-04-10T12:46:01Z

+#ifdef HIPSYCL_WITH_DEVICE_EXCEPTIONS
+#include "hipSYCL/compiler/reflection/ExceptionToAssertionPass.hpp"
+// #include "hipSYCL/compiler/reflection/FunctionNameExtractionPass.hpp"
+#endif


Why do we need this include here? The pass is only used inside TargetSeparationPass, right?

illuhad · 2026-04-10T12:46:44Z

-if(NOT DEFINED HIPSYCL_DEBUG_LEVEL)
-  message(FATAL_ERROR "HIPSYCL_DEBUG_LEVEL was not defined by the top-level CMakeLists.txt.")
+if(NOT HIPSYCL_DEBUG_LEVEL)
+  set(HIPSYCL_DEBUG_LEVEL 2 CACHE INTEGER)


This seems like a change for debugging purposes - please remove from this PR!

illuhad · 2026-04-10T12:47:47Z

+
+set(DEVICE_EXCEPTIONS
+  reflection/ExceptionToAssertionPass.cpp)
+add_definitions(-DHIPSYCL_WITH_DEVICE_EXCEPTIONS)


Just add it to the existing SSCP_COMPILER sources above. No need to create something new and even have a macro for it.

illuhad · 2026-04-10T17:17:14Z

+  // detect if C++ throws occur in source code
+  llvm::Function* oldcxaThrow = nullptr;
+  for(auto& F : M)
+    if(F.getName().contains(CXAThrow)) {


Do we need contains or can we just compare for equality?

illuhad · 2026-04-10T17:18:45Z

+      oldcxaThrow = &F;
+    }
+  if (!oldcxaThrow) {
+    llvm::errs() << "Function " << CXAThrow << " not found. Assuming there is no use of exceptions in source code.\n";


Error messages should be prefixed with the pass that produces them. E.g. llvm::errs() << "ExceptionToAssertionPas: ..."

Also, why would we even error in this case? Isn't it expected that in most cases there's no exception in device code?

illuhad · 2026-04-10T17:21:33Z

+  for(auto& F : M)
+    if(F.getName().contains(CXAThrow)) {
+      oldcxaThrow = &F;
+    }


That whole segment I think can be replaced with M.getFunction(CXAThrow)

illuhad · 2026-04-10T17:23:21Z

+  #if LLVM_VERSION_MAJOR >= 16
+      llvmCharType = llvm::PointerType::get(M.getContext(), 0);
+  #else
+      llvmCharType = llvm::PointerType::get(llvm::Type::getInt8Ty(M.getContext()), 0);


No indentation inside preprocessor if/else

illuhad · 2026-04-10T17:24:01Z

+      llvmCharType = llvm::PointerType::get(llvm::Type::getInt8Ty(M.getContext()), 0);
+  #endif
+  if(auto* F = M.getFunction(ACPPSSCPAssertFail))
+    ACPPSSCPAssertFailDeclaration=F;


That if seems unnecessary.... why not llvm::Function* ACPPSSCPAssertFailDeclaration = M.getFunction(...) ?

illuhad · 2026-04-10T17:26:13Z

Also, please add some LIT test cases for this feature :-)

…unction now to avoid code replica across passes touching device assertions, ADDED: LIT tests for ExceptionToAssertionPass

Update this feature branch with changes from main development branch.

…ce of __cxa_throw symbols directly using the hcf-tool, backend-independently

EkremA · 2026-06-09T18:44:19Z

Code has been slightly restructured. getAssertFailBuiltin is a static member function of DeviceAssertPass now. Added LIT tests which check for absence of function symbol __cxa_throw using the utility acpp-hcf-tool, see hcfinspect.sh.

PR is ready for review.

illuhad · 2026-06-17T17:15:48Z

+              }
+            }
+          }
+          if(auto *Call = llvm::dyn_cast<llvm::CallInst>(&Inst)) {


This indentation is misleading. It looks like the CallInst check is inside the InvokeInst check (which would not make any sense), but I think it's actually outside.

illuhad · 2026-06-17T17:23:41Z

+  }
+  // replace all uses of invoke @__cxa_throw with unreachable instruction and erase the invoke instruction
+  for (int i=0; i<cxaThrowInvokes.size(); i++) {
+    UnreachableForCXAInvokes[i]->replaceAllUsesWith(cxaThrowInvokes[i]);


Isn't this the wrong way around? This looks like you are replacing the unreachables with the cxa throw invokes.

Also, it feels a little overcomplicated to store the unreachables in an array. Couldn't you just:

auto *Unreachable = new llvm::UnreachableInst(M.getContext(), cxaThrowInvokes[i]); cxaThrowInvokes[i]->replaceAllUsesWith(Unreachable);

illuhad · 2026-06-17T17:25:34Z

+    ACPPSSCPAssertFailArgs.push_back(builder.CreateGlobalStringPtr(functionName));
+
+    auto *Call = llvm::CallInst::Create(DeviceAssertPass::getAssertFailBuiltin(M), ACPPSSCPAssertFailArgs);
+    Call->insertBefore(cxaThrowCalls[i]);


This section is duplicated from the invoke case...can't you create a function with this code to share the code between both cases?

illuhad · 2026-06-17T17:26:18Z

+
+
+
+


Do we need all those empty lines? :D

illuhad · 2026-06-17T17:31:02Z

+                cxaAllocExcArgs.push_back(*cxaAllocExcIt);
+                i++;
+              }
+              cxaAllocExcCallsArgs.push_back(cxaAllocExcArgs);


Instead of copying out all arguments, can't you just directly access the first argument with callInst->getArgOperand(0)?

illuhad · 2026-06-17T17:33:26Z

+                                                                  static_cast<int>(ConstInt->getZExtValue()));
+                }
+              }
+              auto* allocainstruction = new llvm::AllocaInst(cxaAllocExcArgs[0]->getType(), 0, allocSize, "exception_alloca", Call);


This identifier name does not look like LLVM coding style to me.

illuhad · 2026-06-17T17:36:06Z

+  //remove all calls to __cxa_allocate_exception
+  for (auto *Call : cxaAllocExcCalls2) {
+      if(Call!=nullptr)
+      Call->eraseFromParent();


Do we really need a separate iteration to erase? Can't we combine that with the RAUW step?

illuhad · 2026-06-17T17:37:06Z

+  set(REFLECTION_BUILTINS "")
+endif()


This block seems to be copy-pasted from above....please remove!

illuhad · 2026-06-17T17:37:33Z

  ${SSCP_COMPILER}
  ${STDPAR_COMPILER}
  ${REFLECTION_BUILTINS}
+  ${DEVICE_EXCEPTIONS}


No longer needed, right?

illuhad · 2026-06-17T17:40:08Z

+  });
+  q.wait();
+  std::cout << *data << std::endl;
+  //CHECK-NOT: CXATHROWHIT


It would be good if we could also check for ACPPASSERTFAILHIT here, which I think you're not actually using anywhere?

EkremA added 9 commits June 4, 2025 18:42

Added empty pass

dc3bc2b

alternative landingpad experiment

401c7dd

RAUW: assert fail call instead of __cxa_throw invoke, alloca call ins…

6988c9a

…tead of __cxa_allocate_exception call

Rearranged IR iteration and modifications + additional comments

6d17f2b

Removed residual debug output

eec341b

Fixed occasional double unreachable inst. in IR

ea7f769

alternative insertion of call instruction

582cd34

Merge remote-tracking branch 'origin/develop' into exception_to_assert

0122261

add: more debug info in assert fail; fix: typo in workflow file macos

a307558

illuhad reviewed Apr 10, 2026

View reviewed changes

EkremA added 7 commits April 13, 2026 21:56

ExceptionToAssertionPass moves to SSCP and first cleanups

2631f12

CHANGE: getAssertFailBuiltin from DeviceAssertPass is static member f…

44059b0

…unction now to avoid code replica across passes touching device assertions, ADDED: LIT tests for ExceptionToAssertionPass

tests will focus on omp backend for now + corrected comment

e97b34d

Merge branch 'develop' into exception_to_assert

255a81b

Update this feature branch with changes from main development branch.

LIT Tests for ExceptionToAssertionPass.cpp are now checking for absen…

e8ea90d

…ce of __cxa_throw symbols directly using the hcf-tool, backend-independently

Merge branch 'develop' into exception_to_assert

767dbad

local development path clean up

1cb02d9

illuhad reviewed Jun 17, 2026

View reviewed changes

Conversation

EkremA commented Apr 9, 2026

Uh oh!

illuhad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illuhad commented Apr 10, 2026

Uh oh!

EkremA commented Jun 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants