From 619e4276a7f8b854a780118c92928c3ff9136769 Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Thu, 17 Apr 2025 13:45:19 +0530 Subject: [PATCH 1/3] Enable support for multiple kernels for xeus-cpp-lite --- CMakeLists.txt | 26 +++++-------------- include/xeus-cpp/xinterpreter_wasm.hpp | 1 + .../kernels/xcpp17/wasm_kernel.json.in | 15 +++++++++++ .../kernels/xcpp20/wasm_kernel.json.in | 3 +-- .../kernels/xcpp23/wasm_kernel.json.in | 15 +++++++++++ src/xinterpreter.cpp | 6 +---- src/xinterpreter_wasm.cpp | 5 ++++ 7 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 share/jupyter/kernels/xcpp17/wasm_kernel.json.in create mode 100644 share/jupyter/kernels/xcpp23/wasm_kernel.json.in diff --git a/CMakeLists.txt b/CMakeLists.txt index da8d5d76..404c1456 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -148,6 +148,7 @@ function(configure_native_kernel kernel) endfunction() function(configure_wasm_kernel kernel) + set(XEUS_CPP_RESOURCE_DIR "/lib/clang/${CPPINTEROP_LLVM_VERSION_MAJOR}") configure_file ( "${CMAKE_CURRENT_SOURCE_DIR}${kernel}wasm_kernel.json.in" @@ -169,17 +170,9 @@ endfunction() message("Configure kernels: ...") if(EMSCRIPTEN) - # TODO: Currently jupyterlite-xeus and xeus-lite do not provide - # methods to fetch information from the arguments present in the - # generated emscripten kernel. - # The following needs to be done here : - # 1) We need to configure the kernel properly - # Check issue https://github.com/compiler-research/xeus-cpp/issues/185. - # 2) Once the above is done we need to add support in jupyterlite-xeus & xeus-lite - # to be able to deal with arguments present in kernel.json - # 3) Finally we should fetch the C++ version from the kernel.json file and - # be able to pass it to our wasm interpreter rather than forcing a version. + configure_wasm_kernel("/share/jupyter/kernels/xcpp17/") configure_wasm_kernel("/share/jupyter/kernels/xcpp20/") + configure_wasm_kernel("/share/jupyter/kernels/xcpp23/") else() configure_native_kernel("/share/jupyter/kernels/xcpp17/") configure_native_kernel("/share/jupyter/kernels/xcpp20/") @@ -444,20 +437,13 @@ if(EMSCRIPTEN) xeus_wasm_link_options(xcpp "web,worker") target_link_options(xcpp PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" + # PUBLIC "SHELL: --preload-file ${CMAKE_INSTALL_PREFIX}${XEUS_CPP_RESOURCE_DIR}/include@${XEUS_CPP_RESOURCE_DIR}" PUBLIC "SHELL: --preload-file ${XEUS_CPP_DATA_DIR}@/share/xeus-cpp" PUBLIC "SHELL: --preload-file ${XEUS_CPP_CONF_DIR}@/etc/xeus-cpp" PUBLIC "SHELL: --post-js ${CMAKE_CURRENT_SOURCE_DIR}/wasm_patches/post.js" ) - # TODO: Emscripten supports preloading files just once before it generates - # the xcpp.data file (containing the binary representation of the file(s) we - # want to include in our application). - # Hence although we are adding support for Standard Headers, Libraries etc - # through emscripten's sysroot for now, we need to do the following: - # 1) Enable CppInterOp to provide us with a resource dir. - # 2) If the above cannot be done, we can use the resource dir provided - # by llvm on emscripten-forge but would involve adding a dependency. - # 3) Shift the resource dir and the sysroot to a common location. - # 4) Preload everything required together. + # TODO: Uncomment the above line regarding preloading clang's resource dir + # once has been supported through cppinterop. endif() # Tests # ===== diff --git a/include/xeus-cpp/xinterpreter_wasm.hpp b/include/xeus-cpp/xinterpreter_wasm.hpp index abb7614e..439775ba 100644 --- a/include/xeus-cpp/xinterpreter_wasm.hpp +++ b/include/xeus-cpp/xinterpreter_wasm.hpp @@ -20,6 +20,7 @@ namespace xcpp public: wasm_interpreter(); + wasm_interpreter(int argc, char** argv); virtual ~wasm_interpreter() = default; }; diff --git a/share/jupyter/kernels/xcpp17/wasm_kernel.json.in b/share/jupyter/kernels/xcpp17/wasm_kernel.json.in new file mode 100644 index 00000000..a6be99dd --- /dev/null +++ b/share/jupyter/kernels/xcpp17/wasm_kernel.json.in @@ -0,0 +1,15 @@ +{ + "display_name": "C++17", + "argv": [ + "@XEUS_CPP_KERNELSPEC_PATH@xcpp", + "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@", + "-std=c++17" + ], + "language": "cpp", + "metadata": { + "debugger": false, + "shared": { + "libclangCppInterOp.so": "lib/libclangCppInterOp.so" + } + } +} diff --git a/share/jupyter/kernels/xcpp20/wasm_kernel.json.in b/share/jupyter/kernels/xcpp20/wasm_kernel.json.in index 697e46a9..ea792b9f 100644 --- a/share/jupyter/kernels/xcpp20/wasm_kernel.json.in +++ b/share/jupyter/kernels/xcpp20/wasm_kernel.json.in @@ -2,8 +2,7 @@ "display_name": "C++20", "argv": [ "@XEUS_CPP_KERNELSPEC_PATH@xcpp", - "-f", - "{connection_file}", + "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@", "-std=c++20" ], "language": "cpp", diff --git a/share/jupyter/kernels/xcpp23/wasm_kernel.json.in b/share/jupyter/kernels/xcpp23/wasm_kernel.json.in new file mode 100644 index 00000000..93ffa621 --- /dev/null +++ b/share/jupyter/kernels/xcpp23/wasm_kernel.json.in @@ -0,0 +1,15 @@ +{ + "display_name": "C++23", + "argv": [ + "@XEUS_CPP_KERNELSPEC_PATH@xcpp", + "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@", + "-std=c++23" + ], + "language": "cpp", + "metadata": { + "debugger": false, + "shared": { + "libclangCppInterOp.so": "lib/libclangCppInterOp.so" + } + } +} diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 85aa07b1..3b5e74df 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -27,10 +27,7 @@ using Args = std::vector; void* createInterpreter(const Args &ExtraArgs = {}) { - Args ClangArgs = {/*"-xc++"*/"-v"}; // ? {"-Xclang", "-emit-llvm-only", "-Xclang", "-diagnostic-log-file", "-Xclang", "-", "-xc++"}; -#ifdef EMSCRIPTEN - ClangArgs.push_back("-std=c++20"); -#else + Args ClangArgs = {/*"-xc++"*/"-v"}; if (std::find_if(ExtraArgs.begin(), ExtraArgs.end(), [](const std::string& s) { return s == "-resource-dir";}) == ExtraArgs.end()) { std::string resource_dir = Cpp::DetectResourceDir(); @@ -45,7 +42,6 @@ void* createInterpreter(const Args &ExtraArgs = {}) { ClangArgs.push_back("-isystem"); ClangArgs.push_back(CxxInclude.c_str()); } -#endif ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end()); // FIXME: We should process the kernel input options and conditionally pass // the gpu args here. diff --git a/src/xinterpreter_wasm.cpp b/src/xinterpreter_wasm.cpp index 7746c7b6..d4bb763a 100644 --- a/src/xinterpreter_wasm.cpp +++ b/src/xinterpreter_wasm.cpp @@ -20,4 +20,9 @@ namespace xcpp : interpreter(0, nullptr) { } + + wasm_interpreter::wasm_interpreter(int argc, char** argv) + : interpreter(argc, argv) + { + } } From 0766b26bf0b359caebf7a12e2d0302d1c24fb11e Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Sun, 27 Apr 2025 11:55:15 +0530 Subject: [PATCH 2/3] Update constructor for wasm interpreter --- CMakeLists.txt | 4 ++-- include/xeus-cpp/xinterpreter_wasm.hpp | 3 +-- src/main_emscripten_kernel.cpp | 19 ++++++++++++++++++- src/xinterpreter.cpp | 10 ++++++---- src/xinterpreter_wasm.cpp | 6 ------ 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 404c1456..bacd6e7f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -148,7 +148,7 @@ function(configure_native_kernel kernel) endfunction() function(configure_wasm_kernel kernel) - set(XEUS_CPP_RESOURCE_DIR "/lib/clang/${CPPINTEROP_LLVM_VERSION_MAJOR}") + set(XEUS_CPP_RESOURCE_DIR "/lib/clang/${CPPINTEROP_LLVM_VERSION_MAJOR}" PARENT_SCOPE) configure_file ( "${CMAKE_CURRENT_SOURCE_DIR}${kernel}wasm_kernel.json.in" @@ -437,7 +437,7 @@ if(EMSCRIPTEN) xeus_wasm_link_options(xcpp "web,worker") target_link_options(xcpp PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" - # PUBLIC "SHELL: --preload-file ${CMAKE_INSTALL_PREFIX}${XEUS_CPP_RESOURCE_DIR}/include@${XEUS_CPP_RESOURCE_DIR}" + #PUBLIC "SHELL: --preload-file ${CMAKE_INSTALL_PREFIX}${XEUS_CPP_RESOURCE_DIR}@${XEUS_CPP_RESOURCE_DIR}" PUBLIC "SHELL: --preload-file ${XEUS_CPP_DATA_DIR}@/share/xeus-cpp" PUBLIC "SHELL: --preload-file ${XEUS_CPP_CONF_DIR}@/etc/xeus-cpp" PUBLIC "SHELL: --post-js ${CMAKE_CURRENT_SOURCE_DIR}/wasm_patches/post.js" diff --git a/include/xeus-cpp/xinterpreter_wasm.hpp b/include/xeus-cpp/xinterpreter_wasm.hpp index 439775ba..8b1d54c5 100644 --- a/include/xeus-cpp/xinterpreter_wasm.hpp +++ b/include/xeus-cpp/xinterpreter_wasm.hpp @@ -19,8 +19,7 @@ namespace xcpp { public: - wasm_interpreter(); - wasm_interpreter(int argc, char** argv); + wasm_interpreter(int argc = 0, char** argv = nullptr); virtual ~wasm_interpreter() = default; }; diff --git a/src/main_emscripten_kernel.cpp b/src/main_emscripten_kernel.cpp index e25f8960..54bd9903 100644 --- a/src/main_emscripten_kernel.cpp +++ b/src/main_emscripten_kernel.cpp @@ -15,9 +15,26 @@ #include "xeus-cpp/xinterpreter_wasm.hpp" +template +static interpreter_type* builder_with_args(emscripten::val js_args) +{ + static std::vector args = emscripten::vecFromJSArray(js_args); + static std::vector argv_vec; + + for (const auto& s : args) + { + argv_vec.push_back(s.c_str()); + } + + int argc = static_cast(argv_vec.size()); + char** argv = const_cast(argv_vec.data()); + + return new interpreter_type(argc, argv); +} + EMSCRIPTEN_BINDINGS(my_module) { xeus::export_core(); using interpreter_type = xcpp::wasm_interpreter; - xeus::export_kernel("xkernel"); + xeus::export_kernel>("xkernel"); } diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 3b5e74df..936aef7f 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -31,10 +31,12 @@ void* createInterpreter(const Args &ExtraArgs = {}) { if (std::find_if(ExtraArgs.begin(), ExtraArgs.end(), [](const std::string& s) { return s == "-resource-dir";}) == ExtraArgs.end()) { std::string resource_dir = Cpp::DetectResourceDir(); - if (resource_dir.empty()) - std::cerr << "Failed to detect the resource-dir\n"; - ClangArgs.push_back("-resource-dir"); - ClangArgs.push_back(resource_dir.c_str()); + if (!resource_dir.empty()) { + ClangArgs.push_back("-resource-dir"); + ClangArgs.push_back(resource_dir.c_str()); + } else { + std::cerr << "Failed to detect the resource-dir\n"; + } } std::vector CxxSystemIncludes; Cpp::DetectSystemCompilerIncludePaths(CxxSystemIncludes); diff --git a/src/xinterpreter_wasm.cpp b/src/xinterpreter_wasm.cpp index d4bb763a..9e65bdb9 100644 --- a/src/xinterpreter_wasm.cpp +++ b/src/xinterpreter_wasm.cpp @@ -15,12 +15,6 @@ namespace xcpp { - - wasm_interpreter::wasm_interpreter() - : interpreter(0, nullptr) - { - } - wasm_interpreter::wasm_interpreter(int argc, char** argv) : interpreter(argc, argv) { From 2e28107c8c280b5cb1cd88433c7ed1e02b0918fd Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Tue, 6 May 2025 14:03:43 +0530 Subject: [PATCH 3/3] address reviews --- src/main_emscripten_kernel.cpp | 6 +++++- src/xinterpreter.cpp | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main_emscripten_kernel.cpp b/src/main_emscripten_kernel.cpp index 54bd9903..555b596a 100644 --- a/src/main_emscripten_kernel.cpp +++ b/src/main_emscripten_kernel.cpp @@ -18,6 +18,7 @@ template static interpreter_type* builder_with_args(emscripten::val js_args) { + // TODO: Refactor interpreter constructor to avoid static args-to-argv conversion. static std::vector args = emscripten::vecFromJSArray(js_args); static std::vector argv_vec; @@ -29,7 +30,10 @@ static interpreter_type* builder_with_args(emscripten::val js_args) int argc = static_cast(argv_vec.size()); char** argv = const_cast(argv_vec.data()); - return new interpreter_type(argc, argv); + auto* res = new interpreter_type(argc, argv); + argv_vec.clear(); + args.clear(); + return res; } EMSCRIPTEN_BINDINGS(my_module) diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 936aef7f..14972324 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -18,6 +18,7 @@ #include "xinput.hpp" #include "xinspect.hpp" #include "xmagics/os.hpp" +#include #ifndef EMSCRIPTEN #include "xmagics/xassist.hpp" #endif