diff --git a/.github/scripts/rstan-fit-check.R b/.github/scripts/rstan-fit-check.R new file mode 100644 index 00000000..2d44b3db --- /dev/null +++ b/.github/scripts/rstan-fit-check.R @@ -0,0 +1,40 @@ +# Compile and fit a trivial Stan model. Building rstan proves the headers and +# link line are right; actually running a model additionally exercises the TBB +# runtime that RcppParallel loaded -- including the reduce_sum / task isolation +# path that the old Rtools42 TBB could not support. + +library(rstan) + +code <- " +data { + int N; + vector[N] y; +} +parameters { + real mu; +} +model { + y ~ normal(mu, 1); +} +" + +set.seed(42) +data <- list(N = 20L, y = rnorm(20, mean = 3)) + +fit <- stan( + model_code = code, + data = data, + chains = 1L, + iter = 200L, + refresh = 0L +) + +estimate <- mean(rstan::extract(fit, "mu")[["mu"]]) +writeLines(sprintf("posterior mean of mu: %.3f (data mean %.3f)", estimate, mean(data$y))) + +# a wildly wrong answer would mean the model ran but the sampler is broken; +# with 20 observations and unit scale this is a very loose bound +if (!is.finite(estimate) || abs(estimate - mean(data$y)) > 1) + stop("posterior mean is implausible; the fit did not work correctly") + +writeLines("** rstan fit check passed") diff --git a/.github/scripts/tbb-downstream-check.R b/.github/scripts/tbb-downstream-check.R new file mode 100644 index 00000000..517bdd84 --- /dev/null +++ b/.github/scripts/tbb-downstream-check.R @@ -0,0 +1,106 @@ +# Build and load a translation unit exercising the parts of the TBB API that +# downstream packages depend on, using only the flags RcppParallel advertises. +# +# StanHeaders -- and so rstan -- needs both of the pieces used here: +# tbb::this_task_arena::isolate, to keep its thread-local AD tape from being +# modified by a task from another arena, and tbb::task_scheduler_observer, to +# install that tape on each worker. Rtools42's TBB 2017 provides neither in a +# usable form, which is what broke rstan on R 4.2 (isolate is gated behind +# TBB_PREVIEW_TASK_ISOLATION, and isolate_within_arena isn't in the library at +# all), so this stands in for a full rstan build. + +code <- ' +#include +#include +#include +#include + +#include +#include + +// R.h remaps names like \'length\' onto Rf_ equivalents by default, which +// collides with the standard library; keep it last, and unremapped +#define R_NO_REMAP +#include +#include + +namespace { + +// mirrors StanHeaders\' ad_tape_observer, which is what pulls the +// task_scheduler_observer entry points into the link +struct observer : public tbb::task_scheduler_observer { + observer() : tbb::task_scheduler_observer() { observe(true); } + void on_scheduler_entry(bool) override {} + void on_scheduler_exit(bool) override {} +}; + +} // end anonymous namespace + +extern "C" SEXP tbb_downstream_check(void) { + + observer obs; + std::atomic total(0); + + // mirrors StanHeaders\' use of task isolation in reduce_sum / map_rect + tbb::this_task_arena::isolate([&] { + tbb::parallel_for( + tbb::blocked_range(0, 1024), + [&](const tbb::blocked_range& range) { + total += static_cast(range.end() - range.begin()); + } + ); + }); + + return Rf_ScalarInteger(total.load()); + +} +' + +# report how RcppParallel is configured, so a failure here can be read +# against the provenance logged during installation +writeLines(c( + sprintf("TBB_ENABLED : %s", RcppParallel:::TBB_ENABLED), + sprintf("TBB_LIB : '%s'", RcppParallel:::TBB_LIB), + sprintf("TBB_INC : '%s'", RcppParallel:::TBB_INC), + sprintf("CxxFlags() : %s", RcppParallel:::tbbCxxFlags()), + sprintf("LdFlags() : %s", RcppParallel:::tbbLdFlags()) +)) + +if (!RcppParallel:::TBB_ENABLED) + stop("RcppParallel was installed without a tbb backend") + +# mirror rstan's src/Makevars.win, which takes all of its TBB configuration +# from RcppParallel: its compiler flags via CxxFlags(), its linker flags via +# RcppParallelLibs(), and the include path itself via 'LinkingTo' +makevars <- c( + "CXX_STD = CXX17", + sprintf("PKG_CPPFLAGS = -I\"%s\"", system.file("include", package = "RcppParallel")), + "PKG_CPPFLAGS += $(shell \"${R_HOME}/bin/Rscript\" -e \"RcppParallel::CxxFlags()\" | tail -n 1)", + "PKG_LIBS += $(shell \"${R_HOME}/bin${R_ARCH_BIN}/Rscript\" -e \"RcppParallel::RcppParallelLibs()\" | tail -n 1)" +) + +dir <- tempfile("tbb-downstream-") +dir.create(dir, recursive = TRUE) +owd <- setwd(dir) +on.exit(setwd(owd), add = TRUE) + +writeLines(code, "check.cpp") + +# deliberately not named 'Makevars': R CMD SHLIB reads a Makevars in the +# working directory as well as R_MAKEVARS_USER, and would apply both. going +# through R_MAKEVARS_USER also keeps any personal ~/.R/Makevars out of the way +writeLines(makevars, "makevars-downstream") +Sys.setenv(R_MAKEVARS_USER = file.path(dir, "makevars-downstream")) +status <- system(paste(shQuote(file.path(R.home("bin"), "R")), "CMD SHLIB check.cpp")) +if (status != 0L) + stop("downstream translation unit failed to build") + +# loading also proves any load-time dependency on the tbb stub resolves +dll <- dyn.load(paste0("check", .Platform$dynlib.ext)) +on.exit(dyn.unload(dll[["path"]]), add = TRUE) + +result <- .Call("tbb_downstream_check") +if (!identical(result, 1024L)) + stop("downstream check returned ", result, "; expected 1024") + +writeLines("** downstream TBB check passed") diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 8b93c60e..37a47702 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -23,6 +23,9 @@ jobs: - {os: ubuntu-24.04-arm, r: 'release'} - {os: windows-latest, r: 'release'} - {os: windows-11-arm, r: 'release'} + # Rtools42 provides only a pre-oneTBB copy of TBB, so this is the + # one configuration that builds the bundled oneTBB on Windows + - {os: windows-latest, r: '4.2'} env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} @@ -32,6 +35,7 @@ jobs: - uses: r-lib/actions/setup-r@v2 with: + r-version: ${{ matrix.config.r }} use-public-rspm: true - uses: r-lib/actions/setup-r-dependencies@v2 @@ -40,3 +44,41 @@ jobs: needs: check - uses: r-lib/actions/check-r-package@v2 + + # Verify that a package can still compile and link against the TBB API that + # RcppParallel advertises. This is the contract rstan (via StanHeaders) + # depends on, and it is not covered by R CMD check: RcppParallel can install + # perfectly well while leaving downstream packages unable to build. + downstream: + runs-on: windows-latest + + name: downstream (${{ matrix.r }}) + + strategy: + fail-fast: false + matrix: + r: ['4.2', 'release'] + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + VERBOSE: 1 + + # the default shell on Windows runners is PowerShell, which aliases 'r' + # to Invoke-History and so mangles 'R CMD INSTALL' + defaults: + run: + shell: bash + + steps: + - uses: actions/checkout@v3 + + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.r }} + use-public-rspm: true + + - name: install RcppParallel + run: R CMD INSTALL --preclean . + + - name: build against RcppParallel + run: Rscript .github/scripts/tbb-downstream-check.R diff --git a/.github/workflows/rstan.yaml b/.github/workflows/rstan.yaml new file mode 100644 index 00000000..a598e1d1 --- /dev/null +++ b/.github/workflows/rstan.yaml @@ -0,0 +1,96 @@ +# Build rstan from source against this branch's RcppParallel. +# +# The 'downstream' job in R-CMD-check.yaml is a stand-in for this: it compiles a +# small translation unit using the two TBB APIs StanHeaders needs. That catches +# the breakage this workflow exists to rule out, in seconds rather than the best +# part of an hour -- so this is deliberately manual, for when the real thing is +# worth waiting for (before a release, or after touching the Windows TBB setup). +on: + workflow_dispatch: + inputs: + r-version: + description: "R version (4.2 uses Rtools42, which has no oneTBB)" + required: true + default: "4.2" + os: + description: "Runner to build on" + required: true + default: "windows-latest" + type: choice + options: + - windows-latest + - macOS-latest + - ubuntu-latest + fit-model: + description: "Also compile and fit a tiny Stan model (slow)" + required: false + default: false + type: boolean + +name: rstan + +jobs: + rstan: + runs-on: ${{ inputs.os }} + + name: rstan (${{ inputs.os }}, R ${{ inputs.r-version }}) + + # building rstan from source is slow, and slower still on Windows + timeout-minutes: 120 + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + VERBOSE: 1 + # rstan's own guidance for building it from source + MAKEFLAGS: -j2 + + defaults: + run: + shell: bash + + steps: + - uses: actions/checkout@v3 + + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ inputs.r-version }} + use-public-rspm: true + + - name: install RcppParallel from this branch + run: R CMD INSTALL --preclean . + + - name: report how RcppParallel resolved TBB + run: | + Rscript -e 'writeLines(c( + paste("TBB_ENABLED", RcppParallel:::TBB_ENABLED), + paste("TBB_LIB ", RcppParallel:::TBB_LIB), + paste("CxxFlags ", RcppParallel:::tbbCxxFlags()), + paste("LdFlags ", RcppParallel:::tbbLdFlags()) + ))' + + # binaries are fine for everything except rstan and StanHeaders, which are + # the packages that actually consume RcppParallel's flags. this is + # rstan's Imports plus its LinkingTo, minus RcppParallel (installed above) + # and StanHeaders (built from source below) + - name: install rstan's dependencies + run: | + Rscript -e 'install.packages(c("Rcpp","RcppEigen","BH","inline","gridExtra","loo","pkgbuild","QuickJSR","ggplot2"))' + + # dependencies = FALSE so that resolving StanHeaders/rstan can't quietly + # pull CRAN's RcppParallel over the one we just built -- that would turn + # this into a test of the released package instead + - name: install StanHeaders and rstan from source + run: | + Rscript -e 'install.packages(c("StanHeaders","rstan"), type = "source", dependencies = FALSE, INSTALL_opts = "--no-multiarch")' + + - name: check RcppParallel is still the one we built + run: | + Rscript -e 'v <- as.character(packageVersion("RcppParallel")); writeLines(paste("RcppParallel", v)); if (!grepl("[.]9000$", v)) stop("RcppParallel was replaced by a released build; the rstan build did not test this branch")' + + - name: load rstan + run: | + Rscript -e 'library(rstan); print(rstan::stan_version())' + + - name: compile and fit a tiny model + if: ${{ inputs.fit-model }} + run: Rscript .github/scripts/rstan-fit-check.R diff --git a/NEWS.md b/NEWS.md index 4d7f7e89..3a43c7de 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,25 @@ 'random_access_iterator' in namespace 'std'". This affected CRAN's macOS x86_64 machines, which pair Apple clang 14 with the macOS 11.3 SDK. +* On Windows, RcppParallel now only uses the copy of TBB provided by Rtools + when that copy is oneTBB; otherwise, the bundled oneTBB is built from + sources, as it already is on other platforms. Rtools42 provides Intel TBB + 2017, whose headers downstream packages cannot build against: StanHeaders + uses `tbb::this_task_arena::isolate`, which that release still gates behind + `TBB_PREVIEW_TASK_ISOLATION` and does not export from its library. As a + result, rstan could no longer be built on R 4.2 for Windows. + +* On Windows, `RcppParallel::RcppParallelLibs()` once again offers the TBB + stub library, so that packages taking all of their linker flags from it + (e.g. rstan) can resolve TBB symbols which `RcppParallel.dll` does not + itself re-export. + +* Fixed an issue on Windows where a failure to build the TBB stub library + could go unreported, since `R CMD SHLIB` has been seen to exit successfully + even when the link failed. Installation completed, but shipped a package + with no `tbb.dll` -- which packages linking `-ltbb` then discovered only + when they failed to load. Installation now fails instead. + * Fixed an issue where building the bundled oneTBB could fail when `CXX` (or `CC`) was configured with a leading compiler launcher such as `ccache` (e.g. `CXX = "ccache g++"`). The launcher is now forwarded to cmake via diff --git a/R/tbb.R b/R/tbb.R index 152fd163..55437c8c 100644 --- a/R/tbb.R +++ b/R/tbb.R @@ -87,12 +87,25 @@ tbbLdFlags <- function() { # on Windows, we statically link to oneTBB if (is_windows()) { - + libPath <- archSystemFile("libs") ldFlags <- sprintf("-L%s -lRcppParallel", asBuildPath(libPath)) + + # also offer the stub library. RcppParallel.dll re-exports whichever TBB + # objects happened to be pulled out of the static library when it was + # linked, which is an incidental export surface rather than a declared + # one; the stub exports the runtime wholesale, so anything missing from + # RcppParallel.dll can still be resolved. this comes last deliberately: + # the linker satisfies symbols in order, so RcppParallel's own exports + # still win, and no dependency on the stub is recorded unless something + # actually needs it + tbbPath <- archSystemFile("lib") + if (file.exists(file.path(tbbPath, "tbb.dll"))) + ldFlags <- paste(ldFlags, sprintf("-L%s -ltbb", asBuildPath(tbbPath))) + return(ldFlags) - + } # shortcut if TBB_LIB defined diff --git a/patches/legacy_tbb_abi.diff b/patches/legacy_tbb_abi.diff new file mode 100644 index 00000000..9cef9c80 --- /dev/null +++ b/patches/legacy_tbb_abi.diff @@ -0,0 +1,260 @@ +--- a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h ++++ b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h +@@ -17,10 +17,13 @@ + #ifndef __TBB_task_scheduler_observer_H + #define __TBB_task_scheduler_observer_H + ++#include ++ + #include "detail/_namespace_injection.h" ++ + #include "task_arena.h" +-#include + ++ + namespace tbb { + namespace detail { + +@@ -112,5 +115,128 @@ + } + } // namespace tbb + ++ ++// Provided for backwards compatibility. This is a local addition, not part of ++// upstream oneTBB, so the tbb.dll stub built by src/install.libs.R must be ++// able to tell whether the TBB headers in use supply these declarations (the ++// bundled copy does) or whether it has to declare them itself (as with the ++// pristine headers shipped by Rtools). ++#define __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED 1 ++ ++namespace tbb { ++namespace interface6 { ++class task_scheduler_observer; ++} ++namespace internal { ++ ++class task_scheduler_observer_v3 { ++ friend class tbb::detail::r1::observer_proxy; ++ friend class tbb::detail::r1::observer_list; ++ friend class interface6::task_scheduler_observer; ++ ++ //! Pointer to the proxy holding this observer. ++ /** Observers are proxied by the scheduler to maintain persistent lists of them. **/ ++ tbb::detail::r1::observer_proxy* my_proxy; ++ ++ //! Counter preventing the observer from being destroyed while in use by the scheduler. ++ /** Valid only when observation is on. **/ ++ std::atomic my_busy_count; ++ ++public: ++ //! Enable or disable observation ++ /** For local observers the method can be used only when the current thread ++ has the task scheduler initialized or is attached to an arena. ++ ++ Repeated calls with the same state are no-ops. **/ ++ void __TBB_EXPORTED_METHOD observe( bool state=true ); ++ ++ //! Returns true if observation is enabled, false otherwise. ++ bool is_observing() const {return my_proxy!=NULL;} ++ ++ //! Construct observer with observation disabled. ++ task_scheduler_observer_v3() : my_proxy(NULL) { my_busy_count.store(0); } ++ ++ //! Entry notification ++ /** Invoked from inside observe(true) call and whenever a worker enters the arena ++ this observer is associated with. If a thread is already in the arena when ++ the observer is activated, the entry notification is called before it ++ executes the first stolen task. ++ ++ Obsolete semantics. For global observers it is called by a thread before ++ the first steal since observation became enabled. **/ ++ virtual void on_scheduler_entry( bool /*is_worker*/ ) {} ++ ++ //! Exit notification ++ /** Invoked from inside observe(false) call and whenever a worker leaves the ++ arena this observer is associated with. ++ ++ Obsolete semantics. For global observers it is called by a thread before ++ the first steal since observation became enabled. **/ ++ virtual void on_scheduler_exit( bool /*is_worker*/ ) {} ++ ++ //! Destructor automatically switches observation off if it is enabled. ++ virtual ~task_scheduler_observer_v3() { if(my_proxy) observe(false);} ++}; ++ ++} // namespace internal ++ ++namespace interface6 { ++class task_scheduler_observer : public internal::task_scheduler_observer_v3 { ++ friend class internal::task_scheduler_observer_v3; ++ friend class tbb::detail::r1::observer_proxy; ++ friend class tbb::detail::r1::observer_list; ++ ++ /** Negative numbers with the largest absolute value to minimize probability ++ of coincidence in case of a bug in busy count usage. **/ ++ // TODO: take more high bits for version number ++ static const intptr_t v6_trait = (intptr_t)((~(uintptr_t)0 >> 1) + 1); ++ ++ //! contains task_arena pointer or tag indicating local or global semantics of the observer ++ intptr_t my_context_tag; ++ enum { global_tag = 0, implicit_tag = 1 }; ++ ++public: ++ //! Construct local or global observer in inactive state (observation disabled). ++ /** For a local observer entry/exit notifications are invoked whenever a worker ++ thread joins/leaves the arena of the observer's owner thread. If a thread is ++ already in the arena when the observer is activated, the entry notification is ++ called before it executes the first stolen task. **/ ++ /** TODO: Obsolete. ++ Global observer semantics is obsolete as it violates master thread isolation ++ guarantees and is not composable. Thus the current default behavior of the ++ constructor is obsolete too and will be changed in one of the future versions ++ of the library. **/ ++ explicit task_scheduler_observer( bool local = false ) { ++ my_context_tag = local? implicit_tag : global_tag; ++ } ++ ++ //! Construct local observer for a given arena in inactive state (observation disabled). ++ /** entry/exit notifications are invoked whenever a thread joins/leaves arena. ++ If a thread is already in the arena when the observer is activated, the entry notification ++ is called before it executes the first stolen task. **/ ++ explicit task_scheduler_observer( task_arena & a) { ++ my_context_tag = (intptr_t)&a; ++ } ++ ++ /** Destructor protects instance of the observer from concurrent notification. ++ It is recommended to disable observation before destructor of a derived class starts, ++ otherwise it can lead to concurrent notification callback on partly destroyed object **/ ++ virtual ~task_scheduler_observer() { if(my_proxy) observe(false); } ++ ++ //! Enable or disable observation ++ /** Warning: concurrent invocations of this method are not safe. ++ Repeated calls with the same state are no-ops. **/ ++ void observe( bool state=true ) { ++ if( state && !my_proxy ) { ++ __TBB_ASSERT( !my_busy_count, "Inconsistent state of task_scheduler_observer instance"); ++ my_busy_count.store(v6_trait); ++ } ++ internal::task_scheduler_observer_v3::observe(state); ++ } ++}; ++ ++} //namespace interface6 ++ ++} // namespace tbb + + #endif /* __TBB_task_scheduler_observer_H */ +--- a/src/tbb/src/tbb/observer_proxy.cpp ++++ b/src/tbb/src/tbb/observer_proxy.cpp +@@ -317,3 +317,23 @@ + } // namespace r1 + } // namespace detail + } // namespace tbb ++ ++// Not on Windows: there the old ABI is published by the tbb.dll stub built ++// from src/tbb-compat/tbb-compat.cpp, which has to define this itself in ++// order for R CMD SHLIB to pick it up when generating the export list. ++// Defining it here as well makes the stub fail to link with "multiple ++// definition of tbb::internal::task_scheduler_observer_v3::observe". ++#ifndef _WIN32 ++ ++namespace tbb { ++namespace internal { ++ ++void __TBB_EXPORTED_FUNC task_scheduler_observer_v3::observe( bool enable ) { ++ auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); ++ tbb::detail::r1::observe(*tso, enable); ++} ++ ++} // namespace internal ++} // namespace tbb ++ ++#endif /* _WIN32 */ +\ No newline at end of file +--- a/src/tbb/src/tbb/def/lin32-tbb.def ++++ b/src/tbb/src/tbb/def/lin32-tbb.def +@@ -17,6 +17,26 @@ + { + global: + ++/* Needed for backwards compatibility */ ++_ZNSt13runtime_errorD1Ev; ++_ZTISt13runtime_error; ++_ZTSSt13runtime_error; ++_ZNSt16invalid_argumentD1Ev; ++_ZTISt16invalid_argument; ++_ZTSSt16invalid_argument; ++_ZNSt11range_errorD1Ev; ++_ZTISt11range_error; ++_ZTSSt11range_error; ++_ZNSt12length_errorD1Ev; ++_ZTISt12length_error; ++_ZTSSt12length_error; ++_ZNSt12out_of_rangeD1Ev; ++_ZTISt12out_of_range; ++_ZTSSt12out_of_range; ++ ++/* Needed by rstan */ ++_ZN3tbb8internal26task_scheduler_observer_v37observeEb; ++ + /* Assertions (assert.cpp) */ + _ZN3tbb6detail2r117assertion_failureEPKciS3_S3_; + +--- a/src/tbb/src/tbb/def/lin64-tbb.def ++++ b/src/tbb/src/tbb/def/lin64-tbb.def +@@ -17,6 +17,26 @@ + { + global: + ++/* Needed for backwards compatibility */ ++_ZNSt13runtime_errorD1Ev; ++_ZTISt13runtime_error; ++_ZTSSt13runtime_error; ++_ZNSt16invalid_argumentD1Ev; ++_ZTISt16invalid_argument; ++_ZTSSt16invalid_argument; ++_ZNSt11range_errorD1Ev; ++_ZTISt11range_error; ++_ZTSSt11range_error; ++_ZNSt12length_errorD1Ev; ++_ZTISt12length_error; ++_ZTSSt12length_error; ++_ZNSt12out_of_rangeD1Ev; ++_ZTISt12out_of_range; ++_ZTSSt12out_of_range; ++ ++/* Needed by rstan */ ++_ZN3tbb8internal26task_scheduler_observer_v37observeEb; ++ + /* Assertions (assert.cpp) */ + _ZN3tbb6detail2r117assertion_failureEPKciS3_S3_; + +--- a/src/tbb/src/tbb/def/mac64-tbb.def ++++ b/src/tbb/src/tbb/def/mac64-tbb.def +@@ -19,6 +19,26 @@ + # be listed WITHOUT one leading underscore. __TBB_SYMBOL macro should add underscore when + # necessary, depending on the intended usage. + ++# Needed for backwards compatibility ++__ZNSt13runtime_errorD1Ev ++__ZTISt13runtime_error ++__ZTSSt13runtime_error ++__ZNSt16invalid_argumentD1Ev ++__ZTISt16invalid_argument ++__ZTSSt16invalid_argument ++__ZNSt11range_errorD1Ev ++__ZTISt11range_error ++__ZTSSt11range_error ++__ZNSt12length_errorD1Ev ++__ZTISt12length_error ++__ZTSSt12length_error ++__ZNSt12out_of_rangeD1Ev ++__ZTISt12out_of_range ++__ZTSSt12out_of_range ++ ++# Needed by rstan ++__ZN3tbb8internal26task_scheduler_observer_v37observeEb ++ + # Assertions (assert.cpp) + __ZN3tbb6detail2r117assertion_failureEPKciS3_S3_ + diff --git a/patches/mingw_gnu_cmake.diff b/patches/mingw_gnu_cmake.diff new file mode 100644 index 00000000..d0a4e541 --- /dev/null +++ b/patches/mingw_gnu_cmake.diff @@ -0,0 +1,64 @@ +diff --git a/src/tbb/cmake/compilers/GNU.cmake b/src/tbb/cmake/compilers/GNU.cmake +index cf6d8bdb..bc86acc1 100644 +--- a/src/tbb/cmake/compilers/GNU.cmake ++++ b/src/tbb/cmake/compilers/GNU.cmake +@@ -47,19 +47,42 @@ endif() + # >=2.31.1). Capturing the output in CMake can be done like below. The version + # information is written to either stdout or stderr. To not make any + # assumptions, both are captured. ++ ++# The null device is spelled differently on Windows. A mingw build driven by ++# a native Windows CMake (rather than cross-compiled from a POSIX host) has no ++# '/dev/null', and the compiler would fail with "no input files". ++if (WIN32) ++ set(_tbb_null_device "NUL") ++else() ++ set(_tbb_null_device "/dev/null") ++endif() ++ + execute_process( +- COMMAND ${CMAKE_COMMAND} -E env "LANG=C" ${CMAKE_CXX_COMPILER} -xc -c /dev/null -Wa,-v -o/dev/null ++ COMMAND ${CMAKE_COMMAND} -E env "LANG=C" ${CMAKE_CXX_COMPILER} -xc -c ${_tbb_null_device} -Wa,-v -o${_tbb_null_device} + OUTPUT_VARIABLE ASSEMBLER_VERSION_LINE_OUT + ERROR_VARIABLE ASSEMBLER_VERSION_LINE_ERR + OUTPUT_STRIP_TRAILING_WHITESPACE + ERROR_STRIP_TRAILING_WHITESPACE + ) ++unset(_tbb_null_device) + set(ASSEMBLER_VERSION_LINE ${ASSEMBLER_VERSION_LINE_OUT}${ASSEMBLER_VERSION_LINE_ERR}) + string(REGEX REPLACE ".*GNU assembler version ([0-9]+)\\.([0-9]+).*" "\\1" _tbb_gnu_asm_major_version "${ASSEMBLER_VERSION_LINE}") + string(REGEX REPLACE ".*GNU assembler version ([0-9]+)\\.([0-9]+).*" "\\2" _tbb_gnu_asm_minor_version "${ASSEMBLER_VERSION_LINE}") + unset(ASSEMBLER_VERSION_LINE_OUT) + unset(ASSEMBLER_VERSION_LINE_ERR) + unset(ASSEMBLER_VERSION_LINE) ++ ++# When the version can't be parsed, the REGEX REPLACE calls above leave the ++# whole (unmatched) probe output behind, which math() below cannot evaluate. ++# Treat that as an assembler too old for waitpkg rather than failing the ++# configure outright -- the probe is only an optimization guard. ++if (NOT _tbb_gnu_asm_major_version MATCHES "^[0-9]+$" OR ++ NOT _tbb_gnu_asm_minor_version MATCHES "^[0-9]+$") ++ message(STATUS "Could not determine the GNU assembler version; assuming it predates waitpkg support") ++ set(_tbb_gnu_asm_major_version 0) ++ set(_tbb_gnu_asm_minor_version 0) ++endif() ++ + message(TRACE "Extracted GNU assembler version: major=${_tbb_gnu_asm_major_version} minor=${_tbb_gnu_asm_minor_version}") + + math(EXPR _tbb_gnu_asm_version_number "${_tbb_gnu_asm_major_version} * 1000 + ${_tbb_gnu_asm_minor_version}") +@@ -81,7 +104,14 @@ endif() + if (NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL Intel) + # gcc 6.0 and later have -flifetime-dse option that controls elimination of stores done outside the object lifetime + set(TBB_DSE_FLAG $<$>:-flifetime-dse=1>) +- set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) ++ ++ # -fstack-clash-protection is broken on mingw's SEH targets: compiling ++ # task_dispatcher.h makes GCC 10 abort with "internal compiler error: in ++ # seh_emit_stackalloc, at config/i386/winnt.c". It is a hardening flag ++ # rather than a correctness one, so simply leave it off there. ++ if (NOT MINGW) ++ set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) ++ endif() + + # Suppress GCC 12.x-13.x warning here that to_wait_node(n)->my_is_in_list might have size 0 + set(TBB_COMMON_LINK_FLAGS ${TBB_COMMON_LINK_FLAGS} $<$>,$>:-Wno-stringop-overflow>) diff --git a/patches/unversioned_libraries.diff b/patches/unversioned_libraries.diff new file mode 100644 index 00000000..3e16be0f --- /dev/null +++ b/patches/unversioned_libraries.diff @@ -0,0 +1,46 @@ +--- a/src/tbb/src/tbb/CMakeLists.txt ++++ b/src/tbb/src/tbb/CMakeLists.txt +@@ -90,8 +90,6 @@ + # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. + set_target_properties(tbb PROPERTIES + DEFINE_SYMBOL "" +- VERSION ${TBB_BINARY_VERSION}.${TBB_BINARY_MINOR_VERSION} +- SOVERSION ${TBB_BINARY_VERSION} + ) + + tbb_handle_ipo(tbb) +--- a/src/tbb/src/tbbbind/CMakeLists.txt ++++ b/src/tbb/src/tbbbind/CMakeLists.txt +@@ -53,8 +53,6 @@ + # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. + set_target_properties(${TBBBIND_NAME} PROPERTIES + DEFINE_SYMBOL "" +- VERSION ${TBBBIND_BINARY_VERSION}.${TBB_BINARY_MINOR_VERSION} +- SOVERSION ${TBBBIND_BINARY_VERSION} + ) + + tbb_handle_ipo(${TBBBIND_NAME}) +--- a/src/tbb/src/tbbmalloc/CMakeLists.txt ++++ b/src/tbb/src/tbbmalloc/CMakeLists.txt +@@ -70,8 +70,6 @@ + # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. + set_target_properties(tbbmalloc PROPERTIES + DEFINE_SYMBOL "" +- VERSION ${TBBMALLOC_BINARY_VERSION}.${TBB_BINARY_MINOR_VERSION} +- SOVERSION ${TBBMALLOC_BINARY_VERSION} + LINKER_LANGUAGE C + ) + +--- a/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt ++++ b/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt +@@ -54,10 +54,6 @@ + ${TBB_COMMON_COMPILE_FLAGS} + ) + +-set_target_properties(tbbmalloc_proxy PROPERTIES +- VERSION ${TBBMALLOC_BINARY_VERSION}.${TBB_BINARY_MINOR_VERSION} +- SOVERSION ${TBBMALLOC_BINARY_VERSION}) +- + if (UNIX AND NOT APPLE) + # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. + set_target_properties(tbbmalloc_proxy PROPERTIES diff --git a/src/install.libs.R b/src/install.libs.R index fba29d42..6ea15481 100644 --- a/src/install.libs.R +++ b/src/install.libs.R @@ -109,6 +109,12 @@ buildTbbStub <- function(tbbDest) { if (!nzchar(tbbInc)) tbbInc <- TBB_INC + # when TBB was built from the bundled sources there is no TBB_INC to + # consult; its headers were copied into the package's include directory + # during the build, so dispatch on those instead + if (!nzchar(tbbInc)) + tbbInc <- "../inst/include" + if (file.exists(file.path(tbbInc, "oneapi"))) { # with oneTBB, the stub provides the old TBB ABI's @@ -146,7 +152,15 @@ buildTbbStub <- function(tbbDest) { } - file.copy("tbb-compat/tbb.dll", file.path(tbbDest, "tbb.dll")) + # 'R CMD SHLIB' has been seen to report success on Windows even when the + # link failed, which would otherwise leave us shipping a package with no + # stub at all -- and packages linking '-ltbb' only discover that when they + # fail to load. Check for the artifact rather than trusting the exit code. + if (!file.exists("tbb-compat/tbb.dll")) + stop("tbb.dll stub was not produced") + + if (!file.copy("tbb-compat/tbb.dll", file.path(tbbDest, "tbb.dll"))) + stop("couldn't copy tbb.dll stub to '", tbbDest, "'") } @@ -374,6 +388,19 @@ useBundledTbb <- function() { ) } + # on Windows, build TBB as a static library: it then gets linked into + # (and re-exported from) RcppParallel.dll, giving the same layout we + # produce with an Rtools-provided oneTBB. the generator has to be named + # explicitly, as cmake otherwise prefers a Visual Studio generator when + # one happens to be installed + if (.Platform$OS.type == "windows") { + cmakeFlags <- c( + "-G", "MSYS Makefiles", + "-DBUILD_SHARED_LIBS=0", + cmakeFlags + ) + } + writeLines("*** configuring tbb") owd <- setwd("tbb/build-tbb") output <- system2(cmake, shQuote(cmakeFlags), stdout = TRUE, stderr = TRUE) @@ -403,16 +430,14 @@ useBundledTbb <- function() { } setwd(owd) - shlibPattern <- switch( - Sys.info()[["sysname"]], - Windows = "^tbb.*\\.dll$", - Darwin = "^libtbb.*\\.dylib$", + # on Windows (as on wasm) TBB is built as a static library, so collect + # the archives rather than the runtime libraries + shlibPattern <- if (.Platform$OS.type == "windows" || R.version$os == "emscripten") { + "^libtbb.*\\.a$" + } else if (Sys.info()[["sysname"]] == "Darwin") { + "^libtbb.*\\.dylib$" + } else { "^libtbb.*\\.so.*$" - ) - - # WASM only supports static libraries - if (R.version$os == "emscripten") { - shlibPattern <- "^libtbb.*\\.a$" } tbbFiles <- list.files( @@ -534,7 +559,8 @@ args <- commandArgs(trailingOnly = TRUE) if (identical(args, "build")) { if (nzchar(tbbLib) && nzchar(tbbInc)) { useSystemTbb(tbbLib, tbbInc) - } else if (.Platform$OS.type == "windows") { + } else if (.Platform$OS.type == "windows" && !nzchar(Sys.getenv("CMAKE"))) { + # configure found neither a usable TBB nor a cmake to build one with writeLines("** building RcppParallel without tbb backend") } else { useBundledTbb() diff --git a/src/tbb-compat/tbb-compat.cpp b/src/tbb-compat/tbb-compat.cpp index cbde70b4..b5799101 100644 --- a/src/tbb-compat/tbb-compat.cpp +++ b/src/tbb-compat/tbb-compat.cpp @@ -14,6 +14,13 @@ # define DLL_EXPORT #endif +// The old ABI's observer classes, as they stood before oneTBB. The bundled +// copy of oneTBB carries these too (see task_scheduler_observer.h), so only +// declare them when compiling against headers that don't -- i.e. the pristine +// ones Rtools ships. Either way, the out-of-line observe() below is what this +// stub exists to export. +#ifndef __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED + namespace tbb { namespace interface6 { @@ -130,11 +137,13 @@ class task_scheduler_observer : public internal::task_scheduler_observer_v3 { } // namespace tbb +#endif /* __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED */ + namespace tbb { namespace internal { DLL_EXPORT -void __TBB_EXPORTED_FUNC task_scheduler_observer_v3::observe( bool enable ) { +void __TBB_EXPORTED_METHOD task_scheduler_observer_v3::observe( bool enable ) { auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); tbb::detail::r1::observe(*tso, enable); } diff --git a/src/tbb/cmake/compilers/GNU.cmake b/src/tbb/cmake/compilers/GNU.cmake index cf6d8bdb..bc86acc1 100644 --- a/src/tbb/cmake/compilers/GNU.cmake +++ b/src/tbb/cmake/compilers/GNU.cmake @@ -47,19 +47,42 @@ endif() # >=2.31.1). Capturing the output in CMake can be done like below. The version # information is written to either stdout or stderr. To not make any # assumptions, both are captured. + +# The null device is spelled differently on Windows. A mingw build driven by +# a native Windows CMake (rather than cross-compiled from a POSIX host) has no +# '/dev/null', and the compiler would fail with "no input files". +if (WIN32) + set(_tbb_null_device "NUL") +else() + set(_tbb_null_device "/dev/null") +endif() + execute_process( - COMMAND ${CMAKE_COMMAND} -E env "LANG=C" ${CMAKE_CXX_COMPILER} -xc -c /dev/null -Wa,-v -o/dev/null + COMMAND ${CMAKE_COMMAND} -E env "LANG=C" ${CMAKE_CXX_COMPILER} -xc -c ${_tbb_null_device} -Wa,-v -o${_tbb_null_device} OUTPUT_VARIABLE ASSEMBLER_VERSION_LINE_OUT ERROR_VARIABLE ASSEMBLER_VERSION_LINE_ERR OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_STRIP_TRAILING_WHITESPACE ) +unset(_tbb_null_device) set(ASSEMBLER_VERSION_LINE ${ASSEMBLER_VERSION_LINE_OUT}${ASSEMBLER_VERSION_LINE_ERR}) string(REGEX REPLACE ".*GNU assembler version ([0-9]+)\\.([0-9]+).*" "\\1" _tbb_gnu_asm_major_version "${ASSEMBLER_VERSION_LINE}") string(REGEX REPLACE ".*GNU assembler version ([0-9]+)\\.([0-9]+).*" "\\2" _tbb_gnu_asm_minor_version "${ASSEMBLER_VERSION_LINE}") unset(ASSEMBLER_VERSION_LINE_OUT) unset(ASSEMBLER_VERSION_LINE_ERR) unset(ASSEMBLER_VERSION_LINE) + +# When the version can't be parsed, the REGEX REPLACE calls above leave the +# whole (unmatched) probe output behind, which math() below cannot evaluate. +# Treat that as an assembler too old for waitpkg rather than failing the +# configure outright -- the probe is only an optimization guard. +if (NOT _tbb_gnu_asm_major_version MATCHES "^[0-9]+$" OR + NOT _tbb_gnu_asm_minor_version MATCHES "^[0-9]+$") + message(STATUS "Could not determine the GNU assembler version; assuming it predates waitpkg support") + set(_tbb_gnu_asm_major_version 0) + set(_tbb_gnu_asm_minor_version 0) +endif() + message(TRACE "Extracted GNU assembler version: major=${_tbb_gnu_asm_major_version} minor=${_tbb_gnu_asm_minor_version}") math(EXPR _tbb_gnu_asm_version_number "${_tbb_gnu_asm_major_version} * 1000 + ${_tbb_gnu_asm_minor_version}") @@ -81,7 +104,14 @@ endif() if (NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL Intel) # gcc 6.0 and later have -flifetime-dse option that controls elimination of stores done outside the object lifetime set(TBB_DSE_FLAG $<$>:-flifetime-dse=1>) - set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) + + # -fstack-clash-protection is broken on mingw's SEH targets: compiling + # task_dispatcher.h makes GCC 10 abort with "internal compiler error: in + # seh_emit_stackalloc, at config/i386/winnt.c". It is a hardening flag + # rather than a correctness one, so simply leave it off there. + if (NOT MINGW) + set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) + endif() # Suppress GCC 12.x-13.x warning here that to_wait_node(n)->my_is_in_list might have size 0 set(TBB_COMMON_LINK_FLAGS ${TBB_COMMON_LINK_FLAGS} $<$>,$>:-Wno-stringop-overflow>) diff --git a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h index f4f91e96..86e141a7 100644 --- a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h +++ b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h @@ -116,7 +116,13 @@ inline namespace v1 { } // namespace tbb -// Provided for backwards compatibility. +// Provided for backwards compatibility. This is a local addition, not part of +// upstream oneTBB, so the tbb.dll stub built by src/install.libs.R must be +// able to tell whether the TBB headers in use supply these declarations (the +// bundled copy does) or whether it has to declare them itself (as with the +// pristine headers shipped by Rtools). +#define __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED 1 + namespace tbb { namespace interface6 { class task_scheduler_observer; diff --git a/src/tbb/src/tbb/observer_proxy.cpp b/src/tbb/src/tbb/observer_proxy.cpp index 012c9e4a..90389f66 100644 --- a/src/tbb/src/tbb/observer_proxy.cpp +++ b/src/tbb/src/tbb/observer_proxy.cpp @@ -318,6 +318,13 @@ void __TBB_EXPORTED_FUNC observe(d1::task_scheduler_observer &tso, bool enable) } // namespace detail } // namespace tbb +// Not on Windows: there the old ABI is published by the tbb.dll stub built +// from src/tbb-compat/tbb-compat.cpp, which has to define this itself in +// order for R CMD SHLIB to pick it up when generating the export list. +// Defining it here as well makes the stub fail to link with "multiple +// definition of tbb::internal::task_scheduler_observer_v3::observe". +#ifndef _WIN32 + namespace tbb { namespace internal { @@ -327,4 +334,6 @@ void __TBB_EXPORTED_FUNC task_scheduler_observer_v3::observe( bool enable ) { } } // namespace internal -} // namespace tbb \ No newline at end of file +} // namespace tbb + +#endif /* _WIN32 */ \ No newline at end of file diff --git a/tools/config/configure.R b/tools/config/configure.R index c3842f9f..03c07c88 100644 --- a/tools/config/configure.R +++ b/tools/config/configure.R @@ -26,22 +26,30 @@ if (file.exists(makevars)) { } } -# on Windows, check for Rtools; if it exists, and we have tbb, use it +# on Windows, check for Rtools; if it exists, and it provides oneTBB, use it. +# +# older toolchains are deliberately passed over even though they do provide +# a TBB. Rtools42 ships TBB 2017, whose headers downstream packages cannot +# build against: StanHeaders uses tbb::this_task_arena::isolate, which that +# release still gates behind TBB_PREVIEW_TASK_ISOLATION, and its library +# doesn't export isolate_within_arena either, so there is nothing to link +# against even if the declaration were forced into view. we build the +# bundled copy of oneTBB in that case instead if (.Platform$OS.type == "windows") { - + gccPath <- normalizePath(Sys.which("gcc"), winslash = "/") - + tbbLib <- Sys.getenv("TBB_LIB", unset = NA) if (is.na(tbbLib)) tbbLib <- normalizePath(file.path(gccPath, "../../lib"), winslash = "/") - + tbbInc <- Sys.getenv("TBB_INC", unset = NA) if (is.na(tbbInc)) tbbInc <- normalizePath(file.path(gccPath, "../../include"), winslash = "/") - + tbbFiles <- list.files(tbbLib, pattern = "^libtbb") - if (length(tbbFiles)) { - + if (length(tbbFiles) && file.exists(file.path(tbbInc, "oneapi"))) { + tbbPattern <- "^lib(tbb\\d*(?:_static)?)\\.a$" tbbName <- grep(tbbPattern, tbbFiles, perl = TRUE, value = TRUE) tbbName <- gsub(tbbPattern, "\\1", tbbName, perl = TRUE) @@ -137,6 +145,65 @@ if (tryAutoDetect) { } +# if we didn't find a TBB to use, we'll build the bundled copy from +# sources -- which requires cmake. on Windows, a missing or too-old cmake +# is not fatal: toolchains that can't build oneTBB (e.g. Rtools40, which +# provides neither cmake nor a TBB) fall back to tinythread instead +define(CMAKE = "") +useBundledTbb <- FALSE + +if (is.na(tbbLib)) { + + cmake <- local({ + + # check for envvar + cmake <- Sys.getenv("CMAKE", unset = NA) + if (!is.na(cmake)) + return(cmake) + + # check for path + cmake <- Sys.which("cmake") + if (nzchar(cmake)) + return(cmake) + + # check for macOS cmake + cmake <- "/Applications/CMake.app/Contents/bin/cmake" + if (file.exists(cmake)) + return(cmake) + + NA + + }) + + # make sure we have an appropriate version of cmake installed + # (use the resolved path; cmake may not be on the PATH) + cmakeVersion <- if (!is.na(cmake)) { + output <- system(paste(shQuote(cmake), "--version"), intern = TRUE)[[1L]] + numeric_version(sub("cmake version ", "", output)) + } + + useBundledTbb <- !is.na(cmake) && cmakeVersion >= "3.5" + + if (useBundledTbb) { + define(CMAKE = cmake) + } else if (.Platform$OS.type != "windows") { + if (is.na(cmake)) + stop("cmake was not found") + stop("error: RcppParallel requires cmake (>= 3.6); you have ", cmakeVersion) + } else if (is.na(cmake)) { + writeLines("cmake was not found; building RcppParallel without a tbb backend") + } else { + fmt <- "cmake %s is too old (need >= 3.5); building RcppParallel without a tbb backend" + writeLines(sprintf(fmt, cmakeVersion)) + } + +} + +# oneTBB appends its binary version to the library name on Windows, so the +# bundled build produces 'libtbb12.a' rather than 'libtbb.a' there +if (.Platform$OS.type == "windows" && useBundledTbb) + tbbName <- Sys.getenv("TBB_NAME", unset = "tbb12") + # now, define TBB_LIB and TBB_INC as appropriate define( TBB_LIB = if (!is.na(tbbLib)) tbbLib else "", @@ -178,6 +245,20 @@ pkgLibs <- if (.Platform$OS.type == "windows") { "-l$(TBB_MALLOC_NAME)" ) + } else if (useBundledTbb) { + + # the bundled oneTBB is built as a static library on Windows, so + # RcppParallel.dll takes exactly the same shape as it does with an + # Rtools oneTBB above -- see that branch for why tbbmalloc is linked + # wholesale, and why it must precede tbb + c( + "-Wl,-Ltbb/build/lib_release", + "-Wl,--whole-archive", + "-l$(TBB_MALLOC_NAME)", + "-Wl,--no-whole-archive", + "-l$(TBB_NAME)" + ) + } } else if (!is.na(tbbLib)) { @@ -218,45 +299,6 @@ if (.Platform$OS.type == "windows") { define(PKG_LIBS = paste(pkgLibs, collapse = " ")) -# if we're going to build tbb from sources, check for cmake -# (not required on Windows, where the bundled TBB is never built; -# without an Rtools TBB, the tinythread fallback is used instead) -define(CMAKE = "") -if (is.na(tbbLib) && .Platform$OS.type != "windows") { - - cmake <- local({ - - # check for envvar - cmake <- Sys.getenv("CMAKE", unset = NA) - if (!is.na(cmake)) - return(cmake) - - # check for path - cmake <- Sys.which("cmake") - if (nzchar(cmake)) - return(cmake) - - # check for macOS cmake - cmake <- "/Applications/CMake.app/Contents/bin/cmake" - if (file.exists(cmake)) - return(cmake) - - stop("cmake was not found") - - }) - - # make sure we have an appropriate version of cmake installed - # (use the resolved path; cmake may not be on the PATH) - output <- system(paste(shQuote(cmake), "--version"), intern = TRUE)[[1L]] - cmakeVersion <- numeric_version(sub("cmake version ", "", output)) - if (cmakeVersion < "3.5") { - stop("error: RcppParallel requires cmake (>= 3.6); you have ", cmakeVersion) - } - - define(CMAKE = cmake) - -} - # now, set up PKG_CPPFLAGS if (!is.na(tbbLib)) { @@ -266,7 +308,7 @@ if (!is.na(tbbLib)) { } # PKG_CXXFLAGS -if (.Platform$OS.type == "windows" && is.na(tbbLib)) { +if (.Platform$OS.type == "windows" && is.na(tbbLib) && !useBundledTbb) { define(TBB_ENABLED = FALSE) define(PKG_CXXFLAGS = "-DRCPP_PARALLEL_USE_TBB=0") } else {