Skip to content

alternate openmp plugin changes #1414

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

Merged
eddelbuettel merged 3 commits into from
Dec 3, 2025
Merged

alternate openmp plugin changes #1414

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da6266a
alternate openmp plugin changes
kevinushey Dec 2, 2025
9ad19e6
alternative using R CMD SHLIB
kevinushey Dec 2, 2025
349a3dc
Merge branch 'master' into feature/macos-openmp-plugin-tweaks
eddelbuettel Dec 3, 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
4 changes: 4 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
2025-12-01 Kevin Ushey <kevinushey@gmail.com>

* R/Attributes.R: Update OpenMP plugin for macOS

2025-11-24 Dirk Eddelbuettel <edd@debian.org>

* inst/include/Rcpp/r/check_r_headers.h: Add RCPP_NO_R_HEADERS_CHECK
Expand Down
63 changes: 60 additions & 3 deletions R/Attributes.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,67 @@ compileAttributes <- function(pkgdir = ".", verbose = getOption("verbose")) {
list(env = list(PKG_CXXFLAGS ="-std=c++2b"))
}

.openmpPluginDefault <- function() {
list(env = list(PKG_CXXFLAGS = "-fopenmp", PKG_LIBS = "-fopenmp"))
}

.openmpPluginDarwin <- function() {

# get the C++ compiler from R
r <- file.path(R.home("bin"), "R")
cxx <- tryCatch(
system2(r, c("CMD", "config", "CXX"), stdout = TRUE),
Copy link
Contributor Author

@kevinushey kevinushey Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, this might not be correct if an alternate plugin causes a different variant of the C++ compiler to be used (e.g. cpp11 will use whatever compiler is registered for CXX11, which in theory might not match CXX). Is this worth worrying about?

Copy link
Member

@eddelbuettel eddelbuettel Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dear. The baker's dozen of CXX11 to CXX26. Good question. On the other hand if we cannot trust R to tell it about its CXX, who can we trust?

FWIW for me R CMD config CXX and R CMD config CXX17 return the same. [ Checks. ] But that is due to my personal .R/Makevars.

Copy link
Contributor Author

@kevinushey kevinushey Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it'd be odd to have one compiler for CXX11, and another for CXX14 -- doubly so on macOS. But lots of people will configure CXX without thinking to configure all of the other versioned CXX variants...

But I now remembered we can get closer using R CMD SHLIB, which lets R choose the appropriate C++ compiler based on whatever environment variables / user Makevars configuration is set, so maybe that's more appropriate. I just made a change to that effect.

Oof. This PR was supposed to be simple!

Copy link
Member

@eddelbuettel eddelbuettel Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing in life ever is provided you look closely enough....

condition = identity
)
if (inherits(cxx, "condition"))
return(.openmpPluginDefault())

# check the compiler version
command <- paste(cxx, "--version")
version <- tryCatch(
system(command, intern = TRUE),
condition = identity
)
if (inherits(version, "condition"))
return(.openmpPluginDefault())

# if we're using Apple clang, use alternate flags
# assume libomp was installed following https://mac.r-project.org/openmp/
if (any(grepl("Apple clang", version))) {

cxxflags <- "-Xclang -fopenmp"
libs <- "-lomp"

}

# if we're using Homebrew clang, add in libomp include paths
else if (any(grepl("Homebrew clang", version))) {

# ensure Homebrew paths are included during compilation
machine <- Sys.info()[["machine"]]
prefix <- if (machine == "arm64") "/opt/homebrew" else "/usr/local"

# build flags
cxxflags <- sprintf("-I%s/opt/libomp/include -fopenmp", prefix)
Copy link
Contributor Author

@kevinushey kevinushey Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that the version of LLVM clang provided in Homebrew accepts -fopenmp as-is, but I'm adding the library paths for libomp just in case.

libs <- sprintf("-L%s/opt/libomp/lib -fopenmp", prefix)

# otherwise, use default -fopenmp flags for other compilers (LLVM clang; gcc)
} else {

cxxflags <- "-fopenmp"
libs <- "-fopenmp"

}

list(env = list(PKG_CXXFLAGS = cxxflags, PKG_LIBS = libs))

}

## built-in OpenMP plugin
.plugins[["openmp"]] <- function() {
list(env = list(PKG_CXXFLAGS="-fopenmp",
PKG_LIBS="-fopenmp"))
.plugins[["openmp"]] <- if (Sys.info()[["sysname"]] == "Darwin") {
Copy link
Contributor

@coatless coatless Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how the detection simplifies the function definition on load.

.openmpPluginDarwin
} else {
.openmpPluginDefault
}

.plugins[["unwindProtect"]] <- function() { # nocov start
Expand Down