Reference-LAPACK / lapack

LAPACK development repository
Other
1.49k stars 436 forks source link

CLAPACK test programs reporting stack overflow on Windows #987

Open phetdam opened 7 months ago

phetdam commented 7 months ago

Description

Several of the CLAPACK test programs, mostly the linear algebra ones, e.g. xeigtstz, xlintstc, use quite a bit of stack space, which is a problem on Windows since the default stack size reserved by the MSVC linker is 1 MB. These test programs all crash since they exhaust the stack, but this can be worked around by passing a larger value via /STACK to the MSVC linker.

Steps to reproduce from my original report:

  1. cd into clapack-3.2.1-CMAKE top-level dir
  2. Build: cmake -S . -B build_windows_x86 -A Win32 && cmake --build build_windows_x86 --config Debug
  3. Test: ctest --test-dir build_windows_x86 -C Debug

Originally, I noticed this when I built the CLAPACK 3.2.1 tarball that includes the CMake configuration, i.e. clapack-3.2.1-CMAKE.tgz from the Netlib CLAPACK page. After poking around a bit, I noticed in TESTING/CMakeLists.txt these lines:

https://github.com/Reference-LAPACK/lapack/blob/c6bc40164ace127bc308da37b135791a9ecb4667/TESTING/CMakeLists.txt#L7-L12

Clearly there was an effort before to work around the smaller Windows stack size. Unfortunately, this is ineffective, as at least for Visual Studio 2019 and 2022 I reproduced the issue on, no /STACK parameter is passed to the MSVC linker by default.

Fortunately, appending /STACK:12000000 (~12 MB stack size) to CMAKE_EXE_LINKER_FLAGS was relatively easy to do and fixed the issue. I determined the stack space to reserve very crudely via experimentation, first trying 10 MB, then 12 MB.

Reporting issue here since CLAPACK forum moved to the mailing list which said to use the GitHub issue tracker.

Checklist

langou commented 7 months ago

Thanks Derek. This is a known issue but you explain it very clearly. CLAPACK is not part of this GitHub repository. But it is fine to report the issue here. Thanks for doing this.

phetdam commented 7 months ago

No problem, thanks for the kind words. I ran into this a while ago but finally found some time to poke around.

Glad that it was relatively easy to fix by replacing the TESTING/CMakeLists.txt lines shown above with this:

if(MSVC)
  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:12000000")
endif()

Update: Confirmed this works for both debug + release configurations as well as for both 32- and 64-bit builds.

martin-frbg commented 7 months ago

IIRC this has been fixed in LAPACK quite a while ago by changing the allocation method used for several huge arrays in the testsuite. Unfortunately CLAPACK has not been updated in a long time and many current LAPACK routines can no longer be machine translated with the equally ancient f2c as they use newer language features of Fortran.

phetdam commented 7 months ago

Got it, thanks for the information. I've been working on a pretty ancient codebase that depends on CLAPACK, albeit in a bit of a roundabout manner to avoid including f2c.h directly, but maybe it's time to considering migrating to LAPACKE instead.