build tbb stub from static library on older toolchains - #258
Merged
Conversation
Rtools42-era toolchains provide an older (non-oneTBB) copy of TBB, so tbb-compat.cpp -- which wraps the oneTBB runtime -- cannot build there, and the install would fail at the stub link. But the old static library already provides the old ABI, so re-export it wholesale instead. Also: - remove stale tbb-compat build artifacts before building the stub; 'make' would otherwise consider a tbb.dll built by a different toolchain (or against a different TBB) up-to-date and re-ship it - clean tbb-compat artifacts in cleanup.R - return invisibly from useTbbPreamble(), so 'NULL' is no longer printed when install.libs.R is run via R -f - standardize on lowercase 'tbb' in logging messages
This was referenced Jul 23, 2026
kevinushey
added a commit
that referenced
this pull request
Jul 28, 2026
* build the bundled onetbb as a shared library on windows Windows was the only platform where TBB was not a shared library loaded at runtime, and nearly every Windows-specific wart traced back to that. Use the bundled oneTBB there too, built shared, shipped as tbb.dll / tbbmalloc.dll and linked exactly as on other platforms. Rtools ships static libraries only, so adopting its TBB meant linking it into RcppParallel.dll. That made the TBB version -- and ABI -- a property of the user's toolchain (Intel TBB 2017 on Rtools42, oneTBB later), and left downstream packages with no TBB library to link against. The workarounds accumulated: re-exporting the whole tbbmalloc archive so the allocator could be resolved (#262), a tbb.dll stub so packages linking -ltbb could load (#249, #250), building that stub differently for legacy toolchains (#258), offering it from RcppParallelLibs() so rstan could link at all (#269), and a library lookup that searched for archives never installed with the package (#270). Worse, the stub linked the TBB archives itself, so it was a second complete copy of the oneTBB scheduler. Both it and RcppParallel.dll export the same 81 tbb::detail::r1:: entry points, both are loaded in every session, and which one a downstream package binds to is up to the linker. Two schedulers with separate thread pools and separate global state is a correctness problem, not a linking inconvenience. This removes: - the Rtools TBB detection in configure.R, and with it the TBB_NAME derivation from archive names and the tbb12 special case - the --whole-archive tbbmalloc re-export, which only existed because there was no DLL to link - BUILD_SHARED_LIBS=0 for Windows - src/tbb-compat/ and buildTbbStub() entirely, along with the #ifndef _WIN32 guard in observer_proxy.cpp and the __TBB_LEGACY_..._PROVIDED macro that let the stub tell which headers it was compiling against - the Windows branches in .install.libs(), loadTbbLibrary(), tbbLdFlags() and tbbLibraryPath(), and the Windows-first load order in .onLoad() Notes on the pieces that are not just deletions: - mingw CMake would name a shared build libtbb.dll; patch PREFIX to "" so the runtime is tbb.dll, the name binaries built against 5.1.11 and earlier import. The import library keeps its lib prefix, so -ltbb still resolves. - the legacy task_scheduler_observer_v3::observe definition now applies on Windows too. mingw has no .def file, so its export comes from a dllexport directive: the declaration gains TBB_EXPORT, which is empty when consuming the headers. - .onLoad loads tbb before RcppParallel on every platform now, since RcppParallel.dll imports from tbb.dll and the package library directory is not on the DLL search path. - tbbLdFlags() keeps -lRcppParallel on Windows: isProcessForkedChild is compiled into RcppParallel.dll, and Windows has no lazy binding to resolve it at load time. The downstream CI check now asserts the property this is all for: the downstream library's TBB symbols come from exactly one module, that module is tbb.dll, and RcppParallel.dll imports from it rather than carrying its own copy. * fix the shared tbb link on rtools42 and aarch64 windows Two failures from the first CI run, both only reachable now that TBB is linked as a shared library on Windows -- a static build has no link step. Rtools42 (gcc 10): the TBB link failed with undefined references to __stack_chk_fail, __strncpy_chk and __strncat_chk. TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE, and on mingw those helpers live in libssp rather than libgcc; newer toolchains pull it in implicitly, Rtools42 does not. Pass -lssp via CMAKE_SHARED_LINKER_FLAGS, mirroring the -lssp that configure.R already adds for RcppParallel's own link. aarch64 Windows: 'lld: error: unknown argument: -z'. Rtools45-aarch64 drives clang with lld, and Clang.cmake adds -Wl,-z,relro,-z,now for any non-Apple target. GNU.cmake already guards this with NOT MINGW; give Clang.cmake the equivalent guard. windows-latest (release) passed on the first run, so the shared mingw build, the PREFIX "" naming and the reordered load in .onLoad() all work. * link libssp via TBB_COMMON_LINK_LIBS rather than the shared linker flags TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE=2, and on mingw the helpers those need (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) live in libssp rather than libgcc. Rtools42 (gcc 10) does not pull it in implicitly, so the link fails with undefined references to them. Passing -lssp through CMAKE_SHARED_LINKER_FLAGS was not enough: those flags are emitted before the objects, and GNU ld only resolves symbols that are already undefined when it reaches a library, so it was discarded. tbb and tbbmalloc happened to survive that because tbb_handle_ipo gives them LTO; tbbmalloc_proxy has no such call and failed to link. Putting ssp in TBB_COMMON_LINK_LIBS routes it through target_link_libraries instead, which lands after the objects, and covers all three targets. * fix the RcppParallel.dll half of the downstream import check Two reasons it was never actually running, both visible in the CI log even though the job passed: - the path was built with paste0("libs", .Platform$r_arch), giving 'libsx64' rather than 'libs/x64'. system.file() returned "" for that, so objdump was handed '/RcppParallel.dll' and reported "No such file". Use archSystemFile(), which the package already has for this, and fail loudly if the library still can't be found. - on aarch64 the runner has an x86_64 objdump from C:/mingw64 ahead of Rtools' own on the PATH, and it cannot read an aarch64 PE, so both halves of the check skipped there. Try the objdump sitting beside the compiler first, and fall back through the remaining candidates. * locate objdump via the compiler R actually builds with The previous attempt looked beside Sys.which("gcc"/"clang"), but on the aarch64 runner those resolve to C:/mingw64 -- the same x86_64 toolchain that leads the PATH -- so both candidates were the one objdump that cannot read an aarch64 PE, and the check still skipped there. They were also not deduplicated, differing only in slash direction. 'R CMD config CC' names the compiler that produced the library, so its objdump can read it by construction. Try that first, and normalize the candidates so one objdump is not tried under two spellings. * don't emit -ltbb when no tbb was installed The Windows branch of tbbLdFlags() named the TBB libraries unconditionally, dropping the file.exists() guard the previous stub-based code had. On a build that fell back to tinythread -- configure.R sets TBB_ENABLED = FALSE only there, when cmake is missing and TBB_LIB is unset -- RcppParallelLibs() then emitted -L<libs> -lRcppParallel -L<lib> -ltbb -ltbbmalloc for libraries that were never built or shipped, so a downstream package would fail to link with "cannot find -ltbb". CxxFlags() correctly reports -DRCPP_PARALLEL_USE_TBB=0 in that configuration, so the package is not even using TBB. Name them only when TBB is enabled and the library actually resolves. Also drop an unverified aside from NEWS: Rtools42 and Rtools45 are confirmed to ship cmake, Rtools40 was not checked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Discovered testing on R 4.2 / Rtools42, which provides an older (non-oneTBB) TBB (
libtbb_static.a,interface5headers). Three issues:R CMD SHLIB -o tbb-compat/tbb.dllconsiders a leftovertbb.dllup-to-date and re-ships it -- observed shipping an Rtools45/oneTBB-built stub into an Rtools42/R 4.2 installation. The stub outputs are now deleted before building, andcleanup.Rremoves them too.tbb-compat.cppwraps the oneTBB runtime (tbb::detail::r1::observe), which doesn't exist in old TBB, so the stub link (and the install) would fail. Skipping the stub isn't an option either: downstream-ltbb(StanHeaders) needs atbb.dllto link, since Rtools42 ships onlylibtbb_static.a. Instead, build the stub by re-exporting the static library wholesale (-Wl,--whole-archive) -- old TBB already provides the old ABI, no shim needed. Dispatch is by presence of<TBB_INC>/oneapi.useTbbPreamble()now returns invisibly (a strayNULLwas printed viaR -fauto-printing), and logging messages standardize on lowercase 'tbb'.The whole-archive path needs validation on a real Rtools42 setup (in progress).