Skip to content

Commit 830f4ed

Browse files
committed
build the bundled onetbb on windows when rtools has no onetbb
Rtools42 ships Intel TBB 2017, and configure adopted it purely on the presence of a libtbb* archive. Downstream packages then compiled against those headers, which is not viable: StanHeaders uses tbb::this_task_arena::isolate unconditionally, TBB 2017 still gates it behind TBB_PREVIEW_TASK_ISOLATION, and its library doesn't export isolate_within_arena, so forcing the declaration into view wouldn't link either. rstan 2.32.7 consequently stopped building on R 4.2 / Windows. Only adopt the Rtools TBB when it is oneTBB, and otherwise build the bundled copy from sources, as we already do elsewhere. Rtools42 provides cmake 3.24.1 in the same directory as gcc, so this needs no new tooling. It is built as a static library so that RcppParallel.dll takes exactly the shape it does with an Rtools oneTBB -- oneTBB marks its entry points __declspec(dllexport) on mingw, so the whole-archive re-export of tbbmalloc carries over unchanged. Toolchains that can build neither (e.g. Rtools40, which has no TBB and no cmake) still fall back to tinythread: a missing or too-old cmake is only an error off Windows, where the bundled build is the sole option. Also add CI covering this. R CMD check gains an R 4.2 Windows job, and a new 'downstream' job compiles and loads a translation unit against the flags RcppParallel advertises, exercising both this_task_arena::isolate and task_scheduler_observer. R CMD check alone would not have caught the rstan breakage: RcppParallel installed perfectly well throughout.
1 parent 0e0e0c1 commit 830f4ed

4 files changed

Lines changed: 259 additions & 57 deletions

