Tests fail to compile with GCC under OpenMP variant (with Makefiles)

[svillemot]

With version 0.3.8, tests fail to compile under GCC with USE_OPENMP=1 (when using Makefiles). The tests binaries are not properly linked against libgomp.

I think this is a consequence of d55b108 and cfe63d8.

A possible solution is to explicitly add -lgomp to CEXTRALIB in test/Makefile and ctest/Makefile when using GCC.

[martin-frbg]

Sorry, I did not see this in my tests with gcc versions between 4.8.5 and 9.1 on x86_64 and arm(and the documentation suggests to me that -fopenmp implies -lomp automatically). What is your environment ?
That change from #2390 could certainly be made conditional on having detected a PGI compiler.

[brada4]

-fopenmp should be sufficient, it adds -lgomp and includes headers behind the scenes.
You have to pass same flags when compiling tests - did you?

[svillemot]

This happens in the context of the official Debian package.

You can see the log there: https://buildd.debian.org/status/fetch.php?pkg=openblas&arch=amd64&ver=0.3.8%2Bds-1&stamp=1581635393&raw=0

Note that we compile OpenBLAS six times in this log (pthread, openmp and serial × 32-bit indexing vs 64-bit indexing), hence it is difficult to understand.

The relevant portion of the log (for the tests under OpenMP + 32-bit indexing) is:

/usr/bin/make -C 0-openmp tests NO_LAPACKE=1 NO_AFFINITY=1 NO_WARMUP=1 CFLAGS="-Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security" FFLAGS="-g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong" COMMON_OPT= NUM_THREADS=64 DYNAMIC_ARCH=1 DYNAMIC_OLDER=1 TARGET=GENERIC  USE_THREAD=1 USE_OPENMP=1 INTERFACE64=0 LIBPREFIX=libopenblas   FCOMMON_OPT=-frecursive
make[3]: Entering directory '/<<PKGBUILDDIR>>/0-openmp'
touch libopenblasp-r0.3.8.a
/usr/bin/make -j 4 -C test all
make[4]: Entering directory '/<<PKGBUILDDIR>>/0-openmp/test'
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c sblat1.f  -o sblat1.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c dblat1.f  -o dblat1.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c cblat1.f  -o cblat1.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c zblat1.f  -o zblat1.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c sblat2.f  -o sblat2.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c dblat2.f  -o dblat2.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c cblat2.f  -o cblat2.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c zblat2.f  -o zblat2.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c sblat3.f  -o sblat3.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c dblat3.f  -o dblat3.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c cblat3.f  -o cblat3.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong  -frecursive -fPIC -c zblat3.f  -o zblat3.o
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o sblat1 sblat1.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o dblat1 dblat1.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o cblat1 cblat1.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o zblat1 zblat1.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o sblat2 sblat2.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o dblat2 dblat2.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o cblat2 cblat2.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o zblat2 zblat2.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o sblat3 sblat3.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o dblat3 dblat3.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o cblat3 cblat3.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp
gfortran -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -frecursive  -Wl,-z,relro -o zblat3 zblat3.o ../libopenblasp-r0.3.8.a -lm -lpthread -lgfortran -lm -lpthread -lgfortran -lgomp

As you can see, -fopenmp is not in the flags, even though I passed USE_OPENMP=1. Also note that the presence of -lgomp is due to the patch that I added to the Debian packaging, visible at: https://sources.debian.org/src/openblas/0.3.8+ds-1/debian/patches/link-tests-openmp.patch/

The same log for 0.3.7 is available there: https://buildd.debian.org/status/fetch.php?pkg=openblas&arch=amd64&ver=0.3.7%2Bds-7&stamp=1576693068&raw=0
The -fopenmp flag was not there either when building the tests, but OpenBLAS' build system would add -lgomp automatically.

[martin-frbg]

Makefile.system should add -fopenmp to FCOMMON_OPT when USE_OPENMP=1 - this is probably getting overwritten through some interaction with the CFLAGS and/or FCOMMON_OPT given on the commandline.

[martin-frbg]

The actual issue (not sure if I should call it a problem from the OpenBLAS perspective except that it is apparently not properly documented) is that Makefile.system does not use override when buiding/extending FCOMMON_OPT.
So when you supply your own choice of options, it must be the full list appropriate for the context. If you want to add to the defaults, FFLAGS appears to work well.

[zbeekman]

I'm not sure if this is related, but my PR to upgrade OpenBLAS in Homebrew (now the default Homebrew BLAS & LAPACK implementation rather than apple's outdated Accelerate framework) also failed when compiling with OpenMP support and GCC 9.2.

However, I noticed in the macOS install instructions that the problems with clang were with older clangs. I switched to build with Apple clang instead of GCC 9 and the issue went away.

Here's the Homebrew PR: Homebrew/homebrew-core#50252
(But I updated this to build with clang which Homebrew prefers, and fixed my problem upgrading)

Here's the build log with failures: https://jenkins.brew.sh/job/Homebrew%20Core%20Pull%20Requests/56916/version=mojave/testReport/junit/brew-test-bot/mojave/install___build_bottle_openblas/

The failure is:

# ...

