Skip to content

[TEMP] rework of migration to cppjit on latest sources - #23557

Closed
aaronj0 wants to merge 53 commits into
root-project:masterfrom
aaronj0:cppyy-interop-migration-r21rc
Closed

aaronj0 wants to merge 53 commits into
root-project:masterfrom
aaronj0:cppyy-interop-migration-r21rc

Conversation

@aaronj0

@aaronj0 aaronj0 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

temporary, for CI results

On Windows, the Dyld's library scan (used as last resort for symbols
that resolve neither in the JIT nor in the process) walks the search
path, which includes System32. Windows still ships the Visual C++ 6
runtime there (msvcp60.dll), which exports std::basic_string and other
std:: members whose mangled names match modern lookups - with a
pre-C++11, refcounted ABI. Binding JIT'd code to those exports silently
corrupts every object they touch.

Seen on the Windows CI of the CppInterOp migration branch: JIT'd
wrapper code calling outlined MSVC STL string methods (msvcp140.dll
exports almost none of basic_string, it is header-inline) landed in
msvcp60.dll, producing strings with NULL data pointers but positive
sizes (test_crossinheritance test37: "NULL string with positive size"
from call_fs building its result via operator+=), access violations in
string concatenation, streaming and default-argument materialization
(test_pythonify test10/test19, test_regression test18, test_stltypes
test09, test_streams, test_fragile test20, test_templates test16),
and a direct stack trace with msvcp60!basic_string::find under JIT
frames in tutorial-roofit-roostats-StandardHypoTestInvDemo-py.

Permanently ignore msvcp*/msvcr*/msvcirt* in the scan. The runtimes
the process actually links are already loaded and their exports are
found in-process before this scan is consulted, so this only removes
the ABI-incompatible legacy candidates; genuinely unresolved symbols
now fail with a clear "symbol unresolved" error instead of corrupting
memory.
@aaronj0 aaronj0 added the clean build Ask CI to do non-incremental build on PR label Sep 30, 2026
@aaronj0
aaronj0 marked this pull request as draft September 30, 2026 17:31
@aaronj0 aaronj0 self-assigned this Sep 30, 2026
@aaronj0 aaronj0 closed this Sep 30, 2026
aaronj0 added 14 commits October 1, 2026 09:10
…Match [upstream]

When the best viable function is a not-yet-instantiated template
specialization, force the instantiation of its body with diagnostics
suppressed and reject the match if the body turns out ill-formed.
Callers use this function to probe whether a valid instantiation exists
(e.g. cpyrt's __cppjit_internal::is_equal fallback for comparison
operators), so an ill-formed body must surface as "no viable function"
rather than as a stream of hard errors when the wrapper is compiled at
call time. This mirrors cling's LookupHelper::overloadFunctionSelector.

Note that probing with an expression-SFINAE helper instead is not
equivalent: C++20 rewritten (reversed) operator candidates make
comparisons like a member operator==(const Base&) ambiguous, which is a
hard substitution failure in a SFINAE context but only a warning in a
function body.

InstantiateTemplate equally refuses to hand out a specialization whose
attempted instantiation marked it invalid: any use of it, such as
compiling a call wrapper, can only fail.

With the error spam gone, test21_access_to_global_variables of
test_advancedcpp passes and loses its xfail marker.
…[upstream]

Layered onto upstream's GetVariableOffset (which since carries the
prefer-definition and instantiation-pattern guards, the evaluation of
const initializers found in the AST ahead of the JIT lookup with constant
arrays materialized into a per-interpreter buffer, and odr-use emission
for discardable-ODR linkage):

- Serve module-local (internal-linkage) const/constexpr values from the
  evaluated initializer before any symbol lookup: no library symbol can
  legitimately be such an entity, and a missed JIT lookup falls to the
  symbol autoloader, which loads any library on the dynamic path
  exporting the name (a REPL constexpr int S pulled in msys-2.0.dll on
  the Windows CI, corrupting the host process at exit)
- Keep the initializer reads (getInit/evaluateValue) inside one
  SynthesizingCodeRAII, since they may lazily deserialize; the final
  symbol lookup and the evaluate fallback stay outside it, or execution
  is deferred
- Also ForceCodeGen strong definitions whose initializer is in the AST
  but whose home binary does not export the symbol (Windows: bindexplib
  does not export read-only data)
- On symbol-lookup failure, fall back to odr-using the variable in an
  interpreter-evaluated expression, as TClingDataMemberInfo::Offset does
…upstream]

