Skip to content

[TEST PR] CallFunc -> JitCall migration #225

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[TEST PR] CallFunc -> JitCall migration #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
607eb67
[metacling] Use InterOps JitCall for default object construction
aaronj0 Jun 18, 2025
d183e14
[interop] Upgrade version to bring in upstreamed patches
aaronj0 Jun 20, 2025
078eb65
[metacling] Drop the ctor wrapper codepath, map, and `tcling_callfunc…
aaronj0 Jun 25, 2025
195a654
[metacling] further simplify by using Cpp::Construct
aaronj0 Jun 26, 2025
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
217 changes: 12 additions & 205 deletions core/metacling/src/TClingCallFunc.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,197 +1065,6 @@ static std::string PrepareStructorWrapper(const Decl *D, const char *wrapper_pre
return wrapper_name;
}

tcling_callfunc_ctor_Wrapper_t TClingCallFunc::make_ctor_wrapper(const TClingClassInfo *info,
ROOT::TMetaUtils::EIOCtorCategory kind, const std::string &type_name)
{
// Make a code string that follows this pattern:
//
// void
// unique_wrapper_ddd(void** ret, void* arena, unsigned long nary)
// {
// if (!arena) {
// if (!nary) {
// *ret = new ClassName;
// }
// else {
// *ret = new ClassName[nary];
// }
// }
// else {
// if (!nary) {
// *ret = new (arena) ClassName;
// }
// else {
// *ret = new (arena) ClassName[nary];
// }
// }
// }
//
// When I/O constructor used:
//
// void
// unique_wrapper_ddd(void** ret, void* arena, unsigned long nary)
// {
// if (!arena) {
// if (!nary) {
// *ret = new ClassName((TRootIOCtor*)nullptr);
// }
// else {
// char *buf = malloc(nary * sizeof(ClassName));
// for (int k=0;k<nary;++k)
// new (buf + k * sizeof(ClassName)) ClassName((TRootIOCtor*)nullptr);
// *ret = buf;
// }
// }
// else {
// if (!nary) {
// *ret = new (arena) ClassName((TRootIOCtor*)nullptr);
// }
// else {
// for (int k=0;k<nary;++k)
// new ((char *) arena + k * sizeof(ClassName)) ClassName((TRootIOCtor*)nullptr);
// *ret = arena;
// }
// }
// }
//
//
// Note:
//
// If the class is of POD type, the form:
//
// new ClassName;
//
// does not initialize the object at all, and the form:
//
// new ClassName();
//
// default-initializes the object.
//
// We are using the form without parentheses because that is what
// CINT did.
//
//--

static map<const Decl *, void *> gCtorWrapperStore;

R__LOCKGUARD_CLING(gInterpreterMutex);

auto D = info->GetDecl();
auto I = gCtorWrapperStore.find(D);
if (I != gCtorWrapperStore.end())
return (tcling_callfunc_ctor_Wrapper_t)I->second;

//
// Make the wrapper name.
//
string class_name;
string wrapper_name = PrepareStructorWrapper(D, "__ctor", class_name);

string constr_arg;
if (kind == ROOT::TMetaUtils::EIOCtorCategory::kIOPtrType)
constr_arg = string("((") + type_name + "*)nullptr)";
else if (kind == ROOT::TMetaUtils::EIOCtorCategory::kIORefType)
constr_arg = string("(*((") + type_name + "*)arena))";

//
// Write the wrapper code.
//
int indent_level = 0;
ostringstream buf;
buf << "__attribute__((used)) ";
buf << "extern \"C\" void ";
buf << wrapper_name;
buf << "(void** ret, void* arena, unsigned long nary)\n";
buf << "{\n";

// if (!arena) {
// if (!nary) {
// *ret = new ClassName;
// }
// else {
// *ret = new ClassName[nary];
// }
// }
indent(buf, ++indent_level);
buf << "if (!arena) {\n";
indent(buf, ++indent_level);
buf << "if (!nary) {\n";
indent(buf, ++indent_level);
buf << "*ret = new " << class_name << constr_arg << ";\n";
indent(buf, --indent_level);
buf << "}\n";
indent(buf, indent_level);
buf << "else {\n";
indent(buf, ++indent_level);
if (constr_arg.empty()) {
buf << "*ret = new " << class_name << "[nary];\n";
} else {
buf << "char *buf = (char *) malloc(nary * sizeof(" << class_name << "));\n";
indent(buf, indent_level);
buf << "for (int k=0;k<nary;++k)\n";
indent(buf, ++indent_level);
buf << "new (buf + k * sizeof(" << class_name << ")) " << class_name << constr_arg << ";\n";
indent(buf, --indent_level);
buf << "*ret = buf;\n";
}
indent(buf, --indent_level);
buf << "}\n";
indent(buf, --indent_level);
buf << "}\n";
// else {
// if (!nary) {
// *ret = new (arena) ClassName;
// }
// else {
// *ret = new (arena) ClassName[nary];
// }
// }
indent(buf, indent_level);
buf << "else {\n";
indent(buf, ++indent_level);
buf << "if (!nary) {\n";
indent(buf, ++indent_level);
buf << "*ret = new (arena) " << class_name << constr_arg << ";\n";
indent(buf, --indent_level);
buf << "}\n";
indent(buf, indent_level);
buf << "else {\n";
indent(buf, ++indent_level);
if (constr_arg.empty()) {
buf << "*ret = new (arena) " << class_name << "[nary];\n";
} else {
buf << "for (int k=0;k<nary;++k)\n";
indent(buf, ++indent_level);
buf << "new ((char *) arena + k * sizeof(" << class_name << ")) " << class_name << constr_arg << ";\n";
indent(buf, --indent_level);
buf << "*ret = arena;\n";
}
indent(buf, --indent_level);
buf << "}\n";
indent(buf, --indent_level);
buf << "}\n";
// End wrapper.
--indent_level;
buf << "}\n";
// Done.
string wrapper(buf.str());
//fprintf(stderr, "%s\n", wrapper.c_str());
//
// Compile the wrapper code.
//
void *F = compile_wrapper(wrapper_name, wrapper,
/*withAccessControl=*/false);
if (F) {
gCtorWrapperStore.insert(make_pair(D, F));
} else {
::Error("TClingCallFunc::make_ctor_wrapper",
"Failed to compile\n ==== SOURCE BEGIN ====\n%s\n ==== SOURCE END ====",
wrapper.c_str());
}
return (tcling_callfunc_ctor_Wrapper_t)F;
}

tcling_callfunc_dtor_Wrapper_t
TClingCallFunc::make_dtor_wrapper(const TClingClassInfo *info)
{
Expand Down Expand Up @@ -1609,20 +1418,18 @@ void *TClingCallFunc::ExecDefaultConstructor(const TClingClassInfo *info,
::Error("TClingCallFunc::ExecDefaultConstructor", "Invalid class info!");
return nullptr;
}
if (tcling_callfunc_ctor_Wrapper_t wrapper = make_ctor_wrapper(info, kind, type_name)) {
//if (!info->HasDefaultConstructor()) {
// // FIXME: We might have a ROOT ioctor, we might
// // have to check for that here.
// ::Error("TClingCallFunc::ExecDefaultConstructor",
// "Class has no default constructor: %s",
// info->Name());
// return 0;
//}
void *obj = nullptr;
(*wrapper)(&obj, address, nary);
return obj;
}
::Error("TClingCallFunc::ExecDefaultConstructor", "Called with no wrapper, not implemented!");
// this function's clients assume nary = 0 for operator new and nary > 0 for array new. JitCall expects
// the number of objects you want to construct; nary = 1 constructs a single object
// This handles this difference in semantics
if (nary == 0)
nary = 1;

clang::Decl *D = const_cast<clang::Decl *>(info->GetDecl());

if (Cpp::IsClass(D) || Cpp::IsConstructor(D))
return Cpp::Construct(D, address, nary);

::Error("TClingCallFunc::ExecDefaultConstructor", "ClassInfo missing a valid Scope/Constructor");
return nullptr;
}

Expand Down
9 changes: 3 additions & 6 deletions core/metacling/src/TClingCallFunc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "TInterpreter.h"

#include "cling/Interpreter/Value.h"
#include <CppInterOp/CppInterOp.h>

#include "clang/AST/ASTContext.h"
#include "llvm/ADT/SmallVector.h"
Expand All @@ -53,9 +54,8 @@ class TClingClassInfo;
class TClingMethodInfo;
class TInterpreterValue;

typedef void (*tcling_callfunc_Wrapper_t)(void*, int, void**, void*);
typedef void (*tcling_callfunc_ctor_Wrapper_t)(void**, void*, unsigned long);
typedef void (*tcling_callfunc_dtor_Wrapper_t)(void*, unsigned long, int);
typedef void (*tcling_callfunc_Wrapper_t)(void *, int, void **, void *);
typedef void (*tcling_callfunc_dtor_Wrapper_t)(void *, unsigned long, int);

class TClingCallFunc {

Expand Down Expand Up @@ -109,9 +109,6 @@ class TClingCallFunc {

tcling_callfunc_Wrapper_t make_wrapper();

tcling_callfunc_ctor_Wrapper_t
make_ctor_wrapper(const TClingClassInfo *, ROOT::TMetaUtils::EIOCtorCategory, const std::string &);

tcling_callfunc_dtor_Wrapper_t
make_dtor_wrapper(const TClingClassInfo* info);

Expand Down
2 changes: 1 addition & 1 deletion interpreter/CppInterOp/.clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
BasedOnStyle: LLVM

Language: Cpp
Standard: Cpp11
Standard: c++17
PointerAlignment: Left

IncludeCategories:
Expand Down
2 changes: 2 additions & 0 deletions interpreter/CppInterOp/Emscripten-build-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ $env:CMAKE_SYSTEM_PREFIX_PATH=$env:PREFIX
```

