search

r-wasm / rwasm

Build R packages for WebAssembly and create a CRAN-like repo for distribution.
https://r-wasm.github.io/rwasm/
Other
54 stars 4 forks source link

Disable $(SHLIB_OPENMP_CFLAGS), $(SHLIB_OPENMP_CXXFLAGS), and $(SHLIB_OPENMP_FFLAGS) #16

Closed coatless closed 7 months ago

coatless commented 8 months ago

At the moment, webR is passing into compilation statements -fopenmp. 

em++ -std=gnu++17 -DNDEBUG
-I'/github/home/R/x86_64-pc-linux-gnu-library/4.3/Rcpp/include' 
-I'/github/home/R/x86_64-pc-linux-gnu-library/4.3/RcppArmadillo/include' 
-I'/github/home/R/x86_64-pc-linux-gnu-library/4.3/RcppEnsmallen/include' 
-I/opt/webr/wasm/include
-I/opt/webr/R/build/R-4.3.2/build/include 
-I/opt/webr/R/build/R-4.3.2/src/include 
-s USE_BZIP2=1 -s USE_ZLIB=1 -I. 
-I../inst/include 
-fopenmp
 -fpic  -Oz -fPIC -fwasm-exceptions -s SUPPORT_LONGJMP=wasm 
-DRCPP_DEMANGLER_ENABLED=0 
-D__STRICT_ANSI__ 
-c RcppExports.cpp -o RcppExports.o

However, the header file is not being found:

In file included from RcppExports.cpp:4:
In file included from ./../inst/include/mlpack.h:52:
../inst/include/mlpack/core.hpp:61:12: fatal error: 'omp.h' file not found
   61 |   #include <omp.h>
      |            ^~~~~~~
2 warnings and 1 error generated.

https://github.com/coatless-mlpack/mlpack-hello-webr/actions/runs/7524258719/job/20510389211#step:5:489

In cases like RcppArmadillo, the armadillo library itself guards quite well against the lack of OpenMP:

https://github.com/RcppCore/RcppArmadillo/blob/20e4ea3cdc5dd2a115f94c92eb4b1754e70f9696/inst/include/armadillo#L98C1-L110C1

However, others are likely just doing a check with: 

#if defined(_OPENMP) 
  #include <omp.h>
#endif

Nullifying the SHLIB_OPENMP_* values in inst/webr-vars.mk should avoid having fopenmp being passed in. The relevant OpenMP flags in the base Makeconf:

$grep fopenmp $(R RHOME)/etc/Makeconf

DYLIB_LDFLAGS = -shared -fopenmp# $(CFLAGS) $(CPICFLAGS) 
MAIN_LDFLAGS = -Wl,--export-dynamic -fopenmp
SHLIB_OPENMP_CFLAGS = -fopenmp
SHLIB_OPENMP_CXXFLAGS = -fopenmp
SHLIB_OPENMP_FFLAGS = -fopenmp

So, I think the nullification can be done with:

override SHLIB_OPENMP_CFLAGS =
override SHLIB_OPENMP_CXXFLAGS =
override SHLIB_OPENMP_FFLAGS =

Note: DYLIB_LDFLAGS is overridden already using $(CFLAGS), which are based on $(WASM_CFLAGS).

/cc @jeroen thoughts? have you run into something similar?

georgestagg commented 7 months ago

Thanks, this sounds reasonable to me.

IIUC code written for openmp should safely degrade to serial code when the -fopenmp flag has not been given? That is, the #pragma omps will be ignored, the #include <omp.h> should be gated behind an #ifdef, and the distributed loops themselves should become single-threaded loops?

If so, I think this change will be OK. I guess nothing is lost, since we don't currently seem to provide a omp.h header under Emscripten anyway.

coatless commented 7 months ago

@georgestagg Yes, OpenMP-enabled code written using pragmas or guarded with #ifdef checking _OPENMP should safely degrade from parallel to serial usage. For the majority (if not all) of the packages on CRAN, this will hold as there are more stringent checks in place due to the lack of native OpenMP support on macOS.