-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Define wasm-eh tags in native code. NFC #25284
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
Conversation
I think this was necessary to make dynamic linking work... What changed? Where do we define these now? |
Today we have two different modes:
One the of the reasons for this change is to unify these two modes into one mode that just works everywhere. In the new mode:
|
This change can land here on the emscripten side before the corresponding llvm changes since the strong definition I'm adding here will always take precedence over any weak definitions in object files. |
@@ -0,0 +1,26 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it maybe makes more sense to define __cpp_exception
in libc++abi. IIRC that's where we define the functions that the compiler emits calls to such as __cxa_throw
.
Slightly annoying that we'd be separating __cpp_exception
from __c_longjmp
but I think its' the right layering.
Speaking of which, where does __c_longjmp
belong? Do we define longjmp
in compiler-rt or libc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We define longjmp-related functions here: https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/emscripten_setjmp.c
Also a bit of Emscripten EH related stuff: https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/emscripten_exception_builtins.c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the latter file in compiler-rt because setThrew
is shared between EH and SJLJ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, setThrew
is also used in Emscripten SjLj. Yeah I think that's the reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I move the definitions into their own files. And I moved __cpp_exception
to libcxxabi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are we putting tag symbols, libcxxabi or compiler-rt? llvm/llvm-project#159143 puts both in compiler-rt but this puts __cpp_exceptions
in libc++abi, so..
Personally I think putting both in compiler-rt is fine, given that these are things we never get to upstream to the upstream libc++abi anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should match llvm/llvm-project#159143 once it lands.
c26d130
to
d1b38cd
Compare
This tags are sometimes also weakly defined in each object file by llvm but I hope to remove those definitions going forward and instead rely on the toolchain/compiler-rt to define them.
These tags have also historically be weakly defined in each object file by llvm (at least for non-PIC object files) but I hope to remove those definitions going forward and instead rely on the toolchain/compiler-rt to define them.
This will remove one of the distinctions between PIC and non-PIC generated code.
This change can land here on the emscripten side before the corresponding llvm changes since the strong definition I'm adding here will always take precedence over any weak definitions in object files.
See llvm/llvm-project#159143