Models MSVC's std::shared_ptr::operator->, a member template with a
defaulted, SFINAE-constrained template parameter. With no call
arguments BestOverloadFunctionMatch must return an instantiation with a
concrete return type rather than the template pattern, which
smart-pointer detection relies on to find the pointee type.
…sizingCodeRAII [upstream]

SizeOf, GetNumBases, GetBaseClass and IsLambdaClass read declarations
that a PCH or module may still have to deserialize, which on Cling
requires an open transaction; without one the lazy loads hit assertion
failures and crashes. Open the RAII scope right before the
deserializing call. GetNamed's guard now also covers the parent's
typedef unwrap, which is what GetScope needed, and GetClassDecls'
guard moves before its own typedef unwrap.

The same guard covers GetEnumConstantDatamembers, whose enumerator
iteration deserializes as well.
Adapts the vendored CppInterOp to ROOT's Cling:

- ExistsFunctionTemplate, LookupDatamember, GetFunctionsUsingName and
  GetClassTemplatedMethods call buildLookup on the parent context
  before the qualified lookup, as GetNamed does, so that declarations
  Cling registers lazily are visible to it
- make_narg_call drops the class_name:: qualification of the callee so
  that the wrapper dispatches virtually (TInterpreter::Declare is pure
  virtual)
- GetTypeAsString builds its PrintingPolicy from default LangOptions
  instead of the ASTContext's policy, which keeps the
  std::basic_string<char> spelling the cpyrt type factory expects
- GetReferencedType returns a null type for a null input instead of
  dereferencing it
cppyy-backend's TClingCallFunc::compile_wrapper hardcodes
withAccessControl=false for every wrapper: the callee was already
selected (and public-filtered) by the caller, and the generated code
may legitimately name entities that are private at the call site.
The wrapper for RResultPtr<std::vector<T>>::begin() spells its return
type through the private member template RIterationHelper; with access
control on it fails to compile and iterating a Take result raises
(rdf_define_pyz.test_cpp_free_function).

Subsumes the narrower relaxAccessControl escape hatch for
using-shadows; the parameter stays in the signature for its callers.
Consumers of the dispatch API spread across several shared libraries
must share a single pointer table. On ELF the extern pointers resolve
across module boundaries by default; on Windows data symbols are not
auto-exported (WINDOWS_EXPORT_ALL_SYMBOLS covers functions only), so
every non-defining DLL failed to link. Mark the table dllexport in the
module that compiles the definition TU (CPPINTEROP_DISPATCH_EXPORTS)
and dllimport everywhere else.
…patch]

GetCompleteName spelled non-type template arguments with their literal
suffix (array<unsigned int,3UL>). ROOT and cppyy always exposed the
suffix-free form, which the names cpyrt matches against expect.
@aaronj0 aaronj0 reopened this Oct 1, 2026
@aaronj0
aaronj0 force-pushed the cppyy-interop-migration-r21rc branch from 9838a93 to 46e10b8 Compare October 1, 2026 10:38
Full tree of compiler-research/cppjit at the pinned revision, minus
.github and zizmor.yml; cppjit-version.tag records the pin. Not yet
wired into the build.
aaronj0 and others added 27 commits October 1, 2026 13:01
…[upstream]

SmartPtrConverter wrapped ordinary objects into the smart pointer
unconditionally; respect the global implicit smart-pointer conversion
policy like the other implicit-conversion sites.

Matches compiler-research/cppjit#110.
…stream]

A std::string executor may hand back a Python str rather than a proxy;
op_str cast the result to CPPInstance unconditionally and read through
it. Take the text from either shape, and treat the generic
"{not representable}" fallback like an address-only result so op_str
falls through to the generic repr.
…tch]

Port of master's CPyCppyy-Always-convert-returned-std-string.patch
(Jonas Rembser): the std::string executors return a Python str instead
of a std::string proxy, the str-forcing __repr__/__str__ wrappers of
the std::string pythonization go, and the tests compare against the
type std::to_string returns. Unlike the master patch, the executors
keep upstream's assignment type check and pending-exception check.
Adapt the interop layer to ROOT:

- Load the dispatch API from libCling via gSystem->DynamicPathName:
  in ROOT, CppInterOp is compiled into libCling rather than shipped as
  a standalone libclangCppInterOp
- Call TClass::GetClass in GetScope for dictionary/module autoloading
- Guard GetActualClass and GetBaseOffset with Cpp::IsComplete to
  handle incomplete types (e.g. TCling with no public header)
