Skip to content
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.

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

Compilation fails on macOS with clang 12 #2805

Closed
fxcoudert opened this issue Aug 31, 2020 · 26 comments · Fixed by #2808
Closed

Compilation fails on macOS with clang 12 #2805

fxcoudert opened this issue Aug 31, 2020 · 26 comments · Fixed by #2808
Milestone

Comments

@fxcoudert
Copy link

I am building on macOS with clang 12 and gfortran 10, using:

make CC=clang FC=gfortran libs netlib shared TARGET=ARMV8

which leads to a build failure:

lapacke_sggsvp_work.c:48:9: error: implicit declaration of function 'sggsvp_' is invalid in C99
      [-Werror,-Wimplicit-function-declaration]
        LAPACK_sggsvp( &jobu, &jobv, &jobq, &m, &p, &n, a, &lda, b, &ldb, &tola,
        ^
../include/lapack.h:3755:23: note: expanded from macro 'LAPACK_sggsvp'
#define LAPACK_sggsvp LAPACK_GLOBAL(sggsvp,SGGSVP)
                      ^
../include/lapacke_mangling.h:12:39: note: expanded from macro 'LAPACK_GLOBAL'
#define LAPACK_GLOBAL(lcname,UCNAME)  lcname##_
                                      ^
<scratch space>:241:1: note: expanded from here
sggsvp_
^
lapacke_sggsvp_work.c:48:9: note: did you mean 'sggsvp3_'?
../include/lapack.h:3755:23: note: expanded from macro 'LAPACK_sggsvp'
#define LAPACK_sggsvp LAPACK_GLOBAL(sggsvp,SGGSVP)
                      ^
../include/lapacke_mangling.h:12:39: note: expanded from macro 'LAPACK_GLOBAL'
#define LAPACK_GLOBAL(lcname,UCNAME)  lcname##_
                                      ^
<scratch space>:241:1: note: expanded from here
sggsvp_
^
../include/lapack.h:3826:6: note: 'sggsvp3_' declared here
void LAPACK_sggsvp3(
     ^
../include/lapack.h:3825:24: note: expanded from macro 'LAPACK_sggsvp3'
#define LAPACK_sggsvp3 LAPACK_GLOBAL(sggsvp3,SGGSVP3)
                       ^
../include/lapacke_mangling.h:12:39: note: expanded from macro 'LAPACK_GLOBAL'
#define LAPACK_GLOBAL(lcname,UCNAME)  lcname##_
                                      ^
<scratch space>:111:1: note: expanded from here
sggsvp3_
^
@martin-frbg
Copy link
Collaborator

That looks like a typo (or copypasta) in Reference-LAPACK (where OpenBLAS gets most of LAPACK from). I think I saw it before, but it would not terminate the build if you did not have -Werror set. Almost certainly the LAPACKE_?ggsvp prototypes in lapack.h should read LAPACK_?ggsvp without the "E" like everything else in that file. I'll prepare a PR for both projects after testing.

@fxcoudert
Copy link
Author

I did not explicitly set -Werror, so it may be the default here or something in the makefiles added it.

@martin-frbg
Copy link
Collaborator

Probably affects all "deprecated" LAPACK functions (which were apparently added to lapack.h as an afterthought), but I am too tired already to prepare a proper fix today.

@fxcoudert
Copy link
Author

With this patch and #2807, make CC=clang FC=gfortran all succeeds to build.

Is there further testing I can do easily? I've run make checks but while it does not complain, it's also not saying anything positive (like “N/N tests passed” or …)

@martin-frbg
Copy link
Collaborator

If you like you can run the full LAPACK testsuite with make lapack-test (requires python for the driver script however), this should show "close to" zero failures in the summary. (There is usually a single elusive "numerical error" each in REAL and COMPLEX)

@fxcoudert
Copy link
Author

Hum, make lapack-test is not giving me any summary. It builds stuff, but that's it:

$ make lapack-test
(cd ./lapack-netlib/TESTING && rm -f x* *.out)
/Applications/Xcode-beta.app/Contents/Developer/usr/bin/make -j 8 -j 1 -C ./lapack-netlib/TESTING/EIG xeigtstc  xeigtstd  xeigtsts  xeigtstz 
make[1]: `xeigtstc' is up to date.
make[1]: `xeigtstd' is up to date.
make[1]: `xeigtsts' is up to date.
make[1]: `xeigtstz' is up to date.
/Applications/Xcode-beta.app/Contents/Developer/usr/bin/make -j 8 -j 1 -C ./lapack-netlib/TESTING/LIN xlintstc  xlintstd  xlintstds  xlintstrfd  xlintstrfz  xlintsts  xlintstz  xlintstzc xlintstrfs xlintstrfc
make[1]: `xlintstc' is up to date.
make[1]: `xlintstd' is up to date.
make[1]: `xlintstds' is up to date.
make[1]: `xlintstrfd' is up to date.
make[1]: `xlintstrfz' is up to date.
make[1]: `xlintsts' is up to date.
make[1]: `xlintstz' is up to date.
make[1]: `xlintstzc' is up to date.
make[1]: `xlintstrfs' is up to date.
make[1]: `xlintstrfc' is up to date.

@martin-frbg
Copy link
Collaborator

If it builds the tests but does not run them, you are missing the python interpreter to run lapack-testing.py (or the build system thinks you are cross-compiling, do you see CROSS=1 in Makefile.conf ?)

@brada4
Copy link
Contributor

brada4 commented Sep 1, 2020

OSX does not yet run on aarch64
You need to set HOSTCC=clang to build for iOS, or remove TARGET of non-native architecture.

@martin-frbg
Copy link
Collaborator

@brada4 he is most likely building on a pre-release Apple system. If CROSS=1 gets set there it is a bug (and probably introduced by me in a recent effort to aid OSX/iOS cross-compilation)

@martin-frbg
Copy link
Collaborator

c_check probably thinks hostarch is arm as it does not expect an uname response of arm64 either...

@fxcoudert
Copy link
Author

Yes I see CROSS=1 in Makefile.conf. I did not set it manually, and I am not cross-compiling (this is indeed running on a macOS pre-release system).

uname -m returns arm64 on macOS (instead of the usual aarch64 😢), then these two lines in c_check:

$hostarch = "arm" if ($hostarch =~ /^arm.*/);
$hostarch = "arm64" if ($hostarch eq "aarch64");

mean hostarch becomes arm (instead of arm64). Later, this:

$architecture = arm64  if ($data =~ /ARCH_ARM64/);

correctly detects architecture as arm64, leading the script to think it's cross-compiling.

@fxcoudert
Copy link
Author

I suggest changing:

$hostarch = "arm" if ($hostarch =~ /^arm.*/);

to

$hostarch = "arm" if ($hostarch ne "arm64" && $hostarch =~ /^arm.*/);

which works for me (CROSS is not defined anymore after that change).

@fxcoudert
Copy link
Author

Everything looking real good, now:

			-->   LAPACK TESTING SUMMARY  <--
SUMMARY             	nb test run 	numerical error   	other error  
================   	===========	=================	================  
REAL             	1300419		1	(0.000%)	0	(0.000%)	
DOUBLE PRECISION	1309007		0	(0.000%)	0	(0.000%)	
COMPLEX          	768366		0	(0.000%)	0	(0.000%)	
COMPLEX16         	769178		0	(0.000%)	0	(0.000%)	

--> ALL PRECISIONS	4146970		1	(0.000%)	0	(0.000%)	

@martin-frbg
Copy link
Collaborator

Thx for confirming - #2812 currently adds it to the arm64 line, not sure which is the more readable or elegant

@h-vetinari
Copy link
Contributor

I believe I have encountered the same problem - see Reference-LAPACK/lapack@127f0bf in Reference-LAPACK/lapack#409.

@zhiyuanzhai
Copy link

Actually I've met with the same problem with older version of macOS even though I have a x86-based Mac. My issue is also resolved in this developing version...

@dimpase
Copy link

dimpase commented Sep 22, 2020

I think this was closed too early. I get errors while trying to build version 0.3.10 on (Intel) macOS 10.15.6 with clang 12 from XCode 12 and gfortran 10, just doing

make -j8 CC=clang FC=gfortran all

and getting exactly the same errors as in the Issue (i.e.

clang -O2 -DMAX_STACK_ALLOC=2048 -DEXPRECISION -Wall -m64 -DF_INTERFACE_GFORT -fPIC -DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=8 -DMAX_PARALLEL_NUMBER=1 -DVERSION="0.3.10" -UASMNAME -UASMFNAME -UNAME -UCNAME -UCHAR_NAME -UCHAR_CNAME -DASMNAME=_ -DASMFNAME=__ -DNAME=_ -DCNAME= -DCHAR_NAME="_" -DCHAR_CNAME="" -DNO_AFFINITY -I. -DHAVE_LAPACK_CONFIG_H -I../include -c -o lapacke_zggsvp.o lapacke_zggsvp.c
lapacke_cggsvp_work.c:52:9: error: implicit declaration of function 'cggsvp_' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        LAPACK_cggsvp( &jobu, &jobv, &jobq, &m, &p, &n, a, &lda, b, &ldb, &tola,
        ^
../include/lapack.h:3771:23: note: expanded from macro 'LAPACK_cggsvp'
#define LAPACK_cggsvp LAPACK_GLOBAL(cggsvp,CGGSVP)
                      ^
../include/lapacke_mangling.h:12:39: note: expanded from macro 'LAPACK_GLOBAL'
#define LAPACK_GLOBAL(lcname,UCNAME)  lcname##_
                                      ^
<scratch space>:228:1: note: expanded from here
cggsvp_
^
...

@dimpase
Copy link

dimpase commented Sep 22, 2020

So we now have the fresh set of tools on macOS 10.15.6 which don't work with 0.3.10. Time for 0.3.11 ?

@martin-frbg
Copy link
Collaborator

There are still a few open issues that I would like to see fixed (or at least given another look) before 0.3.11, among them the proposed renaming of the new bfloat16 functions from SH.... to SB....

@dimpase
Copy link

dimpase commented Sep 22, 2020

could this issue be re-opened?

@martin-frbg martin-frbg added this to the 0.3.11 milestone Sep 22, 2020
@martin-frbg
Copy link
Collaborator

Would tagging the issue with the 0.3.11 milestione be sufficient ? I would like to keep the list of open issues somewhat manageable.

@dimpase
Copy link

dimpase commented Sep 22, 2020

this would do - perhaps, also changing the subject to say in addition macOS intel and arm platforms to make sure people don't confuse it for an arm-only issue

@martin-frbg
Copy link
Collaborator

It is two issues rolled into one, and I still think the lapacke flaw should not stop compilation if your build would not default to -Werror for some reason.

@dimpase
Copy link

dimpase commented Sep 22, 2020

the lapacke flaw should not stop compilation if your build would not default to -Werror for some reason.

-Werror is the default setting of Apple's clang 12 for .implicit declaration errors, we are now seeing it all over the place

@martin-frbg
Copy link
Collaborator

Thanks for the pointer (wonder why the openblas ticket there claims that the problem is "not completely solved" by the patch)

@dimpase
Copy link

dimpase commented Sep 22, 2020

(wonder why the openblas ticket there claims that the problem is "not completely solved" by the patch)

well, now we know that it does, and I'll fix the description. :-)
Indeed, a part of the "not completely solved" confusion is that the issue appears to task only ARM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants