Skip to content

[WebAssembly] Generate invokes with llvm.wasm.(re)throw #128105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 25, 2025

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Feb 21, 2025

Even though __builtin_wasm_throw, which is lowered down to llvm.wasm.throw, throws,

try {
  __builtin_wasm_throw(0, obj);
} catch (...) {
}

does not generate invoke. This is because we have assumed the intrinsic cannot be invoked, which doesn't make much sense. (See #128104 for the historical context)

#128104 made llvm.wasm.throw intrinsic invokable in the backend. This actually generates invokes in Clang for __builtin_wasm_throw.

While we're at it, this also generates invokes for __builtin_wasm_rethrow, which is actually not used anywhere in C++ support. I haven't deleted it just in case in may have uses later. (For example, to support rethrow functionality that carries stack trace with exnref)

Depends on #128104 for the CI to pass.
Fixes #124710.

Even though `__builtin_wasm_throw`, which is lowered down to
`llvm.wasm.throw`, throws,
```cpp
try {
  __builtin_wasm_throw(0, obj);
} catch (...) {
}
```
does not generate `invoke`. This is because we have assumed the
intrinsic cannot be invoked, which doesn't make much sense. (See llvm#128104
for the historical context)

 llvm#128104 made `llvm.wasm.throw` intrinsic invokable in the backend. This
actually generates `invoke`s in Clang for `__builtin_wasm_throw`.

While we're at it, this also generates `invoke`s for
`__builtin_wasm_rethrow`, which is actually not used anywhere in C++
support. I haven't deleted it just in case in may have uses later. (For
example, to support rethrow functionality that carries stack trace
with exnref)

Depends on llvm#128104 for the CI to pass.
Fixes llvm#124710.
@aheejin aheejin requested a review from dschuff February 21, 2025 01:37
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Heejin Ahn (aheejin)

Changes

Even though __builtin_wasm_throw, which is lowered down to llvm.wasm.throw, throws,

try {
  __builtin_wasm_throw(0, obj);
} catch (...) {
}

does not generate invoke. This is because we have assumed the intrinsic cannot be invoked, which doesn't make much sense. (See #128104 for the historical context)

#128104 made llvm.wasm.throw intrinsic invokable in the backend. This actually generates invokes in Clang for __builtin_wasm_throw.

While we're at it, this also generates invokes for __builtin_wasm_rethrow, which is actually not used anywhere in C++ support. I haven't deleted it just in case in may have uses later. (For example, to support rethrow functionality that carries stack trace with exnref)

Depends on #128104 for the CI to pass.
Fixes #124710.


Full diff: https://github.com/llvm/llvm-project/pull/128105.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+2-2)
  • (added) clang/test/CodeGenCXX/builtins-eh-wasm.cpp (+20)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 348cb523b1718..d63e3cc093e17 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -22493,11 +22493,11 @@ Value *CodeGenFunction::EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
     Value *Tag = EmitScalarExpr(E->getArg(0));
     Value *Obj = EmitScalarExpr(E->getArg(1));
     Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_throw);
-    return Builder.CreateCall(Callee, {Tag, Obj});
+    return EmitRuntimeCallOrInvoke(Callee, {Tag, Obj});
   }
   case WebAssembly::BI__builtin_wasm_rethrow: {
     Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
-    return Builder.CreateCall(Callee);
+    return EmitRuntimeCallOrInvoke(Callee);
   }
   case WebAssembly::BI__builtin_wasm_memory_atomic_wait32: {
     Value *Addr = EmitScalarExpr(E->getArg(0));
diff --git a/clang/test/CodeGenCXX/builtins-eh-wasm.cpp b/clang/test/CodeGenCXX/builtins-eh-wasm.cpp
new file mode 100644
index 0000000000000..b0f763d3e54dc
--- /dev/null
+++ b/clang/test/CodeGenCXX/builtins-eh-wasm.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fexceptions -fcxx-exceptions -target-feature +reference-types -target-feature +exception-handling -target-feature +multivalue -exception-model=wasm -emit-llvm -o - %s | FileCheck %s
+
+// Check if __builtin_wasm_throw and __builtin_wasm_rethrow are correctly
+// invoked when placed in try-catch.
+
+void throw_in_try(void *obj) {
+  try {
+    __builtin_wasm_throw(0, obj);
+  } catch (...) {
+  }
+  // CHECK: invoke void @llvm.wasm.throw(i32 0, ptr %{{.*}})
+}
+
+void rethrow_in_try() {
+  try {
+  __builtin_wasm_rethrow();
+  } catch (...) {
+  }
+  // CHECK: invoke void @llvm.wasm.rethrow()
+}

// invoked when placed in try-catch.

void throw_in_try(void *obj) {
try {
Copy link
Member

Choose a reason for hiding this comment

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

do we also expect there to be an invoke when not placed in a try, or does EmitRuntimeCallOrInvoke handle that case automatically?

Copy link
Member Author

Choose a reason for hiding this comment

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

It handles the case automatically (hence CallOrInvoke)


void rethrow_in_try() {
try {
__builtin_wasm_rethrow();
Copy link
Member Author

Choose a reason for hiding this comment

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

This code actually does not make sense because this call is not within a catch, but this test only has the purpose of checking whether invoke is generated and does not go through the backend, so it should be fine
(As I said in the PR description, we don't use this builtin for our C++ implementation, but I didn't remove this because other people might want to use it)

@aheejin aheejin merged commit 40566fd into llvm:main Feb 25, 2025
11 checks passed
@aheejin aheejin deleted the invoke_throw branch February 25, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

llvm.wasm.throw is inconsistently considered non-unwinding or invocable
4 participants