Under runtime_cxxmodules=OFF, autoloading a dictionary registers only a
forward declaration, so GetActualClass saw an incomplete derived scope
and dropped the cast. IsComplete's plain-class branch is a passive
hasDefinition() query that never asks Sema to complete the type, so
ROOT's on-demand autoparse is never triggered. GetOrForceDefinition
completes it through Sema (RequireCompleteType -> autoparse). The
CppInterOp API half landed upstream as root-project#810.
…T-patch]

Take gInterpreterMutex (when ROOT thread safety is enabled) around the
CppInterOp calls of the interop layer, so Python-side reflection
serializes against TCling users on other threads (I/O, RDataFrame
workers). The private recursive mutex still serializes concurrent
Python-side callers when thread safety is off. Lock order is
global-then-local; threads coming through TCling only ever take the
global mutex, so no deadlock.

Constructors and destructors are resolved under the lock but run
without it, like every other wrapped call: their bodies are user code.

cppdef, cppexec, evaluate, include and add_include_path call Cpp
functions directly from Python and would run unlocked; they take the
same lock through _lock_interpreter/_unlock_interpreter, which release
the GIL while waiting. test_root_interpreter_lock.py checks that a
declaration cannot interleave with a TCling user holding the lock.
…tch]

gROOT is a macro expanding to ROOT::GetROOT(), and the
TGlobalMappedFunction ROOT registers for it is not consulted by this
backend. Resolve it to ROOT::Internal::gROOTLocal instead, initializing
it first.
Frontend adaptations for ROOT:

- Expose gbl.gInterpreter via TInterpreter::Instance()
- Route load_library, add_library_path and load_reflection_info through
  gSystem so library resolution follows ROOT's dynamic path (with the
  Windows winmode=0 special case for absolute paths)
- Check cppexec's captured diagnostics besides the Process return code,
  which covers parse errors only: on Windows a runtime lookup failure's
  exception cannot unwind through the JITed wrapper frame, so nothing
  is raised and the code stays 0
…OOT-patch]

Includes loading the test dictionaries by their ACLiC names.
ROOT-owned CMake files inside the vendored tree (src/, python/, test/;
upstream's scikit-build/ExternalProject CMakeLists.txt stays vendored
but unreferenced, keeping future vendor bumps pure): cppjit_backend
STATIC on the etc/cppinterop staging + CppInterOpEtc, cpyrt SHARED
with unresolved-symbol link flags and CPPJIT_USE_CLING (the interpreter
is cling), a thin libcppjit python module, and the pytest suite wired
through ctest with ACLiC-compiled dictionaries. Includes the
PyInit_libcppjit -> Init module split enabling ROOT's three-target
wiring. The build enters these files in the next commit.

The interop layer locates the staged CppInterOp headers through
TROOT::GetEtcDir() instead of a build-directory prefix compiled into
libcpyrt, so an installed ROOT does not depend on its build tree (the
dispatch API itself comes from libCling). The cppjit extension module
installs into the cppjit package directory on every platform, which is
where its loader looks for it.
…patch]

Switch commit: the tree moves from bindings/pyroot/cppyy to the vendored
cppjit in one step so it never holds two live binding stacks.

- Enter the vendored tree's ROOT-owned CMake files (previous commit)
  from bindings/pyroot/CMakeLists.txt
- Install a cppyy compatibility alias package: import cppyy resolves to
  cppjit via sys.modules registration plus a meta-path finder, so
  cppyy.gbl call sites (ROOT facade, roottest, tutorials) keep working;
  the alias loader keeps the module's own __spec__, which importlib
  overwrites on the way
- Retarget pythonizations and TPython to the cpyrt API (includes,
  namespaces, link targets) and the type-repr checks to cppjit
  spellings, including the canonical std::basic_string forms in the
  std::string exclusion list and gInterpreter cached on the facade;
  CPPYY_API_PATH=none and CPPYY_NO_ROOT_FILTER are dropped (cppjit has
  neither the header check nor the filter)
- Pythonize template instantiations that cppjit caches in the namespace
  (they carry __cpp_template__ too), and test membership against
  ns_vars.values(): the previous `instance not in ns_vars` looked at the
  keys, a master bug
- Adapt the roottest atlas-datavector template-instantiation test: with
  cppjit, instantiating foo<AtlasLikeDataVector<CustomStruct>> succeeds,
  since resolving the name autoloads the dictionary and the default
  second template argument becomes visible, so the test asserts the new
  behaviour and checks the demangled typeid name of the instantiation
