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.15k stars 259 forks source link

Allow building without Fortran compiler #180

Closed mmuetzel closed 1 year ago

mmuetzel commented 1 year ago

Some build systems don't have a (working) Fortran compiler (e.g., MSYS2 targeting Windows on ARM). Or some users might prefer not to install a Fortran compiler. If I'm reading the cmake build rules correctly, a Fortran compiler is only needed to build demo code. IIUC, it isn't actually needed to build the libraries.

Would it be possible to add a build option that allows building without a Fortran compiler?

See also this downstream PR for MSYS2: https://github.com/msys2/MINGW-packages/pull/14156

DrTimothyAldenDavis commented 1 year ago

No Fortran compiler? Ack. I'd call that a broken system since the BLAS that I rely on is in Fortran.

In particular, when SuiteSparse_config is compiled, it queries the Fortran compiler to determine the C-to-Fortran calling mechanism (upper/lower case, append an underscore or not, etc). It also determines the BLAS that will be used (MKL, OpenBLAS, etc, and the BLAS integer size). The cmake FindBLAS.cmake script (built into cmake) only finds the Fortran BLAS, not the CBLAS. In any case the CBLAS (C interface to the BLAS) is bizarrely limited to int32 integers in the input parameters, so I can't use it. I have to use the Fortran BLAS.

So without a Fortran compiler, I can't determine how to build SuiteSparse. See https://cmake.org/cmake/help/latest/module/FortranCInterface.html

There are some Fortran codes not in the Demo, in the AMD package. Those could be made optional, if Fortran isn't available. The main problem is that without access to a Fortran compiler, I can't build UMFPACK, CHOLMOD, or SPQR, even though all those codes are purely in C or C++.

I suppose the C-to-Fortran calling mechanism could be specified manually, without using the cmake FortranCInterface module. That will take some work (a revision to the cmake build system, at least).

Windows is always the worst-case portability problem for SuiteSparse. CMake is supposed to fix that, but there are many other things that I have to do to get my codes to port to Windows. MS Visual Studio doesn't fully support ANSI C11 (for C), in particular. It's very nasty to deal with. The lack of a Fortran compiler is not something I expected to be a problem (I only need Fortran 77, after all).

mmuetzel commented 1 year ago

Sorry, I wasn't clear in the original post. This isn't about MSVC. But about the MinGW toolchain (in MSYS2). The toolchain might be broken indeed (at least the LLVM toolchain). But unfortunately, there is no FOSS Fortran compiler that targets Windows on ARM (yet). Afaict, GCC (including gfortran) doesn't target that platform at all. And LLVM-Flang isn't quite there yet.

BLAS/LAPACK can be used with the C implementation of OpenBLAS on that platform.

Having written that, it was possible to build (at least parts of) SuiteSparse without Fortran compiler in previous versions. It would be nice if those parts could be built again.

DrTimothyAldenDavis commented 1 year ago

Got it.

The only problem would be that the C-to-Fortran interface would have to be hard-wired, perhaps with CMAKE variable settings. I would need to know if the BLAS / LAPACK libraries use 32-bit or 64-bit integers, and also how to call them (dgemm with an underscore? DGEMM? DGEMM? dgemm?). That's what the "include ( FortranCinterface )" statement does.

You can see how these settings are used by doing a diff between SuiteSparse_config/Config/SuiteSparse_config.h.in and the SuiteSparse_config/SuiteSparse_config.h files and looking at the Fortran definitions:

555,556c555,556
<     #define SUITESPARSE_FORTRAN(name,NAME) name##_
<     #define SUITESPARSE__FORTRAN(name,NAME) name##_
---
>     #define SUITESPARSE_FORTRAN@FortranCInterface_GLOBAL_MACRO@
>     #define SUITESPARSE__FORTRAN@FortranCInterface_GLOBAL__MACRO@
586c586
<     #define SUITESPARSE_BLAS_INT int32_t
---
>     #define SUITESPARSE_BLAS_INT @SuiteSparse_BLAS_integer@

I suppose I could set a default value to the FortranCinterface_GLOBALMACRO, to be "(name,NAME) name##", if no Fortran compiler exists. Would that work on MinGW? Or is the trailing underscore left off?

I can't use the CBLAS interface to the BLAS, since the cmake FindBLAS.cmake module doesn't find it. It only works with the Fortran interface to the BLAS, even if the BLAS itself is written in C (but with a Fortran interface). Worse yet, the CBLAS interface is limited to 32-bit integers only (which is a terrible oversight). There are cases where I'm can only use 64-bit integers with the BLAS.

DrTimothyAldenDavis commented 1 year ago

This is turning out to be simple to fix. I'll post a draft v6.0.2 on the dev2 branch of SuiteSparse shortly. It will have a new cmake variable that you can define: SUITESPARSE_C_TOFORTRAN. It's a string that defaults to "(name,NAME) name" on Windows, and "(name,NAME) name##" otherwise. That is, for Windows, the underscore is not appended when C calls the Fortran BLAS/LAPACK, and it is appended for all other platforms. If a Fortran compiler is found, this variable is ignored.

I would guess that's the right default for MSYS2. If it isn't, the default setting is easy to change.

I think the search for the BLAS that is done by SuiteSparseBLAS.cmake will still work, even if no Fortran compiler is present.

The lack of a Fortran compiler will mean that amd.f and amdbar.f will not appear in the libamd.so. Some demos won't be compiled (AMD, UMFPACK, and CHOLMOD), but that's no big deal. And finally, the C-to-Fortran name mangling might need to be handled if my defaults are not correct.

DrTimothyAldenDavis commented 1 year ago

Ok: give this branch a try: https://github.com/DrTimothyAldenDavis/SuiteSparse/tree/dev2 . I haven't tagged it as a prerelease yet, but it will become SuiteSpare v6.0.2-betaN once you've given it a try and it works.

mmuetzel commented 1 year ago

Afaict for the MinGW toolchain on Windows (I can't talk much about MSVC), Fortran functions are called from C in lower case with a trailing underscore. Length of character arrays are passed as trailing arguments. Overall, it is the same calling conventions that gfortran uses also for Linux targets iiuc.

I agree that it is a reasonable assumption to default to 32bit integer size.

mmuetzel commented 1 year ago

I tried to build the dev2 branch with a GCC toolchain (including gfortran) for MinGW. But it failed to link cholmod with a lot of undefined reference errors like the following:

[100%] Linking C shared library libcholmod.dll
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/cholmod.dir/objects.a(cholmod_l_super_numeric.c.obj):cholmod_l_super_numeric.c:(.text+0x1e06): undefined reference to `zherk'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/cholmod.dir/objects.a(cholmod_l_super_numeric.c.obj):cholmod_l_super_numeric.c:(.text+0x1fd0): undefined reference to `zgemm'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/cholmod.dir/objects.a(cholmod_l_super_numeric.c.obj):cholmod_l_super_numeric.c:(.text+0x21f3): undefined reference to `zpotrf'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/cholmod.dir/objects.a(cholmod_l_super_numeric.c.obj):cholmod_l_super_numeric.c:(.text+0x276a): undefined reference to `ztrsm'
[...]

It looks like it failed to detect gfortran. Maybe as a result of that, it was using the wrong calling convention? Anyway, here is the output from the cmake configure stage of this subproject:

make[1]: Verzeichnis „/d/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD“ wird betreten
( cd build && cmake -G"MSYS Makefiles" -DCMAKE_INSTALL_PREFIX="/mingw64" -DCMAKE_BUILD_TYPE=Release -DENABLE_CUDA=OFF -DBLA_VENDOR=OpenBLAS -DNPARTITION=ON .. && make --jobs=8 )
-- Building CHOLMOD version: v4.0.2 (Dec 1, 2022)
-- Source:        D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD
-- Build:         D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD/build
-- The C compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Install rpath:
-- Build   rpath: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD/build
-- Build type:    Release
-- Fortran: not available
-- CUDA: not enabled
-- Found OpenMP_C: -fopenmp (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
-- Found SuiteSparse_config: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/SuiteSparse_config/build/libsuitesparseconfig.dll.a (found suitable version "6.0.2", minimum required is "6.0.0")
-- SuiteSparse_config include dir: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/SuiteSparse_config
-- SuiteSparse_config version:     6.0.2
-- Found COLAMD: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/COLAMD/build/libcolamd.dll.a (found suitable version "3.0.0", minimum required is "3.0.0")
-- COLAMD include dir: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/COLAMD/include
-- COLAMD library:     D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/COLAMD/build/libcolamd.dll.a
-- COLAMD version:     3.0.0
-- Found AMD: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/AMD/build/libamd.dll.a (found suitable version "3.0.2", minimum required is "3.0.0")
-- AMD include dir: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/AMD/include
-- AMD library:     D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/AMD/build/libamd.dll.a
-- AMD version:     3.0.2
-- Found CAMD: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CAMD/build/libcamd.dll.a (found suitable version "3.0.0", minimum required is "3.0.0")
-- CAMD include dir: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CAMD/include
-- CAMD library:     D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CAMD/build/libcamd.dll.a
-- CAMD version:     3.0.0
-- Found CCOLAMD: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CCOLAMD/build/libccolamd.dll.a (found suitable version "3.0.0", minimum required is "3.0.0")
-- CCOLAMD include dir: D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CCOLAMD/include
-- CCOLAMD library:     D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CCOLAMD/build/libccolamd.dll.a
-- CCOLAMD version:     3.0.0
-- Looking for 32-BLAS: OpenBLAS
-- Looking for sgemm_
-- Looking for sgemm_ - found
-- Found BLAS: C:/msys64/mingw64/lib/libopenblas.dll.a
-- Looking for cheev_
-- Looking for cheev_ - found
-- Found LAPACK: C:/msys64/mingw64/lib/libopenblas.dll.a;C:/msys64/mingw64/lib/libopenblas.dll.a
-- Found OpenBLAS 32-bit BLAS+LAPACK
-- OpenMP C libraries:      C:/msys64/mingw64/lib/libgomp.dll.a;C:/msys64/mingw64/lib/libmingwthrd.a;C:/msys64/mingw64/lib/libmingwthrd.a
-- OpenMP C include:
-- OpenMP C flags:          -fopenmp
-- BLAS libraries:      C:/msys64/mingw64/lib/libopenblas.dll.a
-- BLAS linker flags:
-- LAPACK libraries:    C:/msys64/mingw64/lib/libopenblas.dll.a;C:/msys64/mingw64/lib/libopenblas.dll.a
-- LAPACK linker flags:
-- Installation will be system-wide (requires 'sudo make install')
-- Skipping the demos in CHOLMOD/Demo
-- ------------------------------------------------------------------------
-- SuiteSparse CMAKE report for: cholmod
-- ------------------------------------------------------------------------
-- install in /mingw64: true
-- install in SuiteSparse/lib and SuiteSparse/include: false
-- build type:           Release
-- NSTATIC:              false (build static library)
-- use OpenMP:           yes
-- C compiler:           GNU
-- C flags:              -march=nocona -msahf -mtune=generic -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong   -fopenmp
-- C++ compiler:
-- C++ flags:             -fopenmp
-- C Flags release:      -O3 -DNDEBUG
-- C++ Flags release:
-- compile definitions:  NPARTITION;BLAS_OpenBLAS
-- BLAS integer:         int32_t
-- NPARTITION:           do not use METIS
-- ------------------------------------------------------------------------
-- Configuring done
-- Generating done
mmuetzel commented 1 year ago

Maybe, it is still necessary to check for the Fortran compiler before using CMAKE_Fortran_COMPILER in a condition? See: https://cmake.org/cmake/help/latest/module/CheckLanguage.html

DrTimothyAldenDavis commented 1 year ago

Sorry for the mixup. I had the "check_language (Fortran)" statement disabled, to test the case when Fortran wasn't found. I just uncommented a line in the SuiteSparse_config/cmake_modules/SuiteSparsePolicy.cmake and pushed the 1-line change.

Can you try it again?

From the output log, cmake says "Looking for sgemm_ -- found". That means it found sgemm with an underscore. So I'm guessing that you do need the underscore after all, at least with mingw.

On the system with no Fortran compiler, you can control the name mangling with SUITESPARSE_C_TO_FORTRAN. I thought Windows didn't append the underscore. Maybe mingw does but msys2 doesn't.

DrTimothyAldenDavis commented 1 year ago

On a different issue: I see you have METIS disabled. It should be OK on Windows, since METIS is now embedded into CHOLMOD (see the CHOLMOD/SuiteSparse_metis folder). I only compile the specific files from METIS that are needed for the graph partitioning library routines, not the programs. So things in METIS that tend to break Windows (gk_regex, and other Linux-like look-alikes) are not compiled at all.

Can you try with METIS enabled?

mmuetzel commented 1 year ago

Afaict, the name mangling is not so much platform-specific but compiler-specific. gfortran is always lower-case + underscore with length of character arrays as trailing arguments. (See my comment from earlier today.) The Visual Fortran Compiler from MSVC might be different. It might be a good idea to change the condition variable for the default from WIN32 to MSVC:


 # default C-to-Fortran name mangling if Fortran compiler not found
-if ( WIN32 )
-     # Windows Fortran compilers do not typically mangle the Fortran name
+if ( MSVC )
+    # The Visual Fortran compiler does not mangle the Fortran name
     set ( SUITESPARSE_C_TO_FORTRAN "(name,NAME) name"
         CACHE STRING "C to Fortan name mangling" )

I used the build commands from the PR linked in the original report:

cd build && cmake -G"MSYS Makefiles" -DCMAKE_INSTALL_PREFIX="/mingw64" -DCMAKE_BUILD_TYPE=Release -DENABLE_CUDA=OFF -DBLA_VENDOR=OpenBLAS -DNPARTITION=ON .. && make --jobs=8

That works perfectly with your recent change with a GCC toolchain (including gfortran).

I removed -DNPARTITION=ON and it failed to build with the following error:

In file included from D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD/SuiteSparse_metis/GKlib/GKlib.h:23,
                 from D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD/Partition/cholmod_metis_wrapper.c:31:
D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-MINGW64/CHOLMOD/SuiteSparse_metis/GKlib/gk_arch.h:44:12: fatal error: sys/resource.h: No such file or directory
   44 |   #include <sys/resource.h>
      |            ^~~~~~~~~~~~~~~~
compilation terminated.
make[4]: *** [CMakeFiles/cholmod_static.dir/build.make:1378: CMakeFiles/cholmod_static.dir/Partition/cholmod_metis_wrapper.c.obj] Fehler 1

Afaict, that is a POSIX specific header that doesn't exist on Windows: https://pubs.opengroup.org/onlinepubs/7908799/xsh/sysresource.h.html

You might also want to have a look at 0002-Fix-building-spqr-without-cholmod-partition.patch from that PR. Or PR #181 in this repository. IIUC, that is necessary to be able to build SPQR with NPARTITION=ON (kudos to @MehdiChinoune):

--- a/SPQR/CMakeLists.txt
+++ b/SPQR/CMakeLists.txt
@@ -201,8 +201,14 @@

 # cholmod:
 target_link_libraries ( spqr PUBLIC ${CHOLMOD_LIBRARIES} )
+if ( NPARTITION )
+  target_compile_definitions(spqr PRIVATE NPARTITION)
+endif()
 if ( NOT NSTATIC )
 target_link_libraries ( spqr_static PUBLIC ${CHOLMOD_LIBRARIES} )
+  if ( NPARTITION )
+    target_compile_definitions(spqr_static PRIVATE NPARTITION)
+  endif()
 endif ( )

 # CUDA

I also tried to build with a LLVM toolchain for 32-bit Windows (no Fortran compiler) with the same commands that worked for the GCC toolchain. But that failed with similar linker errors as before your recent change:

[ 99%] Linking C shared library libcholmod.dll
[ 99%] Building C object CMakeFiles/cholmod_static.dir/Supernodal/cholmod_super_symbolic.c.obj
ld.lld: error: undefined symbol: _dsyrk
>>> referenced by objects.a(cholmod_l_super_numeric.c.obj):(_r_cholmod_super_numeric)
>>> referenced by objects.a(cholmod_super_numeric.c.obj):(_r_cholmod_super_numeric)

ld.lld: error: undefined symbol: _dgemm
>>> referenced by objects.a(cholmod_l_super_numeric.c.obj):(_r_cholmod_super_numeric)
>>> referenced by objects.a(cholmod_l_super_solve.c.obj):(_r_cholmod_super_lsolve)
>>> referenced by objects.a(cholmod_l_super_solve.c.obj):(_r_cholmod_super_ltsolve)
>>> referenced 3 more times
[...]

I modified the build command like the following, and it seems to be linking fine:

cd build && cmake -G"MSYS Makefiles" -DCMAKE_INSTALL_PREFIX="/clang32" -DCMAKE_BUILD_TYPE=Release -DENABLE_CUDA=OFF -DBLA_VENDOR=OpenBLAS -DNPARTITION=ON -DSUITESPARSE_C_TO_FORTRAN="(name,NAME) name##_" .. && make --jobs=8

So, changing the default might indeed be helpful.

DrTimothyAldenDavis commented 1 year ago

I've changed the default C-to-Fortran name mangling.

I've posted a draft on the dev2 branch of a fix for SPQR, when CHOLMOD is compiled with -DNPARTITION.

METIS has 4 files that use "#ifdef MSC" for handling parts of METIS that must be compiled differently on Windows. This is controlled here: https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/4faadddd6d373e1b85f567dc034e9eb9733247c7/CHOLMOD/SuiteSparse_metis/GKlib/GKlib.h#L15 .

When METIS is compiled for use inside CHOLMOD, I could modify that to:

#if defined (_MSC_VER) || defined ( MSYS2_WHATEVER )
#define __MSC__
#endif

Then METIS might build just fine on Windows with MSYS2. Does MSYS2 provide a predefined macro like _MSC_VER? Or should that code above be changed to the following?

#if defined ( _WIN32)
#define __MSC__
#endif
mmuetzel commented 1 year ago

Afaict, there is no MSYS2-specific pre-processor variable. To give a little bit of background: MSYS2 is a collection of different combinations of compiler toolchains, headers and libraries. There are two main groups of these build "environments": The "classic" MSYS2 environments that are basically slightly different versions of Cygwin (i.e., "POSIX" on Windows). And the native build environments (Windows API instead of POSIX API). SuiteSparse is distributed as binary packages for the native environments by the MSYS2 project.

The preprocessor macro _MSC_VER is usually used to guard compiler-specific code (e.g., some function attributes or pre-processor pragmas that are specific to the MS Visual Studio compilers). The POSIX headers aren't compiler specific. They are platform specific. In that case, checking if _WIN32 is defined would indeed be the "right thing". However, I don't know how __MSC__ is used in SuiteSparse (or CHOLMOD). It sounds like it might also be used to guard compiler-specific code. But I can only guess from the name...

MehdiChinoune commented 1 year ago

The common preprocessor for mingw-w64 is __MINGW32__

mmuetzel commented 1 year ago

I tested building the latest head of the dev2 branch (39ce416) with GCC for 64-bit Windows (incl. Fortran compiler) and Clang for 32-bit Windows (no Fortran compiler). Both compiled correctly without the patch 0002-Fix-building-spqr-without-cholmod-partition.patch and without specifying SUITESPARSE_C_TO_FORTRAN (but with -DNPARTITION=ON). 🎉

Looks good to me. 👍

DrTimothyAldenDavis commented 1 year ago

I don't use __MSC__ myself. It is unique to METIS, and is basically George Karypis' synonym for _MSC_VER (George is the author of METIS). It's not a good choice for a name since it starts with two underscores, putting in the name space of the system or compiler, not a particular package. That's confusing.

And it turns out that I don't actually need it at all.

The way it is used in METIS is as a platform-specific #define. However, it's only needed for the METIS programs (reading files and such), not for the library routines. I don't include any file operations in my build of METIS, so I can actually remove the #include <sys/resource.h> completely.

CHOLMOD/SuiteSparse_metis is a very lightly editted copy of the original metis-5.1.0. I don't compile all the files in CHOLMOD/SuiteSparse_metis, just a subset.

I think it should be possible to compile METIS on Windows with these changes. Can you try the version of dev2 that I just updated?

https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/27e922425c3626b3e7deaedfbe18a0fe285b6b68

mmuetzel commented 1 year ago

The METIS seems to be building now. However, I'm seeing a bunch of warnings like the following:

In file included from D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/Partition/cholmod_metis_wrapper.c:77:
D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/SuiteSparse_metis/libmetis/fm.c:68:14: warning: absolute value function 'labs' given an argument of type 'long long' but has parameter of type 'long' which may cause truncation of value [-Wabsolute-value]
  origdiff = iabs(tpwgts[0]-pwgts[0]);
             ^
D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/SuiteSparse_metis/include/metis.h:114:25: note: expanded from macro 'iabs'
  #define iabs          labs
                        ^
D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/SuiteSparse_metis/libmetis/fm.c:68:14: note: use function 'llabs' instead
  origdiff = iabs(tpwgts[0]-pwgts[0]);
             ^~~~
             llabs

That might be because long is only 32-bits wide on native Windows (not Cygwin).

DrTimothyAldenDavis commented 1 year ago

Thanks for the update. The latest push to dev2 should fix this. Let me know if there are other warnings.

mmuetzel commented 1 year ago

There are still some warnings of the following type with clang for 32-bit Windows:

In file included from D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/Check/cholmod_l_read.c:12:
D:/MINGW-packages-mmuetzel/mingw-w64-suitesparse/src/build-CLANG32/CHOLMOD/Check/cholmod_read.c:560:56: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
    if (!ok || nrow > Int_max || ncol > Int_max || nnz > Int_max)
                                                   ~~~ ^ ~~~~~~~

But I don't think they are new. And it is probably safe to ignore them.

Regarding the latest changeset: Would it be necessary to add an overflow check if the input is LLONG_MIN? I'm not exactly sure what the behavior is in this case (wrap-around?).

Quite off-topic: Mongoose, GraphBLAS, and SPEX currently don't build. But that isn't new. Building those parts is currently skipped for the MSYS2 binary packages.

MehdiChinoune commented 1 year ago

Quite off-topic: Mongoose, GraphBLAS, and SPEX currently don't build.

We don't skip SPEX.

mmuetzel commented 1 year ago

@MehdiChinoune: Maybe, I'm misunderstanding. But doesn't this part of your PR skip SPEX with GCC? https://github.com/msys2/MINGW-packages/pull/14156/commits/88e73aaa628ad55a0babc185b7ffd6b1b17c31b7

MehdiChinoune commented 1 year ago

@MehdiChinoune: Maybe, I'm misunderstanding. But doesn't this part of your PR skip SPEX with GCC? https://github.com/msys2/MINGW-packages/pull/14156/commits/88e73aaa628ad55a0babc185b7ffd6b1b17c31b7

That's just a workaround. With the new mpfr it builds successfully.

DrTimothyAldenDavis commented 1 year ago

I would be interested to know why Mongoose and GraphBLAS don't build, perhaps posted as a new issue (for Mongoose on this SuiteSparse repo, and GraphBLAS on https://github.com/DrTimothyAldenDavis/GraphBLAS ). GraphBLAS builds for MATLAB on Windows, so I know it can be done. I don't ever use Windows myself so porting there is very difficult for me to do. I stick to ANSI C11 (or try to at least) and hope it works ... but there are many non-ANSI-C11-standard things that Windows does. They do not fully support ANSI C11.

DrTimothyAldenDavis commented 1 year ago

Regarding the nnz > Int_max warning: yes, that is intentionally tautological when building the 64-bit CHOLMOD functions, but of course essential when the same code is compiled with 32-bit integers.

Integer overflow can't occur in the iabs or abs functions with this latest update: https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/1293f0914e2e227b678f56f7b6f9c3535b90450a

I have redefined iabs and abs (via macros) to be calls to a static inline function, SuiteSparse_metis_abs64, with int64_t inputs and outputs. That should be safe.

mmuetzel commented 1 year ago

Regarding the possible overflow:

#include <stdio.h>
#include <limits.h>

int main()
{
    long long int a = LLONG_MIN;
    long long int b = -a;

    printf("a: %lld, b: %lld\n", a, b);

    return 0;
}

This outputs the following for me on Linux with GCC:

a: -9223372036854775808, b: -9223372036854775808

I don't know if this is something that could lead to errors in your code.

mmuetzel commented 1 year ago

Fwiw, same with llabs:

#include <stdio.h>
#include <limits.h>
#include <stdlib.h>

int main()
{
    long long int a = LLONG_MIN;
    long long int b = -a;
    long long int c = llabs(a);

    printf("a: %lld, b: %lld, c: %lld\n", a, b, c);

    return 0;
}

Output:

a: -9223372036854775808, b: -9223372036854775808, c: -9223372036854775808

So at least, it's not worse than before.

DrTimothyAldenDavis commented 1 year ago

No, this won't cause an error. This would be expected behavior when working with signed int64_t types. In CHOLMOD, it would be impossible to get an integer that large as a row/col index or matrix dimension. In METIS, the integers are node/edge weights, which are counts of number of nodes/edges in the original graph, and those cannot be this large either. I already guard against this in CHOLMOD and all my other codes.

mmuetzel commented 1 year ago

Good news: Mongoose seems to be building just fine from the dev2 branch. I no longer recall what the original issue was that triggered to disable it.

I'll open an issue on the repository you pointed out for the build issue with GraphBLAS.

mmuetzel commented 1 year ago

The cholmod library exports a lot less symbols with the version from the dev2 branch: See, e.g.: https://github.com/mmuetzel/MINGW-packages/actions/runs/3567744148/jobs/5997461650#step:3:48

Is this expected?

DrTimothyAldenDavis commented 1 year ago

I don't understand how to read that output.

mmuetzel commented 1 year ago

If I understand correctly what the package grokker does, it takes the shared libraries of the current binary package in MSYS2 (which is at version 5.13.0) and creates lists with the symbols that are exported by those libraries. It then creates the same lists for the shared libraries in the updated package (which was built from the dev2 branch in this case). In a last step, it compares these lists and displays the symbols that have been exported by the old version but are no longer exported by the new version. That means, the following symbols have been exported by the cholmod library in version 5.13.0 but are no longer exported by the version built from the dev2 branch:

  {b'libcholmod.dll': {b'cholmod_aat',
                       b'cholmod_add',
                       b'cholmod_add_size_t',
                       b'cholmod_allocate_dense',
                       b'cholmod_allocate_factor',
                       b'cholmod_allocate_sparse',
                       b'cholmod_allocate_triplet',
                       b'cholmod_allocate_work',
                       b'cholmod_amd',
                       b'cholmod_analyze',
                       b'cholmod_analyze_ordering',
                       b'cholmod_analyze_p',
                       b'cholmod_analyze_p2',
                       b'cholmod_band',
                       b'cholmod_band_inplace',
                       b'cholmod_bisect',
                       b'cholmod_calloc',
                       b'cholmod_camd',
                       b'cholmod_ccolamd',
                       b'cholmod_change_factor',
                       b'cholmod_check_common',
                       b'cholmod_check_dense',
                       b'cholmod_check_factor',
                       b'cholmod_check_parent',
                       b'cholmod_check_perm',
                       b'cholmod_check_sparse',
                       b'cholmod_check_subset',
                       b'cholmod_check_triplet',
                       b'cholmod_clear_flag',
                       b'cholmod_colamd',
                       b'cholmod_collapse_septree',
                       b'cholmod_copy',
                       b'cholmod_copy_dense',
                       b'cholmod_copy_dense2',
                       b'cholmod_copy_factor',
                       b'cholmod_copy_sparse',
                       b'cholmod_copy_triplet',
                       b'cholmod_csymamd',
                       b'cholmod_dbound',
                       b'cholmod_defaults',
                       b'cholmod_dense_to_sparse',
                       b'cholmod_dense_xtype',
                       b'cholmod_divcomplex',
                       b'cholmod_drop',
                       b'cholmod_ensure_dense',
                       b'cholmod_error',
                       b'cholmod_etree',
                       b'cholmod_eye',
                       b'cholmod_factor_to_sparse',
                       b'cholmod_factor_xtype',
                       b'cholmod_factorize',
                       b'cholmod_factorize_p',
                       b'cholmod_finish',
                       b'cholmod_free',
                       b'cholmod_free_dense',
                       b'cholmod_free_factor',
                       b'cholmod_free_sparse',
                       b'cholmod_free_triplet',
                       b'cholmod_free_work',
                       b'cholmod_gpu_stats',
                       b'cholmod_horzcat',
                       b'cholmod_hypot',
                       b'cholmod_l_aat',
                       b'cholmod_l_add',
                       b'cholmod_l_add_size_t',
                       b'cholmod_l_allocate_dense',
                       b'cholmod_l_allocate_factor',
                       b'cholmod_l_allocate_sparse',
                       b'cholmod_l_allocate_triplet',
                       b'cholmod_l_allocate_work',
                       b'cholmod_l_amd',
                       b'cholmod_l_analyze',
                       b'cholmod_l_analyze_ordering',
                       b'cholmod_l_analyze_p',
                       b'cholmod_l_analyze_p2',
                       b'cholmod_l_band',
                       b'cholmod_l_band_inplace',
                       b'cholmod_l_bisect',
                       b'cholmod_l_calloc',
                       b'cholmod_l_camd',
                       b'cholmod_l_ccolamd',
                       b'cholmod_l_change_factor',
                       b'cholmod_l_check_common',
                       b'cholmod_l_check_dense',
                       b'cholmod_l_check_factor',
                       b'cholmod_l_check_parent',
                       b'cholmod_l_check_perm',
                       b'cholmod_l_check_sparse',
                       b'cholmod_l_check_subset',
                       b'cholmod_l_check_triplet',
                       b'cholmod_l_clear_flag',
                       b'cholmod_l_colamd',
                       b'cholmod_l_collapse_septree',
                       b'cholmod_l_copy',
                       b'cholmod_l_copy_dense',
                       b'cholmod_l_copy_dense2',
                       b'cholmod_l_copy_factor',
                       b'cholmod_l_copy_sparse',
                       b'cholmod_l_copy_triplet',
                       b'cholmod_l_csymamd',
                       b'cholmod_l_dbound',
                       b'cholmod_l_defaults',
                       b'cholmod_l_dense_to_sparse',
                       b'cholmod_l_dense_xtype',
                       b'cholmod_l_divcomplex',
                       b'cholmod_l_drop',
                       b'cholmod_l_ensure_dense',
                       b'cholmod_l_error',
                       b'cholmod_l_etree',
                       b'cholmod_l_eye',
                       b'cholmod_l_factor_to_sparse',
                       b'cholmod_l_factor_xtype',
                       b'cholmod_l_factorize',
                       b'cholmod_l_factorize_p',
                       b'cholmod_l_finish',
                       b'cholmod_l_free',
                       b'cholmod_l_free_dense',
                       b'cholmod_l_free_factor',
                       b'cholmod_l_free_sparse',
                       b'cholmod_l_free_triplet',
                       b'cholmod_l_free_work',
                       b'cholmod_l_gpu_stats',
                       b'cholmod_l_horzcat',
                       b'cholmod_l_hypot',
                       b'cholmod_l_lsolve_pattern',
                       b'cholmod_l_malloc',
                       b'cholmod_l_maxrank',
                       b'cholmod_l_metis',
                       b'cholmod_l_metis_bisector',
                       b'cholmod_l_mult_size_t',
                       b'cholmod_l_nested_dissection',
                       b'cholmod_l_nnz',
                       b'cholmod_l_norm_dense',
                       b'cholmod_l_norm_sparse',
                       b'cholmod_l_ones',
                       b'cholmod_l_pack_factor',
                       b'cholmod_l_postorder',
                       b'cholmod_l_print_common',
                       b'cholmod_l_print_dense',
                       b'cholmod_l_print_factor',
                       b'cholmod_l_print_parent',
                       b'cholmod_l_print_perm',
                       b'cholmod_l_print_sparse',
                       b'cholmod_l_print_subset',
                       b'cholmod_l_print_triplet',
                       b'cholmod_l_ptranspose',
                       b'cholmod_l_rcond',
                       b'cholmod_l_read_dense',
                       b'cholmod_l_read_matrix',
                       b'cholmod_l_read_sparse',
                       b'cholmod_l_read_triplet',
                       b'cholmod_l_realloc',
                       b'cholmod_l_realloc_multiple',
                       b'cholmod_l_reallocate_column',
                       b'cholmod_l_reallocate_factor',
                       b'cholmod_l_reallocate_sparse',
                       b'cholmod_l_reallocate_triplet',
                       b'cholmod_l_resymbol',
                       b'cholmod_l_resymbol_noperm',
                       b'cholmod_l_row_lsubtree',
                       b'cholmod_l_row_subtree',
                       b'cholmod_l_rowadd',
                       b'cholmod_l_rowadd_mark',
                       b'cholmod_l_rowadd_solve',
                       b'cholmod_l_rowcolcounts',
                       b'cholmod_l_rowdel',
                       b'cholmod_l_rowdel_mark',
                       b'cholmod_l_rowdel_solve',
                       b'cholmod_l_rowfac',
                       b'cholmod_l_rowfac_mask',
                       b'cholmod_l_rowfac_mask2',
                       b'cholmod_l_scale',
                       b'cholmod_l_score_comp',
                       b'cholmod_l_sdmult',
                       b'cholmod_l_solve',
                       b'cholmod_l_solve2',
                       b'cholmod_l_sort',
                       b'cholmod_l_sparse_to_dense',
                       b'cholmod_l_sparse_to_triplet',
                       b'cholmod_l_sparse_xtype',
                       b'cholmod_l_speye',
                       b'cholmod_l_spsolve',
                       b'cholmod_l_spzeros',
                       b'cholmod_l_ssmult',
                       b'cholmod_l_start',
                       b'cholmod_l_submatrix',
                       b'cholmod_l_super_lsolve',
                       b'cholmod_l_super_ltsolve',
                       b'cholmod_l_super_numeric',
                       b'cholmod_l_super_symbolic',
                       b'cholmod_l_super_symbolic2',
                       b'cholmod_l_symmetry',
                       b'cholmod_l_transpose',
                       b'cholmod_l_transpose_sym',
                       b'cholmod_l_transpose_unsym',
                       b'cholmod_l_triplet_to_sparse',
                       b'cholmod_l_triplet_xtype',
                       b'cholmod_l_updown',
                       b'cholmod_l_updown_mark',
                       b'cholmod_l_updown_mask',
                       b'cholmod_l_updown_mask2',
                       b'cholmod_l_updown_solve',
                       b'cholmod_l_version',
                       b'cholmod_l_vertcat',
                       b'cholmod_l_write_dense',
                       b'cholmod_l_write_sparse',
                       b'cholmod_l_zeros',
                       b'cholmod_lsolve_pattern',
                       b'cholmod_malloc',
                       b'cholmod_maxrank',
                       b'cholmod_metis',
                       b'cholmod_metis_bisector',
                       b'cholmod_mult_size_t',
                       b'cholmod_nested_dissection',
                       b'cholmod_nnz',
                       b'cholmod_norm_dense',
                       b'cholmod_norm_sparse',
                       b'cholmod_ones',
                       b'cholmod_pack_factor',
                       b'cholmod_postorder',
                       b'cholmod_print_common',
                       b'cholmod_print_dense',
                       b'cholmod_print_factor',
                       b'cholmod_print_parent',
                       b'cholmod_print_perm',
                       b'cholmod_print_sparse',
                       b'cholmod_print_subset',
                       b'cholmod_print_triplet',
                       b'cholmod_ptranspose',
                       b'cholmod_rcond',
                       b'cholmod_read_dense',
                       b'cholmod_read_matrix',
                       b'cholmod_read_sparse',
                       b'cholmod_read_triplet',
                       b'cholmod_realloc',
                       b'cholmod_realloc_multiple',
                       b'cholmod_reallocate_column',
                       b'cholmod_reallocate_factor',
                       b'cholmod_reallocate_sparse',
                       b'cholmod_reallocate_triplet',
                       b'cholmod_resymbol',
                       b'cholmod_resymbol_noperm',
                       b'cholmod_row_lsubtree',
                       b'cholmod_row_subtree',
                       b'cholmod_rowadd',
                       b'cholmod_rowadd_mark',
                       b'cholmod_rowadd_solve',
                       b'cholmod_rowcolcounts',
                       b'cholmod_rowdel',
                       b'cholmod_rowdel_mark',
                       b'cholmod_rowdel_solve',
                       b'cholmod_rowfac',
                       b'cholmod_rowfac_mask',
                       b'cholmod_rowfac_mask2',
                       b'cholmod_scale',
                       b'cholmod_score_comp',
                       b'cholmod_sdmult',
                       b'cholmod_solve',
                       b'cholmod_solve2',
                       b'cholmod_sort',
                       b'cholmod_sparse_to_dense',
                       b'cholmod_sparse_to_triplet',
                       b'cholmod_sparse_xtype',
                       b'cholmod_speye',
                       b'cholmod_spsolve',
                       b'cholmod_spzeros',
                       b'cholmod_ssmult',
                       b'cholmod_start',
                       b'cholmod_submatrix',
                       b'cholmod_super_lsolve',
                       b'cholmod_super_ltsolve',
                       b'cholmod_super_numeric',
                       b'cholmod_super_symbolic',
                       b'cholmod_super_symbolic2',
                       b'cholmod_symmetry',
                       b'cholmod_transpose',
                       b'cholmod_transpose_sym',
                       b'cholmod_transpose_unsym',
                       b'cholmod_triplet_to_sparse',
                       b'cholmod_triplet_xtype',
                       b'cholmod_updown',
                       b'cholmod_updown_mark',
                       b'cholmod_updown_mask',
                       b'cholmod_updown_mask2',
                       b'cholmod_updown_solve',
                       b'cholmod_version',
                       b'cholmod_vertcat',
                       b'cholmod_write_dense',
                       b'cholmod_write_sparse',
                       b'cholmod_zeros'},

Maybe, only the name mangling changed for some reason?

mmuetzel commented 1 year ago

I built locally. And those symbols are exported by libcholmod.dll. I don't know why the package grokker is listing them. Maybe, it's best to just ignore that for now.

DrTimothyAldenDavis commented 1 year ago

I think all these issues are now resolve in the current dev branch of SuiteSparse. I'll tag a v6.0.2.beta1 shortly.

DrTimothyAldenDavis commented 1 year ago

Thanks again for all your help!