on Windows. Now to build and test your Emscripten build of CppInterOp using node on Linux and osx execute the following
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)

```bash
mkdir build
Expand All @@ -190,6 +191,7 @@ emmake make -j $(nproc --all) check-cppinterop
```

To build and test your Emscripten build of CppInterOp on using node Windows execute the following
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)

```powershell
emcmake cmake -DCMAKE_BUILD_TYPE=Release `
Expand Down
12 changes: 10 additions & 2 deletions interpreter/CppInterOp/cmake/modules/GoogleTest.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,18 @@ include(ExternalProject)
if (EMSCRIPTEN)
if (CMAKE_C_COMPILER MATCHES ".bat")
set(config_cmd emcmake.bat cmake)
set(build_cmd emmake.bat make)
if(CMAKE_GENERATOR STREQUAL "Ninja")
set(build_cmd emmake.bat ninja)
else()
set(build_cmd emmake.bat make)
endif()
else()
set(config_cmd emcmake cmake)
set(build_cmd emmake make)
if(CMAKE_GENERATOR STREQUAL "Ninja")
set(build_cmd emmake ninja)
else()
set(build_cmd emmake make)
endif()
endif()
else()
set(config_cmd ${CMAKE_COMMAND})
Expand Down
2 changes: 1 addition & 1 deletion interpreter/CppInterOp/cppinterop-version.tag
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130dd30548b59d8f618339158ab917c083184968
9c9423069529caea5aee79b61580fe9a7b1eb73b
2 changes: 1 addition & 1 deletion interpreter/CppInterOp/docs/DebuggingCppInterOp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Here's a comprehensive example that demonstrates common CppInterOp usage pattern