gcc-9 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_sgemm_thread_nn -DASMFNAME=_sgemm_thread_nn_ -DNAME=sgemm_thread_nn_ -DCNAME=sgemm_thread_nn -DCHAR_NAME=\"sgemm_thread_nn_\" -DCHAR_CNAME=\"sgemm_thread_nn\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX  -c -DTHREADED_LEVEL3 -UDOUBLE -UCOMPLEX -DNN gemm.c -o sgemm_thread_nn.o
gcc-9 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_sgemm_thread_nt -DASMFNAME=_sgemm_thread_nt_ -DNAME=sgemm_thread_nt_ -DCNAME=sgemm_thread_nt -DCHAR_NAME=\"sgemm_thread_nt_\" -DCHAR_CNAME=\"sgemm_thread_nt\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX  -c -DTHREADED_LEVEL3 -UDOUBLE -UCOMPLEX -DNT gemm.c -o sgemm_thread_nt.o
gcc-9 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_sgemm_thread_tn -DASMFNAME=_sgemm_thread_tn_ -DNAME=sgemm_thread_tn_ -DCNAME=sgemm_thread_tn -DCHAR_NAME=\"sgemm_thread_tn_\" -DCHAR_CNAME=\"sgemm_thread_tn\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX  -c -DTHREADED_LEVEL3 -UDOUBLE -UCOMPLEX -DTN gemm.c -o sgemm_thread_tn.o
gcc-9 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_sgemm_thread_tt -DASMFNAME=_sgemm_thread_tt_ -DNAME=sgemm_thread_tt_ -DCNAME=sgemm_thread_tt -DCHAR_NAME=\"sgemm_thread_tt_\" -DCHAR_CNAME=\"sgemm_thread_tt\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX  -c -DTHREADED_LEVEL3 -UDOUBLE -UCOMPLEX -DTT gemm.c -o sgemm_thread_tt.o
gcc-9 -c -DTHREADED_LEVEL3 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_ssymm_thread_LU -DASMFNAME=_ssymm_thread_LU_ -DNAME=ssymm_thread_LU_ -DCNAME=ssymm_thread_LU -DCHAR_NAME=\"ssymm_thread_LU_\" -DCHAR_CNAME=\"ssymm_thread_LU\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX -UDOUBLE -UCOMPLEX -ULOWER -URSIDE -DNN symm_k.c -o ssymm_thread_LU.o
gcc-9 -c -DTHREADED_LEVEL3 -O2 -DMAX_STACK_ALLOC=2048 -fopenmp -Wall -DF_INTERFACE_GFORT -fPIC -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=6 -DMAX_PARALLEL_NUMBER=1 -DVERSION=\"0.3.8\" -DASMNAME=_ssymm_thread_LL -DASMFNAME=_ssymm_thread_LL_ -DNAME=ssymm_thread_LL_ -DCNAME=ssymm_thread_LL -DCHAR_NAME=\"ssymm_thread_LL_\" -DCHAR_CNAME=\"ssymm_thread_LL\" -DNO_AFFINITY -I../.. -UDOUBLE  -UCOMPLEX -UDOUBLE -UCOMPLEX -DLOWER -URSIDE -DNN symm_k.c -o ssymm_thread_LL.o
level3_thread.c:354:2: error: invalid instruction mnemonic 'dmb'
        dmb  ish
        ^~level3_thread.c:354~
:2: error: invalid instruction mnemonic 'dmb'
        dmb  ish
        ^~~
level3_thread.c:401:2: error: invalid instruction mnemonic 'dmb'
        dmb  ishst
        ^~~
# More similar errors
make[1]: *** [sgemm_thread_nt.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: *** [sgemm_thread_nn.o] Error 1
level3_thread.c:354:2: error: invalid instruction mnemonic 'dmb'
        dmb  ish
        ^~~
level3_thread.c:401:2: error: invalid instruction mnemonic 'dmb'
        dmb  ishst
        ^~~
# More similar errors
make[1]: *** [ssymm_thread_LU.o] Error 1
level3_thread.c:354:2: error: invalid instruction mnemonic 'dmb'
        dmb  ish
        ^~~
level3_thread.c:401:2: error: invalid instruction mnemonic 'dmb'
        dmb  ishst
        ^~~
# More similar errors
make[1]: *** [ssymm_thread_LL.o] Error 1
level3_thread.c:354:2: error: invalid instruction mnemonic 'dmb'
        dmb  ish
        ^~~
level3_thread.c:401:2: error: invalid instruction mnemonic 'dmb'
        dmb  ishst
        ^~~
# More similar errors
make[1]: *** [sgemm_thread_tt.o] Error 1
make: *** [libs] Error 1

[martin-frbg]

Eariier I wrote

if you want to add to the defaults, FFLAGS appears to work well.

but actually this turns out to be a bad idea for very similar reasons - FFLAGS is used in lapack-netlib so pre-setting it in the environment can have the effect of overriding important options like -frecursive or -fdefault-integer-8 (INTERFACE64). Unfortunately there does not appear to be a way (in GNU Makefile syntax) to "force-push" only selected important options into an already defined variable - once override is applied, no "weaker" addition can be performed i.e. all further additions must be done with override set.
This I think would remove the ability to "unset" options set in the makefiles - not sure yet if this would be a serious limitation. (Clearly the ability to break the build via "harmless" environment settings is not particularly desirable either, although the makefiles must have been like that for some 15 to 20 years in all.)

[svillemot]

Thanks for your feedback.

As you suggested, I added back -fopenmp to FCOMMON_OPT for the OpenMP variants, and everything now works as expected (without the patch aforementioned).

I’m thus closing this.

[martin-frbg]

I must admit I am still unsure how best to handle this - but perhaps it just needs to be documented properly.