DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.17k stars 262 forks source link

Blas C prototype functions leak to public headers and cause build issues #176

Closed antonio-rojas closed 1 year ago

antonio-rojas commented 1 year ago

Describe the bug The C prototype functions form SuiteSparse_config.h are pulled when including the suitesparse libraries public headers, which can cause conflicts with similar declarations from other projects which link to suitesparse. This affects at least octave.

To Reproduce Try to build octave against suitesparse 6.0

In file included from /usr/include/amd.h:43,
                 from ./liboctave/util/oct-sparse.h:43,
                 from liboctave/array/dSparse.cc:47:
/usr/include/SuiteSparse_config.h:942:6: error: conflicting declaration of C function 'void dsyrk_(const char*, const char*, const int32_t*, const int32_t*, const double*, const double*, const int32_t*, const double*, double*, const int32_t*)'
  942 | void SUITESPARSE_BLAS_DSYRK         // C = alpha*A*A' + beta*C, or A'A
      |      ^~~~~~~~~~~~~~~~~~~~~~
In file included from liboctave/array/dSparse.cc:27:
liboctave/numeric/lo-lapack-proto.h:1755:13: note: previous declaration 'void dsyrk_(const char*, const char*, const F77_INT&, const F77_INT&, const F77_DBLE&, const F77_DBLE*, const F77_INT&, const F77_DBLE&, F77_DBLE*, const F77_INT&, std::size_t, std::size_t)'
 1755 |   F77_FUNC (dsyrk, DSYRK) (F77_CONST_CHAR_ARG_DECL,
      |             ^~~~~
./config.h:66:29: note: in definition of macro 'F77_FUNC'
   66 | #define F77_FUNC(name,NAME) name ## _
      |                             ^~~~
/usr/include/SuiteSparse_config.h:972:6: error: conflicting declaration of C function 'void zherk_(const char*, const char*, const int32_t*, const int32_t*, const void*, const void*, const int32_t*, const void*, void*, const int32_t*)'
  972 | void SUITESPARSE_BLAS_ZHERK         // C = alpha*A*A^H + beta*C, or A^H*A
      |      ^~~~~~~~~~~~~~~~~~~~~~
liboctave/numeric/lo-lapack-proto.h:1361:13: note: previous declaration 'void zherk_(const char*, const char*, const F77_INT&, const F77_INT&, const F77_DBLE&, const F77_DBLE_CMPLX*, const F77_INT&, const F77_DBLE&, F77_DBLE_CMPLX*, const F77_INT&, std::size_t, std::size_t)'
 1361 |   F77_FUNC (zherk, ZHERK) (F77_CONST_CHAR_ARG_DECL,
      |             ^~~~~
./config.h:66:29: note: in definition of macro 'F77_FUNC'
   66 | #define F77_FUNC(name,NAME) name ## _
      |                             ^~~~
/usr/include/SuiteSparse_config.h:1002:6: error: conflicting declaration of C function 'void dpotrf_(const char*, const int32_t*, double*, const int32_t*, int32_t*)'
 1002 | void SUITESPARSE_LAPACK_DPOTRF      // Cholesky factorization
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~
liboctave/numeric/lo-lapack-proto.h:1602:13: note: previous declaration 'void dpotrf_(const char*, const F77_INT&, F77_DBLE*, const F77_INT&, F77_INT&, std::size_t)'
 1602 |   F77_FUNC (dpotrf, DPOTRF) (F77_CONST_CHAR_ARG_DECL,
      |             ^~~~~~
./config.h:66:29: note: in definition of macro 'F77_FUNC'
   66 | #define F77_FUNC(name,NAME) name ## _
      |                             ^~~~
/usr/include/SuiteSparse_config.h:1028:6: error: conflicting declaration of C function 'void zpotrf_(const char*, const int32_t*, void*, const int32_t*, int32_t*)'
 1028 | void SUITESPARSE_LAPACK_ZPOTRF      // Cholesky factorization
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~
liboctave/numeric/lo-lapack-proto.h:1614:13: note: previous declaration 'void zpotrf_(const char*, const F77_INT&, F77_DBLE_CMPLX*, const F77_INT&, F77_INT&, std::size_t)'
 1614 |   F77_FUNC (zpotrf, ZPOTRF) (F77_CONST_CHAR_ARG_DECL,
      |             ^~~~~~
./config.h:66:29: note: in definition of macro 'F77_FUNC'
   66 | #define F77_FUNC(name,NAME) name ## _
      |                             ^~~~
make[2]: *** [Makefile:21516: liboctave/array/libarray_la-dSparse.lo] Error 1

Expected behavior Builds

Desktop (please complete the following information):

DrTimothyAldenDavis commented 1 year ago

That's odd. What are the 2 extra std::size_t parameters at the end of the Octave calls to the Fortran BLAS? Or one for dpotrf? Those don't appear in the Fortran BLAS at all. I can try to avoid exposing those C definitions of the Fortran BLAS, but if I called those BLAS from CHOLMOD, UMFPACK, or SPQR, what happens with the extra 1 or 2 parameters?

DrTimothyAldenDavis commented 1 year ago

It's a simple fix on my side. I shouldn't be exposing those C prototypes of the Fortran BLAS to the user application. I'm just puzzled that they're different from what I've always used inside my codes, in the old cholmod_blas.h file for example. I had these prototypes there.

antonio-rojas commented 1 year ago

Those are for specifying the size of the strings, they are also present in netlib's own cblas:

https://github.com/Reference-LAPACK/lapack/blob/79a37b29a591cb0339048e5acf27d676da4f6058/CBLAS/include/cblas_f77.h#L1001

DrTimothyAldenDavis commented 1 year ago

Got it. Thanks for pointing that out. I had never needed to use those, oddly enough, but have never had any problems calling the Fortran BLAS from C, using any compilers. Maybe that's because the Fortran BLAS doesn't look at the whole string, but just checks the first character. My old prototypes in cholmod_blas.h didn't include those size_t parameters.

I just pushed an update to the dev2 branch of SuiteSparse. Can you give it a try? I'm traveling and can't do my full tests so I'd rather not post it as a beta release until I've had a chance to run all my brutal tests. This update passes my basic tests and it should be fine under my brutal tests.

antonio-rojas commented 1 year ago

octave builds fine with suitesparse from the dev2 branch, thanks for the quick fix!

DrTimothyAldenDavis commented 1 year ago

Fixed in v6.0.1. Thanks for the catch.

https://github.com/DrTimothyAldenDavis/SuiteSparse/releases/tag/v6.0.1