File tree

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# Build and load a translation unit exercising the parts of the TBB API that
2+
# downstream packages depend on, using only the flags RcppParallel advertises.
3+
#
4+
# StanHeaders -- and so rstan -- needs both of the pieces used here:
5+
# tbb::this_task_arena::isolate, to keep its thread-local AD tape from being
6+
# modified by a task from another arena, and tbb::task_scheduler_observer, to
7+
# install that tape on each worker. Rtools42's TBB 2017 provides neither in a
8+
# usable form, which is what broke rstan on R 4.2 (isolate is gated behind
9+
# TBB_PREVIEW_TASK_ISOLATION, and isolate_within_arena isn't in the library at
10+
# all), so this stands in for a full rstan build.
11+
12+
code <- '
13+
#include <tbb/blocked_range.h>
14+
#include <tbb/parallel_for.h>
15+
#include <tbb/task_arena.h>
16+
#include <tbb/task_scheduler_observer.h>
17+
18+
#include <atomic>
19+
#include <cstddef>
20+
21+
// R.h remaps names like \'length\' onto Rf_ equivalents by default, which
22+
// collides with the standard library; keep it last, and unremapped
23+
#define R_NO_REMAP
24+
#include <R.h>
25+
#include <Rinternals.h>
26+
27+
namespace {
28+
29+
// mirrors StanHeaders\' ad_tape_observer, which is what pulls the
30+
// task_scheduler_observer entry points into the link
31+
struct observer : public tbb::task_scheduler_observer {
32+
observer() : tbb::task_scheduler_observer() { observe(true); }
33+
void on_scheduler_entry(bool) override {}
34+
void on_scheduler_exit(bool) override {}
35+
};
36+
37+
} // end anonymous namespace
38+
39+
extern "C" SEXP tbb_downstream_check(void) {
40+
41+
observer obs;
42+
std::atomic<int> total(0);
43+
44+
// mirrors StanHeaders\' use of task isolation in reduce_sum / map_rect
45+
tbb::this_task_arena::isolate([&] {
46+
tbb::parallel_for(
47+
tbb::blocked_range<std::size_t>(0, 1024),
48+
[&](const tbb::blocked_range<std::size_t>& range) {
49+
total += static_cast<int>(range.end() - range.begin());
50+
}
51+
);
52+
});
53+
54+
return Rf_ScalarInteger(total.load());
55+
56+
}
57+
'
58+
59+
# report how RcppParallel is configured, so a failure here can be read
60+
# against the provenance logged during installation
61+
writeLines(c(
62+
sprintf("TBB_ENABLED : %s", RcppParallel:::TBB_ENABLED),
63+
sprintf("TBB_LIB : '%s'", RcppParallel:::TBB_LIB),
64+
sprintf("TBB_INC : '%s'", RcppParallel:::TBB_INC),
65+
sprintf("CxxFlags() : %s", RcppParallel:::tbbCxxFlags()),
66+
sprintf("LdFlags() : %s", RcppParallel:::tbbLdFlags())
67+
))
68+
69+
if (!RcppParallel:::TBB_ENABLED)
70+
stop("RcppParallel was installed without a tbb backend")
71+
72+
# mirror rstan's src/Makevars.win, which takes all of its TBB configuration
73+
# from RcppParallel: its compiler flags via CxxFlags(), its linker flags via
74+
# RcppParallelLibs(), and the include path itself via 'LinkingTo'
75+
makevars <- c(
76+
"CXX_STD = CXX17",
77+
sprintf("PKG_CPPFLAGS = -I\"%s\"", system.file("include", package = "RcppParallel")),
78+
"PKG_CPPFLAGS += $(shell \"${R_HOME}/bin/Rscript\" -e \"RcppParallel::CxxFlags()\" | tail -n 1)",
79+
"PKG_LIBS += $(shell \"${R_HOME}/bin${R_ARCH_BIN}/Rscript\" -e \"RcppParallel::RcppParallelLibs()\" | tail -n 1)"
80+
)
81+
82+
dir <- tempfile("tbb-downstream-")
83+
dir.create(dir, recursive = TRUE)
84+
owd <- setwd(dir)
85+
on.exit(setwd(owd), add = TRUE)
86+
87+
writeLines(code, "check.cpp")
88+
89+
# deliberately not named 'Makevars': R CMD SHLIB reads a Makevars in the
90+
# working directory as well as R_MAKEVARS_USER, and would apply both. going
91+
# through R_MAKEVARS_USER also keeps any personal ~/.R/Makevars out of the way
92+
writeLines(makevars, "makevars-downstream")
93+
Sys.setenv(R_MAKEVARS_USER = file.path(dir, "makevars-downstream"))
94+
status <- system(paste(shQuote(file.path(R.home("bin"), "R")), "CMD SHLIB check.cpp"))
95+
if (status != 0L)
96+
stop("downstream translation unit failed to build")
97+
98+
# loading also proves any load-time dependency on the tbb stub resolves
99+
dll <- dyn.load(paste0("check", .Platform$dynlib.ext))
100+
on.exit(dyn.unload(dll[["path"]]), add = TRUE)
101+
102+
result <- .Call("tbb_downstream_check")
103+
if (!identical(result, 1024L))
104+
stop("downstream check returned ", result, "; expected 1024")
105+
106+
writeLines("** downstream TBB check passed")

.github/workflows/R-CMD-check.yaml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ jobs:
2323
- {os: ubuntu-24.04-arm, r: 'release'}
2424
- {os: windows-latest, r: 'release'}
2525
- {os: windows-11-arm, r: 'release'}
26+
# Rtools42 provides only a pre-oneTBB copy of TBB, so this is the
27+
# one configuration that builds the bundled oneTBB on Windows
28+
- {os: windows-latest, r: '4.2'}
2629

2730
env:
2831
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
@@ -32,6 +35,7 @@ jobs:
3235

3336
- uses: r-lib/actions/setup-r@v2
3437
with:
38+
r-version: ${{ matrix.config.r }}
3539
use-public-rspm: true
3640

3741
- uses: r-lib/actions/setup-r-dependencies@v2
@@ -40,3 +44,35 @@ jobs:
4044
needs: check
4145

