Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ if(EMSCRIPTEN)
set(XEUS_CPP_USE_SHARED_XEUS_CPP OFF)
# ENV (https://github.com/emscripten-core/emscripten/commit/6d9681ad04f60b41ef6345ab06c29bbc9eeb84e0)
set(EMSCRIPTEN_FEATURES "${EMSCRIPTEN_FEATURES} -s \"EXPORTED_RUNTIME_METHODS=[ENV']\"")
set(XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS "-fwasm-exceptions" CACHE STRING "Extra compiler/linker flags for wasm builds")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to have sSUPPORT_LONGJMP=wasm as it is enabled by default based on the EH model (wasm or emscripten)

https://emscripten.org/docs/porting/setjmp-longjmp.html#c-setjmp-longjmp-support

Can still mention it, if we want !

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
set(XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS "-fwasm-exceptions" CACHE STRING "Extra compiler/linker flags for wasm builds")
set(XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS "-fwasm-exceptions -sSUPPORT_LONGJMP=wasm" CACHE STRING "Extra compiler/linker flags for wasm builds")

If you look at the Windows Emscripten jobs in this CppInterOp PR https://github.com/compiler-research/CppInterOp/pull/803/checks which tests this xeus-cpp PR, you'll see that it has undefined errors for symbols such as __wasm_longjmp . Although -sSUPPORT_LONGJMP=wasm seems to be brought it automatically with -fwasm-exceptions on Unix systems, it is not being done automatically on Windows systems (probably some bug in Emscripten). With this change the CppInterOp ci should pass for the Emscripten build on all systems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How does that make any sense ? Aren't we cross compiling here for the emscripten-wasm32 target ?

If it works of any platform X, it should work on platform Y.

-sSUPPORT_LONGJMP=wasm seems to be brought it automatically with -fwasm-exceptions

Also I remember going through the source code for this upstream. Its a simple check, nothing to do with any platform is what I remember.

Check the windows CI for cppinterop if that's concerning. If you can't figure it out, I'll add this !

Copy link
Collaborator

Choose a reason for hiding this comment

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

This has been set to resolved, without actually resolving. Please apply this change if you would like to resolve this comment. Without this change CppInterOp cannot drop the need for EMCC_CFLAGS when building xeus-cpp, since -sSUPPORT_LONGJMP=wasm is not being brought in automatically with -fwasm-exceptions on Windows.

set(XEUS_CPP_WASM_KERNEL_EXTRA_ARGS "-fwasm-exceptions;-mllvm;-wasm-enable-sjlj" CACHE STRING "Extra argv entries for wasm kernel.json")
endif()

# Dependencies
Expand Down Expand Up @@ -123,6 +125,10 @@ endif ()
function(configure_kernel kernel)
if(EMSCRIPTEN)
set(prefix "wasm_")
set(XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON "")
foreach(arg IN LISTS XEUS_CPP_WASM_KERNEL_EXTRA_ARGS)
string(APPEND XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON "\"${arg}\", ")
endforeach()
else()
set(XEUS_CPP_PATH "$ENV{PATH}")
set(XEUS_CPP_LD_LIBRARY_PATH "$ENV{LD_LIBRARY_PATH}")
Expand Down Expand Up @@ -220,7 +226,7 @@ macro(xeus_cpp_set_common_options target_name)
target_compile_options(${target_name} PUBLIC /wd4018 /wd4267 /wd4715 /wd4146 /wd4129)
target_compile_options(${target_name} PUBLIC /EHsc)
elseif (EMSCRIPTEN)
target_compile_options(${target_name} PUBLIC -fwasm-exceptions)
target_compile_options(${target_name} PUBLIC ${XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS})
else ()
target_compile_options(${target_name} PUBLIC -fexceptions)
endif ()
Expand Down
4 changes: 1 addition & 3 deletions share/jupyter/kernels/xc11/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"-Xclang", "-iwithsysroot/include/compat",
"-xc",
"-std=c11",
"-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj",
"-msimd128"
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "c",
"metadata": {
Expand Down
4 changes: 1 addition & 3 deletions share/jupyter/kernels/xc17/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"-Xclang", "-iwithsysroot/include/compat",
"-xc",
"-std=c17",
"-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj",
"-msimd128"
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "c",
"metadata": {
Expand Down
4 changes: 1 addition & 3 deletions share/jupyter/kernels/xc23/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"-Xclang", "-iwithsysroot/include/compat",
"-xc",
"-std=c23",
"-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj",
"-msimd128"
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "c",
"metadata": {
Expand Down
4 changes: 2 additions & 2 deletions share/jupyter/kernels/xcpp17/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"@XEUS_CPP_KERNELSPEC_PATH@xcpp",
"-resource-dir", "/lib/clang/@CPPINTEROP_LLVM_VERSION_MAJOR@",
"-Xclang", "-iwithsysroot/include/compat",
"-std=c++17", "-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj", "-msimd128"
"-std=c++17",
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "cpp",
"metadata": {
Expand Down
4 changes: 2 additions & 2 deletions share/jupyter/kernels/xcpp20/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"@XEUS_CPP_KERNELSPEC_PATH@xcpp",
"-resource-dir", "/lib/clang/@CPPINTEROP_LLVM_VERSION_MAJOR@",
"-Xclang", "-iwithsysroot/include/compat",
"-std=c++20", "-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj", "-msimd128"
"-std=c++20",
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "cpp",
"metadata": {
Expand Down
4 changes: 2 additions & 2 deletions share/jupyter/kernels/xcpp23/wasm_kernel.json.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"@XEUS_CPP_KERNELSPEC_PATH@xcpp",
"-resource-dir", "/lib/clang/@CPPINTEROP_LLVM_VERSION_MAJOR@",
"-Xclang", "-iwithsysroot/include/compat",
"-std=c++23", "-fwasm-exceptions",
"-mllvm", "-wasm-enable-sjlj", "-msimd128"
"-std=c++23",
@XEUS_CPP_WASM_KERNEL_EXTRA_ARGS_JSON@"-msimd128"
],
"language": "cpp",
"metadata": {
Expand Down
4 changes: 2 additions & 2 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ if(EMSCRIPTEN)
target_link_libraries(test_xeus_cpp PRIVATE xeus-cpp-static doctest::doctest)

target_compile_options(test_xeus_cpp
PUBLIC "SHELL: -fwasm-exceptions"
PUBLIC "SHELL: ${XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS}"
)
# So we create a html file, as well as the javascript file
set_target_properties(test_xeus_cpp PROPERTIES
Expand All @@ -63,7 +63,7 @@ if(EMSCRIPTEN)
# Without this MINIFY_HTML=0 flag you end up with the situation where the creation of the
# test_xeus_cpp.html file breaks on MacOS, but not Ubuntu
target_link_options(test_xeus_cpp
PUBLIC "SHELL: -fwasm-exceptions"
PUBLIC "SHELL: ${XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS}"
PUBLIC "SHELL: -s MAIN_MODULE=1"
PUBLIC "SHELL: -s WASM_BIGINT"
PUBLIC "SHELL: -s ASSERTIONS=0"
Expand Down