.. code-block:: cpp

#include "clang/Interpreter/CppInterOp.h"
#include <CppInterOp/CppInterOp.h>
#include <iostream>

void run_code(std::string code) {
Expand Down
2 changes: 2 additions & 0 deletions interpreter/CppInterOp/docs/Emscripten-build-instructions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ and
$env:CMAKE_SYSTEM_PREFIX_PATH=$env:PREFIX

on Windows. Now to build and test your Emscripten build of CppInterOp on Linux and osx execute the following
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)

.. code:: bash

Expand All @@ -214,6 +215,7 @@ on Windows. Now to build and test your Emscripten build of CppInterOp on Linux a
emmake make -j $(nproc --all) check-cppinterop

To build and test your Emscripten build of CppInterOp on Windows execute the following
(BUILD_SHARED_LIBS=ON is only needed if building xeus-cpp, as CppInterOp can be built as an Emscripten static library)

.. code:: powershell

Expand Down
4 changes: 3 additions & 1 deletion interpreter/CppInterOp/include/CppInterOp/CppInterOp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ class JitCall {
///\param[in] nary - Use array new if we have to construct an array of
/// objects (nary > 1).
///\param[in] args - a pointer to a argument list and argument size.
///\param[in] is_arena - a pointer that indicates if placement new is to be
/// used
// FIXME: Change the type of withFree from int to bool in the wrapper code.
void InvokeConstructor(void* result, unsigned long nary = 1,
ArgList args = {}, void* is_arena = nullptr) const {
Expand Down Expand Up @@ -849,7 +851,7 @@ CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,

/// Creates one or more objects of class \c scope by calling its default
/// constructor.
/// \param[in] scope Class to construct
/// \param[in] scope Class to construct, or handle to Constructor
/// \param[in] arena If set, this API uses placement new to construct at this
/// address.
/// \param[in] is used to indicate the number of objects to construct.
Expand Down
8 changes: 7 additions & 1 deletion interpreter/CppInterOp/lib/CppInterOp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ endif()


if(EMSCRIPTEN)
if(BUILD_SHARED_LIBS)
# FIXME: When dynamically linking the Emscripten shared library to the
# unit tests main_module you get errors due to undefined symbols. The reading of the file
# below into a SYMBOLS_LIST variable is a temporary workaround that exports the undefined
Expand All @@ -131,8 +132,13 @@ if(EMSCRIPTEN)
PRIVATE "SHELL: -s SIDE_MODULE=1"
PRIVATE "SHELL: ${SYMBOLS_LIST}"
)
else()
target_link_options(clangCppInterOp
PRIVATE "SHELL: -s WASM_BIGINT"
)
endif()
if (CPPINTEROP_ENABLE_TESTING)
# When compiling Emscripten tests the shared library it links to is expected to be in the same folder as the compiled Javascript
# When compiling Emscripten tests the CppInterOp library it links to is expected to be in the same folder as the compiled Javascript
add_custom_command(TARGET clangCppInterOp POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:clangCppInterOp> ${CMAKE_BINARY_DIR}/unittests/CppInterOp/
)
Expand Down
Loading