- Retarget the PyException regression macro (ScottCppyy.C and the
  regression test) from CPyCppyy::PyException to
  cppjit::cpyrt::PyException and the cpyrt/PyException.h header
- Export CppGetProcAddress from libCling's linker script for the
  dispatch mechanism
- Drop the superseded bindings/pyroot/cppyy tree, including master's
  patches/ machinery
bindexplib deliberately excluded all read-only data from the generated
.def files, so const globals in ACLiC-built libraries could never be
resolved at runtime. The interpreter needs exactly that:
test_advancedcpp accesses 'extern const char my_global_string2[]'
whose definition lives in the ACLiC library, and with the symbol
unexported the lookup fails and there is no way to reconstruct the
value (the initializer is not in the AST either).

Export '?'-prefixed (C++-mangled) external read-only data symbols as
DATA. User const globals are what the interpreter needs to resolve;
restricting to C++-mangled names excludes both the compiler-generated
symbols (vtables ??_7, RTTI ??_R, string literals ??_C, deleting dtors
??_G/??_E) and MSVC's constant pools (__real@/__xmm@) and exception
data (_CT*/_TI*). The latter's names get garbled by the cdecl
'@'-truncation into non-existent symbols such as '_real', which would
otherwise break every ACLiC library with a floating-point literal
(LNK2001/LNK1120, 242 CI failures). This only affects runtime lookup:
compile-time clients never linked against these symbols (without
dllimport they could not), so no existing link can change behavior.
Running TPython from an interpreted macro requires code JIT-compiled in
the cling session to resolve the
CppInternal::DispatchRaw::MakeFunctionCallable_interp dispatch slot. On
Windows that fails: the slot is a data symbol (a function-pointer
variable), which the bindexplib-generated export tables do not cover, so
the IncrementalExecutor reports it unresolved and the test fails
deterministically.

Properly exporting writable data across a DLL boundary (or better,
registering the dispatch slots by value in the JIT instead of relying on
symbol lookup, an upstream CppInterOp change) is not worth blocking the
migration for this niche path. Disable the test on MSVC following the
existing precedent in this file, behind the usual win_broken_tests
escape hatch.
…atch]

These fits JIT-compile their formulas through cling on top of an
already large Python + ROOT process, and on the win32 CI the JIT runs
out of 32-bit address space during TH1::Fit:

    LLVM ERROR: out of memory
    Buffer allocation failed
    error code: Exit code 0xc0000409

The baseline does not fail, so this is a memory-footprint effect of the
larger per-method JIT wrapper volume of the CppInterOp-based bindings,
which the 32-bit address space no longer absorbs for these tutorials.
fitNormSum failed this way on the old migration branch and
fitConvolution on the cppjit stack (round 6, identical signature).
Veto them on MSVC x86 following the existing 32-bit veto precedents,
behind the usual win_broken_tests escape hatch.
GetType() resolves names that Cpp::GetType() cannot handle by declaring
`using <unique id> = __typeof__(<name>);` in the interpreter. Without a
cache, every call parses and compiles code and adds one more alias to
the AST, growing the interpreter state unboundedly for repeated names.

Memoize the resolved types by name. A name that fails to resolve may
succeed once more declarations are available, so failures are retried.

Upstream motivation: in ROOT, reading a TTree array branch creates the
converter for the branch's element pointer type (e.g. "Double_t*") on
every access, so an event loop went through the trampoline for every
entry. The original reproducer:

    import array
    import time
    import ROOT
    n = array.array("i", [3])
    e = array.array("d", [1.0, 2.0, 3.0])
    tree = ROOT.TTree("tree", "")
    tree.Branch("n", n, "n/I")
    tree.Branch("e", e, "e[n]/D")
    for _ in range(10000):
        tree.Fill()
    start = time.perf_counter()
    for event in tree:
        event.e  # array branch: the converter for "Double_t*" is created on each access
    print(f"loop over 10000 entries reading an array branch: {time.perf_counter() - start:.2f} s")

This takes 0.11 s instead of 1.66 s with this change.

On performance in cppjit proper, the honest result of validating this is
that no equivalent hot path exists here: with an instrumented build, the
full test suite (559 tests) and targeted scenarios (STL container
template instantiation, low-level views, callbacks through
std::function, operator dispatch, exception translation) produce exactly
one trampoline invocation per process, the one-time resolution of
std::exception. Spellings like "double*" are resolved by Cpp::GetType
directly. So there is no cppjit-side number to quote; the cache prevents
AST alias growth for consumers that do drive this path repeatedly, at
the cost of one map lookup per call.