4246
- uses: r-lib/actions/check-r-package@v2
47+
48+
# Verify that a package can still compile and link against the TBB API that
49+
# RcppParallel advertises. This is the contract rstan (via StanHeaders)
50+
# depends on, and it is not covered by R CMD check: RcppParallel can install
51+
# perfectly well while leaving downstream packages unable to build.
52+
downstream:
53+
runs-on: windows-latest
54+
55+
name: downstream (${{ matrix.r }})
56+
57+
strategy:
58+
fail-fast: false
59+
matrix:
60+
r: ['4.2', 'release']
61+
62+
env:
63+
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
64+
VERBOSE: 1
65+
66+
steps:
67+
- uses: actions/checkout@v3
68+
69+
- uses: r-lib/actions/setup-r@v2
70+
with:
71+
r-version: ${{ matrix.r }}
72+
use-public-rspm: true
73+
74+
- name: install RcppParallel
75+
run: R CMD INSTALL --preclean .
76+
77+
- name: build against RcppParallel
78+
run: Rscript .github/scripts/tbb-downstream-check.R

src/install.libs.R

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,12 @@ buildTbbStub <- function(tbbDest) {
109109
if (!nzchar(tbbInc))
110110
tbbInc <- TBB_INC
111111

112+
# when TBB was built from the bundled sources there is no TBB_INC to
113+
# consult; its headers were copied into the package's include directory
114+
# during the build, so dispatch on those instead
115+
if (!nzchar(tbbInc))
116+
tbbInc <- "../inst/include"
117+
112118
if (file.exists(file.path(tbbInc, "oneapi"))) {
113119

114120
# with oneTBB, the stub provides the old TBB ABI's
@@ -374,6 +380,19 @@ useBundledTbb <- function() {
374380
)
375381
}
376382

383+
# on Windows, build TBB as a static library: it then gets linked into
384+
# (and re-exported from) RcppParallel.dll, giving the same layout we
385+
# produce with an Rtools-provided oneTBB. the generator has to be named
386+
# explicitly, as cmake otherwise prefers a Visual Studio generator when
387+
# one happens to be installed
388+
if (.Platform$OS.type == "windows") {
389+
cmakeFlags <- c(
390+
"-G", "MSYS Makefiles",
391+
"-DBUILD_SHARED_LIBS=0",
392+
cmakeFlags
393+
)
394+
}
395+
377396
writeLines("*** configuring tbb")
378397
owd <- setwd("tbb/build-tbb")
379398
output <- system2(cmake, shQuote(cmakeFlags), stdout = TRUE, stderr = TRUE)
@@ -403,16 +422,14 @@ useBundledTbb <- function() {
403422
}
404423
setwd(owd)
405424

406-
shlibPattern <- switch(
407-
Sys.info()[["sysname"]],
408-
Windows = "^tbb.*\\.dll$",
409-
Darwin = "^libtbb.*\\.dylib$",
425+
# on Windows (as on wasm) TBB is built as a static library, so collect
426+
# the archives rather than the runtime libraries
427+
shlibPattern <- if (.Platform$OS.type == "windows" || R.version$os == "emscripten") {
428+
"^libtbb.*\\.a$"
429+
} else if (Sys.info()[["sysname"]] == "Darwin") {
430+
"^libtbb.*\\.dylib$"
431+
} else {
410432
"^libtbb.*\\.so.*$"
411-
)
412-
413-
# WASM only supports static libraries
414-
if (R.version$os == "emscripten") {
415-
shlibPattern <- "^libtbb.*\\.a$"
416433
}
417434

418435
tbbFiles <- list.files(
@@ -534,7 +551,8 @@ args <- commandArgs(trailingOnly = TRUE)
534551
if (identical(args, "build")) {
535552
if (nzchar(tbbLib) && nzchar(tbbInc)) {
536553
useSystemTbb(tbbLib, tbbInc)
537-
} else if (.Platform$OS.type == "windows") {
554+
} else if (.Platform$OS.type == "windows" && !nzchar(Sys.getenv("CMAKE"))) {
555+
# configure found neither a usable TBB nor a cmake to build one with
538556
writeLines("** building RcppParallel without tbb backend")
539557
} else {
540558
useBundledTbb()

0 commit comments

Comments
 (0)