Revert "Use SHLIB_OPENMP_ flags only if the compiler supports OpenMP"

[elbamos]

Reverts elbamos/largeVis#48

Unfortunately, since Makeconf isn't read until after Makevars, CXX isn't going to be set and this approach would fail for users (like me) who use ~/.R/Makevars to override the Makeconf settings.

[jimhester]

CXX is definitely available in the package Makevars, I think the problem is you need to override CXX11 or CXX1X (depending on R version), not CXX in your ~/.R/Makevars.

[elbamos]

The problem is that Makeconf isn't sourced until after package Makevars.

On May 12, 2017, at 4:06 AM, Jim Hester notifications@github.com wrote:

CXX is definitely available in the package Makevars, I think the problem is you need to override CXX11, not CXX in your ~/.R/Makevars.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

[jimhester]

You are mistaken, Makeconf is always sourced first (https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Creating-shared-objects)

make is called with makefiles R_HOME/etcR_ARCH/Makeconf, src/Makefile and any personal Makevars files (in that order)

See also the code for R CMD SHLIB, which is used when you have a src/Makevars.

https://github.com/wch/r-source/blob/30ff12aba27b09e03e9fafafe4de56cd60a57401/src/library/tools/R/install.R#L1754

[elbamos]

I thought I had tracked this down but I will take a closer look at it - thank you again.

On May 12, 2017, at 10:15 AM, Jim Hester notifications@github.com wrote:

You are mistaken, Makeconf is always sourced first (https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Creating-shared-objects)

make is called with makefiles R_HOME/etcR_ARCH/Makeconf, src/Makefile and any personal Makevars files (in that order)

See also the code for R CMD SHLIB, which is used when you have a src/Makevars.

https://github.com/wch/r-source/blob/30ff12aba27b09e03e9fafafe4de56cd60a57401/src/library/tools/R/install.R#L1754

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

[elbamos]

You're right -- I misspoke.

The issue arises because of the interaction between this:

It is also possible to set such variables in personal Makevars files, which are read after the local Makevars and the system makefiles or in a site-wide Makevars.site file. See Customizing package compilation in R Installation and Administration,

(https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Creating-shared-objects)

and:

Important note: R 3.4.0 El Capitan binaries are using Clang 4.0.0 and GNU Fortran 6.1 to provide OpenMP parallelization support and C++17 standard features. If you want to compile R packages from sources, please download GNU Fortran binary from the official GNU Fortran Binaries page - in particular OS X 10.11 gfortran 6.1. We are also providing Clang 4.0.0 binaries for OS X 10.11 and higher in our libs directory (note that the offical Clang 4.0.0 binaries only support macOS 10.12) and will provide an Apple Installer package here soon. For the time being you may want to either create ~/.R/Makevars such as

CC=/usr/local/clang4/bin/clang CXX=/usr/local/clang4/bin/clang++ LDFLAGS=-L/usr/local/clang4/lib

(https://cran.r-project.org/bin/macosx/tools/)

On OS X, in 3.4, Makeconf sets CXX* to clang, which is the Apple compiler, unless you muck with system paths, which can cause all kinds of trouble. The way people enable OpenMP is by overriding the compiler paths in ~/.R/Makevars, as recommended in the link above.

So, using the solution you proposed, on OS X the test for whether CXX supports -fopenmp will always return negative, because when the test is run CXX (and all its relatives) point to Apple clang. So the test effectively disabled OpenMP completely on OS X (unless you change Makeconf or muck with the PATH.

I may be missing something here, and I'd be thrilled if you could point me to where I went wrong.

My thinking about how to handle this, is that by enabling OpenMP by default in the package Makevars, this will succeed on every system except where the user is running R 3.4 on OS X and is choosing to compile from source instead of downloading from CRAN, but has not overridden the system compiler in their personal Makevars.

If you have a different view about that, I would appreciate understanding your perspective.

Thank you again!

[jimhester]

You are correct, I was mistaken because you can use the variables are available when running the make rules, but are not available when variables are defined. However you can retrieve the variables by querying R CMD config which makes this work as I was expecting.

diff --git a/src/Makevars b/src/Makevars
index 49dd43e..5c7b88a 100644
--- a/src/Makevars
+++ b/src/Makevars
@@ -1,4 +1,6 @@
-ifeq ($(shell $(CXX1X) -fopenmp -E -xc++ - && echo 'true'), true)
+CXX11=$(shell "${R_HOME}"/bin/R CMD config CXX11)
+
+ifeq ($(shell $(CXX11) -fopenmp -E -xc++ - 2>&1 >/dev/null && echo 'true'), true)
   PKG_CFLAGS = $(SHLIB_OPENMP_CFLAGS)
   PKG_LIBS = $(SHLIB_OPENMP_CXXFLAGS) $(SHLIB_OPENMP_CFLAGS) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS)
   PKG_CXXFLAGS = $(SHLIB_OPENMP_CXXFLAGS) -DARMA_64BIT_WORD -DNDEBUG

I verified this does not use -fopenmp using the default Apple compiler, but does use it with llvm-4.0.0 if it is specified in ~/.R/Makevars.

[elbamos]

Ah! I spent like 3 hours trying to figure out a way to do it -- thanks so much!

[jimhester]

Thank you for your patience with me, R's config files are a complicated beast!