Matches compiler-research/cppjit#117.
Every polymorphic object handed to Python goes through GetActualClass(),
which demangles the dynamic type's name and resolves it with a full
interpreter name lookup, and then through GetBaseOffset(), which walks
the inheritance paths. Neither result was cached, so this cost was paid
again for every returned object.

Memoize both in the interop wrapper, where the calls are already
serialized by the interop lock:

- GetActualClass per (static class, dynamic type_info). Only outcomes
  that cannot change are cached: a failed lookup may succeed once more
  declarations are available, so it is retried.
- GetBaseOffset per (derived, base) class pair. The object address was
  already ignored, so the offset is fixed per pair once both records are
  complete. An incomplete record has no walkable bases (the lookup would
  read an empty base path), so -- like the error case -- it is treated
  silently and retried next time rather than frozen into the cache.

For example, getting objects out of a container of base pointers, with
auto-downcast to the actual derived class on each access:

    import time
    import cppjit

    cppjit.cppdef(r"""
    #include <vector>
    struct PerfBase {
        virtual ~PerfBase() = default;
        virtual int value() const { return 1; }
    };
    struct PerfDerived : PerfBase {
        int value() const override { return 2; }
    };
    std::vector<PerfBase*> make_perf_vec(int n) {
        static PerfDerived d[64];
        std::vector<PerfBase*> v;
        for (int i = 0; i < n; ++i) v.push_back(&d[i % 64]);
        return v;
    }
    """)

    vec = cppjit.gbl.make_perf_vec(100)
    vec.at(0)  # warm-up
    start = time.perf_counter()
    for _ in range(2000):
        for i in range(100):
            vec.at(i)  # returns PerfBase*, auto-downcast to PerfDerived
    print(f"200k vector<PerfBase*>.at() calls: {time.perf_counter() - start:.2f} s")

This takes 0.18 s instead of 0.40 s per 200k accesses (Release build
against the pinned CppInterOp, min of 7 repetitions, interleaved A/B
against the pristine library, ~2.3x). An instrumented build confirms one
GetActualClass() and one GetBaseOffset() call per element access before
this change.

If the name lookup is expensive -- the actual class is templated with a
non-type template argument, e.g. `Impl<float, (EOp)0>`, whose spelled
name needs interpreter-assisted resolution -- the difference is much
larger: a single returned base pointer cost ~420 us and now costs
~0.6 us.

Matches compiler-research/cppjit#115.
Passing an object of a derived class to a formal base-class argument, as
well as binding a method call whose 'self' is an instance of a class
derived from the method's declaring class, makes the bindings check
interop::IsSubclass() on every call, and the clang derivation check is
not free.

Memoize IsSubclass() per class pair in the interop wrapper, where the
calls are already serialized by the interop lock. A class that is still
incomplete can gain bases once its definition is loaded, so a negative
answer is only cached when both classes are complete.

For example, calling a C++ function that takes a base-class reference
with a derived instance:

    import time
    import cppjit

    cppjit.cppdef(r"""
    struct SubBase {
        virtual ~SubBase() = default;
        virtual int value() const { return 1; }
    };
    struct SubDerived : SubBase {
        int value() const override { return 2; }
    };
    int take_base(const SubBase& b) { return b.value(); }
    """)

    d = cppjit.gbl.SubDerived()
    cppjit.gbl.take_base(d)  # warm-up
    start = time.perf_counter()
    for _ in range(200000):
        cppjit.gbl.take_base(d)
    print(f"200k calls: {time.perf_counter() - start:.2f} s")

This takes 0.08 s instead of 0.22 s per 200k calls in a pristine build
(0.18 s with only the parent commit's caches applied; ~2x further).
Release build against the pinned CppInterOp, min of 7 repetitions,
interleaved A/B. An instrumented build confirms the converter queries
IsSubclass() once per call on this path, so the saved cost is the clang
derivation check itself.

Matches compiler-research/cppjit#116.
@aaronj0
aaronj0 force-pushed the cppyy-interop-migration-r21rc branch from 46e10b8 to 8058981 Compare October 1, 2026 11:07
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Test Results

    24 files      24 suites   4d 2h 57m 34s ⏱️
 3 871 tests  3 868 ✅ 0 💤 3 ❌
83 786 runs  83 780 ✅ 0 💤 6 ❌

For more details on these failures, see this check.

Results for commit 8058981.

@aaronj0 aaronj0 closed this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants