From 46261608a720f89b7a92ab9580707ac7f6842c30 Mon Sep 17 00:00:00 2001 From: Jim Hester Date: Mon, 21 Oct 2019 09:54:45 -0400 Subject: [PATCH 1/4] Detect OpenMP support This detects support for OpenMP and only includes the compiler flags for it if OpenMP is supported. This solves the common case on macOS, because R on macOS ships with `-fopenmp` in SHLIB_OPENMP_CFLAGS, as the CRAN compiler toolchain _does_ support OpenMP, however the XCode toolchain distributed by Apple _does not_ support OpenMP, which causes data.table to fail to compile from source. Fixes #2161 --- .Rbuildignore | 1 + .gitignore | 1 + NEWS.md | 4 ++++ cleanup | 2 ++ configure | 19 ++++++++++++++++++- src/{Makevars => Makevars.in} | 7 ++----- src/Makevars.win | 5 +++++ 7 files changed, 33 insertions(+), 6 deletions(-) create mode 100755 cleanup rename src/{Makevars => Makevars.in} (70%) create mode 100644 src/Makevars.win diff --git a/.Rbuildignore b/.Rbuildignore index a3193d4f25..2b3483fa0e 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -15,6 +15,7 @@ ^NEWS\.0\.md$ ^README\.md$ ^_pkgdown\.yml$ +^src/Makevars$ ^\.RData$ ^\.Rhistory$ diff --git a/.gitignore b/.gitignore index ce159ee5f6..35a25bc087 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ Rplots.pdf *-Ex.R data.table_*.tar.gz data.table.Rcheck +src/Makevars # Emacs IDE files .emacs.desktop diff --git a/NEWS.md b/NEWS.md index f53b8fa6e7..2de1b0e1c0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,10 @@ ## NEW FEATURES +* Compiler support for OpenMP is now detected during installation, which allows + data.table to compile even if the users' toolchain differs from CRANs, as is + common on macOS. (#2161, @jimhester) + ## BUG FIXES ## NOTES diff --git a/cleanup b/cleanup new file mode 100755 index 0000000000..3c020d3881 --- /dev/null +++ b/cleanup @@ -0,0 +1,2 @@ +#!/bin/sh +rm -f src/Makevars diff --git a/configure b/configure index 96fc844a18..38da46e0f8 100755 --- a/configure +++ b/configure @@ -50,5 +50,22 @@ fi version=`pkg-config --modversion zlib` echo "zlib ${version} is available ok" -exit 0 +# Find R compilers +CC=`${R_HOME}/bin/R CMD config CC` +CFLAGS=`${R_HOME}/bin/R CMD config CFLAGS` +CPPFLAGS=`${R_HOME}/bin/R CMD config CPPFLAGS` + +# Test if we have a OPENMP compatible compiler +echo "#include \nint main () { return omp_get_num_threads (); }" | ${CC} ${CPPFLAGS} ${PKG_CFLAGS} ${CFLAGS} ${SHLIB_OPENMP_CFLAGS} -E -xc - >/dev/null 2>&1 || R_NO_OPENMP=1; + +# Write to Makevars +if [ $R_NO_OPENMP ]; then + sed -e "s|@openmp_cflags@||" src/Makevars.in > src/Makevars +else + echo "OpenMP supported" + sed -e "s|@openmp_cflags@|\$(SHLIB_OPENMP_CFLAGS)|" src/Makevars.in > src/Makevars +fi + + +exit 0 diff --git a/src/Makevars b/src/Makevars.in similarity index 70% rename from src/Makevars rename to src/Makevars.in index e8e586004c..8f20d70560 100644 --- a/src/Makevars +++ b/src/Makevars.in @@ -1,9 +1,6 @@ - -PKG_CFLAGS = $(SHLIB_OPENMP_CFLAGS) -PKG_LIBS = $(SHLIB_OPENMP_CFLAGS) -lz +PKG_CFLAGS = @openmp_cflags@ +PKG_LIBS = @openmp_cflags@ -lz all: $(SHLIB) mv $(SHLIB) datatable$(SHLIB_EXT) if [ "$(OS)" != "Windows_NT" ] && [ `uname -s` = 'Darwin' ]; then install_name_tool -id datatable$(SHLIB_EXT) datatable$(SHLIB_EXT); fi - - diff --git a/src/Makevars.win b/src/Makevars.win new file mode 100644 index 0000000000..3ea29da12d --- /dev/null +++ b/src/Makevars.win @@ -0,0 +1,5 @@ +PKG_CFLAGS = $(SHLIB_OPENMP_CFLAGS) +PKG_LIBS = $(SHLIB_OPENMP_CFLAGS) -lz + +all: $(SHLIB) + mv $(SHLIB) datatable$(SHLIB_EXT) From 7eb7e749630c8c56fc7ca7650474876578dd0831 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 21 Oct 2019 22:34:08 +0800 Subject: [PATCH 2/4] standardized NEWS item --- NEWS.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 2de1b0e1c0..0817734ae2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,9 +6,7 @@ ## NEW FEATURES -* Compiler support for OpenMP is now detected during installation, which allows - data.table to compile even if the users' toolchain differs from CRANs, as is - common on macOS. (#2161, @jimhester) +* Compiler support for OpenMP is now detected during installation, which allows data.table to compile even if the users' toolchain differs from CRANs, as is common on macOS, [#2161](https://github.com/Rdatatable/data.table/issues/2161). Thanks @jimhesterm for the PR. ## BUG FIXES From 576c8526613e33e1955aed2fa9fec9491f5c2fa1 Mon Sep 17 00:00:00 2001 From: Jim Hester Date: Thu, 24 Oct 2019 15:14:01 -0400 Subject: [PATCH 3/4] Add message if OpenMP is not supported --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 38da46e0f8..53a3925f50 100755 --- a/configure +++ b/configure @@ -61,6 +61,7 @@ echo "#include \nint main () { return omp_get_num_threads (); }" | ${CC} # Write to Makevars if [ $R_NO_OPENMP ]; then + echo "OpenMP not supported!\nFor details on how to install the neessesary toolchains on your OS see:\n https://github.com/Rdatatable/data.table/wiki/Installation " sed -e "s|@openmp_cflags@||" src/Makevars.in > src/Makevars else echo "OpenMP supported" From 9ab5b33cdec80eafebdceb36a54938f7b91d96ed Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 25 Oct 2019 11:51:06 +0800 Subject: [PATCH 4/4] fix typo and increase visibility --- configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 53a3925f50..b20ba04b4b 100755 --- a/configure +++ b/configure @@ -61,7 +61,11 @@ echo "#include \nint main () { return omp_get_num_threads (); }" | ${CC} # Write to Makevars if [ $R_NO_OPENMP ]; then - echo "OpenMP not supported!\nFor details on how to install the neessesary toolchains on your OS see:\n https://github.com/Rdatatable/data.table/wiki/Installation " + echo "*** OpenMP not supported! data.table uses OpenMP to automatically" + echo "*** parallelize operations like sorting, grouping, file reading, etc." + echo "*** For details on how to install the necessary toolchains on your OS see:" + echo "*** https://github.com/Rdatatable/data.table/wiki/Installation" + echo "*** Continuing installation without OpenMP support..." sed -e "s|@openmp_cflags@||" src/Makevars.in > src/Makevars else echo "OpenMP supported"