#32424 closed defect (fixed)

Upgrade to OpenBLAS 0.3.18 and make Sage fat binaries portable

Reported by: Matthias Köppe Owned by:
Priority: blocker Milestone: sage-9.5
Component: porting Keywords: upgrade, openblas
Cc: William Stein, gh-kliem, Dima Pasechnik, Nathan Dunfield, Marc Culler, Samuel Lelièvre, Michael Orlitzky, Volker Braun, Max Alekseyev, Thierry Monteil, John Palmieri Merged in:
Authors: Isuru Fernando, Matthias Koeppe, Volker Braun Reviewers: Matthias Koeppe, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 2f5f195 (Commits, GitHub, GitLab) Commit: 2f5f1957034aee0d9b130dae3f4e52f0e855986e
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

as reported in:

Related:

  • #32363 Rename configure option --enable-fat-binary to --enable-portable-binary
  • #32434 Do not require AVX when building with SAGE_FAT_BINARY

Change History (90)

comment:1 Changed 16 months ago by gh-kliem

The signal error is caused by nullspaceMP, which is in the iml library.

Iml itself doesn't use architecture specific instructions, but the dependencies are. But then again, OPENBLAS and gmp are already built with make DYNAMIC_ARCH=1 resp. --enable-fat. Both of them got updated between sage 9.0 and sage 9.3.

comment:2 Changed 16 months ago by Matthias Köppe

Description: modified (diff)

comment:3 Changed 16 months ago by Nathan Dunfield

I suspect the cause is OpenBLAS. While this library is built with DYNAMIC_ARCH=1, there is still the non-performance critical code in it which will use whatever instructions are available on the machine at compile time unless you also set TARGET. See

https://github.com/xianyi/OpenBLAS/issues/3056

For the macOS app, Marc Culler had to set TARGET=CORE2 in addition to DYNAMIC_ARCH=1 so that it would run on a 2013 cylindrical Mac Pro.

comment:4 Changed 16 months ago by Nathan Dunfield

Cc: Nathan Dunfield Marc Culler added

comment:5 Changed 16 months ago by Isuru Fernando

Authors: Isuru Fernando
Branch: u/isuruf/openblas-target
Commit: 343d6a15cfc21f69024d19bba66e73833ef58966
Status: newneeds_review

New commits:

cff0e2bset target for dynamic_arch
343d6a1AVX512 support is now much better

comment:6 Changed 16 months ago by Matthias Köppe

should we also throw in an openblas upgrade? https://repology.org/projects/o/?inrepo=sagemath_stable

comment:7 Changed 16 months ago by Isuru Fernando

Yes, that would be good to have. I didn't realize openblas was way behind. Feel free to push to this branch

comment:8 Changed 16 months ago by Matthias Köppe

on macOS:

[openblas-0.3.17] ./spkg-install: line 55: conditional binary operator expected
[openblas-0.3.17] ./spkg-install: line 55: syntax error near `~='
[openblas-0.3.17] ./spkg-install: line 55: `    if [[ $os-$machine ~= darwin-x86_64 ]]; then'

comment:9 Changed 16 months ago by Matthias Köppe

Branch: u/isuruf/openblas-targetu/mkoeppe/openblas-target

comment:10 Changed 16 months ago by Samuel Lelièvre

Cc: Samuel Lelièvre added
Commit: 343d6a15cfc21f69024d19bba66e73833ef58966c8cbf1fac644982d29c617669bb4d0958b18fa5c
Keywords: upgrade openblas added
Summary: Sage 9.4 still creates non-portable binaries with SAGE_FAT_BINARY="yes"Upgrade to OpenBLAS 0.3.17 and make Sage fat binaries portable

New commits:

c8cbf1fbuild/pkgs/openblas: Update to 0.3.17

comment:11 Changed 16 months ago by Matthias Köppe

Status: needs_reviewneeds_work

comment:12 Changed 16 months ago by Isuru Fernando

Branch: u/mkoeppe/openblas-targetu/isuruf/openblas-target
Commit: c8cbf1fac644982d29c617669bb4d0958b18fa5c1b03f2bde68d5548d483c43157cc4795c4579e23

New commits:

bed527eset target for dynamic_arch
825f704AVX512 support is now much better
1b03f2bbuild/pkgs/openblas: Update to 0.3.17

comment:13 Changed 15 months ago by Matthias Köppe

Status: needs_workneeds_review

comment:14 Changed 15 months ago by Matthias Köppe

Authors: Isuru FernandoIsuru Fernando, Matthias Koeppe
Reviewers: https://github.com/mkoeppe/sage/actions/runs/1175727870, https://github.com/mkoeppe/sage/actions/runs/1175727869

comment:15 Changed 15 months ago by Matthias Köppe

Reviewers: https://github.com/mkoeppe/sage/actions/runs/1175727870, https://github.com/mkoeppe/sage/actions/runs/1175727869https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1175727875, ...

comment:16 Changed 15 months ago by Matthias Köppe

Reviewers: https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1175727875, ...Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1175727875, ...

comment:17 Changed 15 months ago by Matthias Köppe

Reviewers: Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1175727875, ...Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1177591365, ...

comment:18 Changed 15 months ago by Matthias Köppe

Cc: Michael Orlitzky added

comment:19 Changed 15 months ago by Michael Orlitzky

For SAGE_FAT_BINARY, I think we have to set the TARGET to the "oldest" value that we expect the binaries to run on. If that's right, then using the uname will make it so that the binaries are portable only to other machines with the same (or "newer") uname.

CORE2 sounds like a good choice to me. Old enough to work for most people, but not crippled performance-wise.

comment:20 Changed 15 months ago by Isuru Fernando

If that's right, then using the uname will make it so that the binaries are portable only to other machines with the same (or "newer") uname.

That's correct. What's wrong with that? Do you expect binaries built on ppc64le to run on x86_64?

CORE2 sounds like a good choice to me. Old enough to work for most people, but not crippled performance-wise.

CORE2 is only for x86_64. Besides TARGET is needed only for non-performance critical code paths.

comment:21 in reply to:  20 Changed 15 months ago by Michael Orlitzky

Replying to isuruf:

If that's right, then using the uname will make it so that the binaries are portable only to other machines with the same (or "newer") uname.

That's correct. What's wrong with that?

Nothing, I didn't look closely enough at the cases. The only two that overlap in any way are x86_64 and x86, and setting TARGET to an x86 value on an x86_64 machine won't magically make it produce 32-bit binaries, so... LGTM now.

comment:22 Changed 15 months ago by Matthias Köppe

I think instead of uname, we should use $CC -dumpmachine like we do in build/pkgs/gfortran/spkg-build.in from #29241

comment:23 in reply to:  20 Changed 15 months ago by Matthias Köppe

Replying to isuruf:

If that's right, then using the uname will make it so that the binaries are portable only to other machines with the same (or "newer") uname.

That's correct. What's wrong with that? Do you expect binaries built on ppc64le to run on x86_64?

They often do, via qemu and binfmt_misc..., see https://hub.docker.com/r/multiarch/ubuntu-core

comment:24 Changed 15 months ago by Matthias Köppe

Cc: Volker Braun added

comment:25 Changed 15 months ago by Matthias Köppe

Cc: Max Alekseyev Thierry Monteil added
Description: modified (diff)

comment:26 Changed 15 months ago by Matthias Köppe

Description: modified (diff)

comment:27 Changed 15 months ago by Isuru Fernando

They often do, via qemu and binfmt_misc

That's not relevant either. qemu would emulate the instruction set including cpuid instruction.

comment:28 Changed 15 months ago by Matthias Köppe

But because of the binfmt_misc emulation route, you cannot rely on uname to reason about the target architecture, i.e., comment:22 applies not just for x86_64/i686.

comment:29 Changed 15 months ago by Isuru Fernando

But because of the binfmt_misc emulation route, you cannot rely on uname to reason about the target architecture, i.e., comment:22 applies not just for x86_64/i686.

Openblas spkg-install.in doesn't support building for non-native architecture even on develop branch.

comment:30 Changed 15 months ago by Matthias Köppe

We do test this at least for i686 on x86_64 - see for example https://github.com/sagemath/sage/runs/3392765581?check_suite_focus=true (which needed #29241)

comment:31 Changed 15 months ago by git

Commit: 1b03f2bde68d5548d483c43157cc4795c4579e2376dc19c9161051c52cf428dc666533a1cfbde013

Branch pushed to git repo; I updated commit sha1. New commits:

76dc19cuse compiler triplet

comment:32 Changed 15 months ago by Matthias Köppe

Reviewers: Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1176095684, https://github.com/mkoeppe/sage/actions/runs/1177591365, ...Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1192736161, https://github.com/mkoeppe/sage/actions/runs/1192736162

testing on top of 9.5.beta0

comment:33 Changed 15 months ago by Matthias Köppe

The tests on Linux and macOS (https://github.com/mkoeppe/sage/actions/runs/1192736161 ) look normal. Note that they do not test --enable-fat-binary at all. Unchanged: centos-7-i386 gives SIGFPE while building dochtml. Reported in https://groups.google.com/g/sage-release/c/ocdEAV4pFMo/m/4BEdJZQ8BAAJ for 9.4.rc1 (and probably earlier).

The test on Cygwin (https://github.com/mkoeppe/sage/actions/runs/1192736162) uses --enable-fat-binary. Unchanged: It gives the same segfaults whenever plotting is used (https://github.com/mkoeppe/sage/runs/3506881828) that showed up in the previous betas.

So this seems safe to merge, but it remains unknown whether it helps at all. Some help from the people who build and test binaries is needed here.

comment:34 Changed 15 months ago by Matthias Köppe

Description: modified (diff)

comment:35 Changed 15 months ago by Dima Pasechnik

Reviewers: Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/1192736161, https://github.com/mkoeppe/sage/actions/runs/1192736162Matthias Koeppe, Dima Pasechnik
Status: needs_reviewpositive_review

let's merge this then

comment:36 Changed 14 months ago by Volker Braun

Status: positive_reviewneeds_work

Segfaults on OSX

sage: for i in range(0,n2):
   A[i,i]=4*h2+1.
   if i+1<n2: A[i,int(i+1)]=-h2
   if i>0:    A[i,int(i-1)]=-h2
   if i+n<n2: A[i,int(i+n)]=-h2
   if i-n>=0: A[i,int(i-n)]=-h2 ## line 366 ##
sage: Acsc = A.tocsc() ## line 372 ##
sage: b = array([1 for i in range(0,n2)]) ## line 373 ##
sage: solve = factorized(Acsc) # LU factorization ## line 374 ##
------------------------------------------------------------------------
0   signals.cpython-39-darwin.so        0x0000000103db93e2 print_backtrace + 66
1   signals.cpython-39-darwin.so        0x0000000103dbd097 sigdie + 39
2   signals.cpython-39-darwin.so        0x0000000103dbcf84 cysigs_signal_handler + 404
3   libsystem_platform.dylib            0x00007fff7132b5fd _sigtramp + 29
4   ???                                 0x0000000000000013 0x0 + 19
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.
------------------------------------------------------------------------
**********************************************************************
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/tests/books/computational-mathematics-with-sagemath/linsolve_doctest.py  # Killed due to segmentation fault
----------------------------------------------------------------------

comment:37 Changed 14 months ago by Matthias Köppe

Obviously we need more detail about the platform

comment:38 Changed 14 months ago by Matthias Köppe

In the meantime 0.3.18 has come out

comment:39 Changed 14 months ago by Dima Pasechnik

Branch: u/isuruf/openblas-targetu/dimpase/packages/openblas/0.3.18
Commit: 76dc19c9161051c52cf428dc666533a1cfbde0135d6f68b7edd2ed151dce45f4df0b7413b61efb00
Status: needs_workneeds_review

New commits:

0ea5dc0set target for dynamic_arch
121dda1AVX512 support is now much better
d48c8abbuild/pkgs/openblas: Update to 0.3.17
d49adc0use compiler triplet
5d6f68bversion bump

comment:40 Changed 14 months ago by Dima Pasechnik

update to 0.3.18

comment:41 Changed 14 months ago by Matthias Köppe

Summary: Upgrade to OpenBLAS 0.3.17 and make Sage fat binaries portableUpgrade to OpenBLAS 0.3.18 and make Sage fat binaries portable

comment:42 Changed 14 months ago by Dima Pasechnik

Status: needs_reviewpositive_review

let's get this in, see if it helps with that not easy to reproduce bug.

comment:43 Changed 14 months ago by Volker Braun

Branch: u/dimpase/packages/openblas/0.3.185d6f68b7edd2ed151dce45f4df0b7413b61efb00
Resolution: fixed
Status: positive_reviewclosed

comment:44 Changed 14 months ago by Volker Braun

Branch: 5d6f68b7edd2ed151dce45f4df0b7413b61efb00u/dimpase/packages/openblas/0.3.18
Resolution: fixed
Status: closednew

still segfaults on mac in the same spot

comment:45 Changed 14 months ago by Dima Pasechnik

are you unmerging this ticket? it is a valid update, no regressions, right?

comment:46 Changed 14 months ago by Volker Braun

There is a regression, this used to work and is now segfaulting on osx:

>>> solve = factorized(Acsc) # LU factorization
Process 56588 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000107d55588 libopenblas_penrynp-r0.3.18.dylib`.L01
libopenblas_penrynp-r0.3.18.dylib`.L01:
->  0x107d55588 <+0>:  movaps %xmm4, (%rbp)
    0x107d5558c <+4>:  movaps %xmm4, 0x10(%rbp)
    0x107d55590 <+8>:  movaps %xmm4, 0x20(%rbp)
    0x107d55594 <+12>: movaps %xmm4, 0x30(%rbp)
Target 0: (python3) stopped.

comment:47 Changed 14 months ago by Volker Braun

Building Sage with

OPENBLAS_CONFIGURE="TARGET=PENRYN DEBUG=1 USE_OPENMP=0" make

works, so its apparently a threading-related issue. Kinda guessed that from the messed up stack trace, seems to be in a thread thats not doing so well.

comment:49 in reply to:  46 Changed 14 months ago by Dima Pasechnik

Replying to vbraun:

There is a regression, this used to work and is now segfaulting on osx:

Volker, more details please:

  • was OpenBLAS built from source?
  • On what version of macOS?
  • On what CPU?

comment:50 Changed 14 months ago by Dima Pasechnik

Cc: John Palmieri added
Status: newneeds_review

comment:51 Changed 13 months ago by Volker Braun

I'm running full builds; Does the following work for you? For me, it segfaults:

make distclean
OPENBLAS_CONFIGURE="TARGET=PENRYN" make -j10
sage -t --long --random-seed=0 src/sage/tests/books/computational-mathematics-with-sagemath/linsolve_doctest.py

Both Catalina and BigSur. CPU is qemu64 (lowest-common denominator, which is why openblas picks Penryn).

comment:52 Changed 13 months ago by Dima Pasechnik

hmm, so it's a qemu64 running macOS on a Mac?

Or a hackintosh, i.e. qemu running macOS on Linux?

How is this a meaningful test, and not testing the resolve of Apple to prevent "illegal" macOS emulators?

comment:53 in reply to:  51 ; Changed 13 months ago by Matthias Köppe

Replying to vbraun:

I'm running full builds; Does the following work for you? For me, it segfaults:

make distclean
OPENBLAS_CONFIGURE="TARGET=PENRYN" make -j10
sage -t --long --random-seed=0 src/sage/tests/books/computational-mathematics-with-sagemath/linsolve_doctest.py

Volker, this recipe is wildly underspecified. When you post failures like this, could you please attach at least the config.log and the build log of the package?

comment:54 Changed 13 months ago by Volker Braun

It currently works and an openblas update breaks it, for once thats not going to be Apple's fault.

BTW you have an account on catalina-osx, too ;)

comment:55 Changed 13 months ago by Isuru Fernando

Can we split the two things in this ticket?

Fixing fat binaries and upgrading openblas?

comment:56 in reply to:  54 Changed 13 months ago by Dima Pasechnik

Replying to vbraun:

It currently works and an openblas update breaks it, for once thats not going to be Apple's fault.

you might be just hitting a deficiency in your Hackingtosh, not a real bug.

BTW you have an account on catalina-osx, too ;)

comment:57 Changed 13 months ago by Volker Braun

comment:58 Changed 13 months ago by Dima Pasechnik

from config.log one can see you have another OpenBLAS installed on the box (pkg-config finds it)

Can you, just as a sanitary check, uninstall it and check that you can reproduce the crash?

comment:59 Changed 13 months ago by Volker Braun

I don't have another openblas; it seems the configure prints configure:12750: result: openblas by default even if you don't have any blas:

  12734 # Check whether --with-blas was given.
  12735 if test "${with_blas+set}" = set; then :
  12736   withval=$with_blas;
  12737 else
  12738   with_blas=openblas  # default
  12739 
  12740 fi
  12741 
  12742   case "$with_blas" in #(
  12743   openblas) :
  12744      ;; #(
  12745   atlas) :
  12746     sage_spkg_install_openblas=no ;; #(
  12747   *) :
  12748     as_fn_error $? "allowed values for --with-blas are 'atlas' and 'openblas'" "$LINENO" 5 ;;
  12749 esac
  12750   { $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_blas" >&5

comment:60 Changed 13 months ago by Dima Pasechnik

maybe it needs gfortran 11?

comment:61 Changed 13 months ago by Dima Pasechnik

also, one might need more details on the hist CPU and how qemu is started/configured.

comment:62 Changed 13 months ago by Matthias Köppe

Branch: u/dimpase/packages/openblas/0.3.18u/mkoeppe/packages/openblas/0.3.18

comment:63 in reply to:  53 ; Changed 13 months ago by Matthias Köppe

Commit: 5d6f68b7edd2ed151dce45f4df0b7413b61efb0018044f326508a898561fb787a902279b1a527ee6

Replying to mkoeppe:

Replying to vbraun:

I'm running full builds; Does the following work for you? For me, it segfaults:

make distclean
OPENBLAS_CONFIGURE="TARGET=PENRYN" make -j10
sage -t --long --random-seed=0 src/sage/tests/books/computational-mathematics-with-sagemath/linsolve_doctest.py

Based on the provided logs, I have built a similar configuration using OPENBLAS_CONFIGURE=TARGET=PENRYN tox -e local-macos-nohomebrew -- bash on a real Mac (Intelr Core i9, Big Sur). The tests pass.

I'll try again now after merging 9.5.beta5.


New commits:

143e907tox.ini (local): passenv OPENBLAS_CONFIGURE
18044f3Merge tag '9.5.beta5' into t/32424/openblas-target

comment:64 Changed 13 months ago by Matthias Köppe

Volker, I'd suggest that you take the problem on your configuration upstream by trying to reproduce it outside of Sage.

comment:65 in reply to:  63 Changed 13 months ago by Matthias Köppe

Replying to mkoeppe:

I'll try again now after merging 9.5.beta5.

Same result, all good.

comment:66 Changed 13 months ago by Matthias Köppe

Status: needs_reviewpositive_review

comment:67 Changed 13 months ago by Volker Braun

Status: positive_reviewneeds_work

comment:68 Changed 13 months ago by Volker Braun

Something is definitely not right. Its not just TARGET=PENRYN, with that it also works for me.

comment:69 Changed 13 months ago by Matthias Köppe

So what is the configuration that fails for you?

comment:70 Changed 13 months ago by Volker Braun

This is the Makefile.conf that segfaults:

OSNAME=Darwin
ARCH=x86_64
C_COMPILER=CLANG
BINARY32=
BINARY64=1
FU=_
CEXTRALIB=-L/Users/vbraun/Sage/local/lib -L/usr/local/lib  -lto_library -lSystem  /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/lib/darwin/libclang_rt.osx.a 
F_COMPILER=GFORTRAN
FC=gfortran
BU=_
FEXTRALIB=-L/Users/vbraun/Sage/local/lib -L/Users/vbraun/Sage/local/lib/gcc/x86_64-apple-darwin19.6.0/10.3.0 -L/Users/vbraun/Sage/local/lib/gcc/x86_64-apple-darwin19.6.0/10.3.0/../../.. -L/Users/vbraun/Sage/local/lib -L/Users/vbraun/Sage/local/lib/gcc/x86_64-apple-darwin19.6.0/10.3.0 -L/Users/vbraun/Sage/local/lib/gcc/x86_64-apple-darwin19.6.0/10.3.0/../../..  -lgfortran -lSystem -lquadmath -lm -lSystem -lgfortran -lSystem -lquadmath -lm -lSystem  
CORE=PENRYN
LIBCORE=penryn
NUM_CORES=8
HAVE_MMX=1
HAVE_SSE=1
HAVE_SSE2=1
HAVE_SSE3=1
HAVE_SSSE3=1
HAVE_SSE4_1=1
HAVE_SSE4_2=1
HAVE_AVX=1
MAKE += -j 8
SBGEMM_UNROLL_M=8
SBGEMM_UNROLL_N=4
SGEMM_UNROLL_M=8
SGEMM_UNROLL_N=4
DGEMM_UNROLL_M=4
DGEMM_UNROLL_N=4
QGEMM_UNROLL_M=2
QGEMM_UNROLL_N=2
CGEMM_UNROLL_M=4
CGEMM_UNROLL_N=2
ZGEMM_UNROLL_M=2
ZGEMM_UNROLL_N=2
XGEMM_UNROLL_M=1
XGEMM_UNROLL_N=1
CGEMM3M_UNROLL_M=8
CGEMM3M_UNROLL_N=4
ZGEMM3M_UNROLL_M=4
ZGEMM3M_UNROLL_N=4
XGEMM3M_UNROLL_M=2
XGEMM3M_UNROLL_N=2
Last edited 13 months ago by Volker Braun (previous) (diff)

comment:71 Changed 13 months ago by Volker Braun

The build log is identical to one with OPENBLAS_CONFIGURE="TARGET=PENRYN HAVE_AVX=1" except that the latter doesn't segfault, so it must be in the config.h value

comment:72 Changed 13 months ago by Volker Braun

config.h:

#define OS_DARWIN	1
#define ARCH_X86_64	1
#define C_CLANG	1
#define __64BIT__	1
#define FUNDERSCORE	_
#define HAVE_C11	1
#define PTHREAD_CREATE_FUNC	pthread_create
#define BUNDERSCORE	_
#define NEEDBUNDERSCORE	1
#define PENRYN
#define L1_CODE_SIZE 32768
#define L1_CODE_ASSOCIATIVE 8
#define L1_CODE_LINESIZE 64
#define L1_DATA_SIZE 32768
#define L1_DATA_ASSOCIATIVE 8
#define L1_DATA_LINESIZE 64
#define L2_SIZE 2097152
#define L2_ASSOCIATIVE 8
#define L2_LINESIZE 64
#define L3_SIZE 16777216
#define L3_ASSOCIATIVE 16
#define L3_LINESIZE 64
#define DTB_DEFAULT_ENTRIES 32
#define HAVE_CMOV
#define HAVE_MMX
#define HAVE_SSE
#define HAVE_SSE2
#define HAVE_SSE3
#define HAVE_SSSE3
#define HAVE_SSE4_1
#define HAVE_SSE4_2
#define HAVE_AVX
#define HAVE_CFLUSH
#define NUM_SHAREDCACHE 1
#define NUM_CORES 1
#define CORE_PENRYN
#define CHAR_CORENAME "PENRYN"
#define SLOCAL_BUFFER_SIZE	32768
#define DLOCAL_BUFFER_SIZE	16384
#define CLOCAL_BUFFER_SIZE	32768
#define ZLOCAL_BUFFER_SIZE	16384
#define GEMM_MULTITHREAD_THRESHOLD	4

comment:73 Changed 13 months ago by Matthias Köppe

My configuration (works):

Makefile.conf:

OSNAME=Darwin
ARCH=x86_64
C_COMPILER=CLANG
BINARY32=
BINARY64=1
FU=_
CEXTRALIB=-L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib  -lto_library -lSystem  /Library/Developer/CommandLineTools/usr/lib/clang/13.0.0/lib/darwin/libclang_rt.osx.a 
F_COMPILER=GFORTRAN
FC=gfortran
BU=_
FEXTRALIB=-L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib -L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib/gcc/x86_64-apple-darwin20.6.0/10.3.0 -L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib/gcc/x86_64-apple-darwin20.6.0/10.3.0/../../.. -L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib -L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib/gcc/x86_64-apple-darwin20.6.0/10.3.0 -L/Users/mkoeppe/s/sage/sage-rebasing/.tox/local-macos-nohomebrew/local/lib/gcc/x86_64-apple-darwin20.6.0/10.3.0/../../..  -lgfortran -lSystem -lquadmath -lm -lSystem -lgfortran -lSystem -lquadmath -lm -lSystem  
CORE=PENRYN
LIBCORE=penryn
NUM_CORES=12
PENRYN=1
L1_DATA_SIZE=32768
L1_DATA_LINESIZE=64
L2_SIZE=1048576
L2_LINESIZE=64
DTB_DEFAULT_ENTRIES=256
DTB_SIZE=4096
HAVE_CMOV=1
HAVE_MMX=1
HAVE_SSE=1
HAVE_SSE2=1
HAVE_SSE3=1
HAVE_SSSE3=1
HAVE_SSE4_1=1
MAKE += -j 12
SBGEMM_UNROLL_M=8
SBGEMM_UNROLL_N=4
SGEMM_UNROLL_M=8
SGEMM_UNROLL_N=4
DGEMM_UNROLL_M=4
DGEMM_UNROLL_N=4
QGEMM_UNROLL_M=2
QGEMM_UNROLL_N=2
CGEMM_UNROLL_M=4
CGEMM_UNROLL_N=2
ZGEMM_UNROLL_M=2
ZGEMM_UNROLL_N=2
XGEMM_UNROLL_M=1
XGEMM_UNROLL_N=1
CGEMM3M_UNROLL_M=8
CGEMM3M_UNROLL_N=4
ZGEMM3M_UNROLL_M=4
ZGEMM3M_UNROLL_N=4
XGEMM3M_UNROLL_M=2
XGEMM3M_UNROLL_N=2

config.h:

#define OS_DARWIN	1
#define ARCH_X86_64	1
#define C_CLANG	1
#define __64BIT__	1
#define FUNDERSCORE	_
#define HAVE_C11	1
#define PTHREAD_CREATE_FUNC	pthread_create
#define BUNDERSCORE	_
#define NEEDBUNDERSCORE	1
#define PENRYN
#define L1_DATA_SIZE 32768
#define L1_DATA_LINESIZE 64
#define L2_SIZE 1048576
#define L2_LINESIZE 64
#define DTB_DEFAULT_ENTRIES 256
#define DTB_SIZE 4096
#define HAVE_CMOV
#define HAVE_MMX
#define HAVE_SSE
#define HAVE_SSE2
#define HAVE_SSE3
#define HAVE_SSSE3
#define HAVE_SSE4_1
#define CORE_PENRYN
#define CHAR_CORENAME "PENRYN"
#define SLOCAL_BUFFER_SIZE	32768
#define DLOCAL_BUFFER_SIZE	16384
#define CLOCAL_BUFFER_SIZE	32768
#define ZLOCAL_BUFFER_SIZE	16384
#define GEMM_MULTITHREAD_THRESHOLD	4

comment:74 in reply to:  71 ; Changed 13 months ago by Matthias Köppe

Replying to vbraun:

The build log is identical to one with OPENBLAS_CONFIGURE="TARGET=PENRYN HAVE_AVX=1" except that the latter doesn't segfault, so it must be in the config.h value

The files that you showed in comment:70, comment:72 have HAVE_AVX, so what's going on?

comment:75 Changed 13 months ago by Volker Braun

PENRYN=1 is apparently not the same as CORE=PENRYN

I do have AVX (and building with TARGET=PENRYN HAVE_AVX=1 works fine):

  <qemu:commandline>
    <qemu:arg value="-device"/>
    <qemu:arg value="isa-applesmc,osk=ourhardworkbythesewordsguardedpleasedontsteal(c)AppleComputerInc"/>
    <qemu:arg value="-smbios"/>
    <qemu:arg value="type=2"/>
    <qemu:arg value="-device"/>
    <qemu:arg value="vmware-svga"/>
    <qemu:arg value="-cpu"/>
    <qemu:arg value="Penryn,kvm=on,vendor=GenuineIntel,+invtsc,vmware-cpuid-freq=on,+pcid,+ssse3,+sse4.2,+popcnt,+avx,+aes,+xsave,+xsaveopt,check"/>
  </qemu:commandline>

comment:76 in reply to:  74 Changed 13 months ago by Matthias Köppe

Replying to mkoeppe:

Replying to vbraun:

The build log is identical to one with OPENBLAS_CONFIGURE="TARGET=PENRYN HAVE_AVX=1" except that the latter doesn't segfault, so it must be in the config.h value

The files that you showed in comment:70, comment:72 have HAVE_AVX, so what's going on?

So when you pass TARGET=PENRYN HAVE_AVX=1, how do config.h and Makefile.conf change compared to what you posted in comment:70, comment:72?

comment:77 Changed 13 months ago by Volker Braun

  • src-segfault is from running make
  • src-working is from running make TARGET=PENRYN HAVE_AVX=1

config.h

vbraun@catalina-osx openblas-test % diff -u src-segfault/config.h src-working/config.h
--- src-segfault/config.h	2021-11-01 15:01:57.000000000 -0700
+++ src-working/config.h	2021-11-01 15:02:16.000000000 -0700
@@ -8,19 +8,12 @@
 #define BUNDERSCORE	_
 #define NEEDBUNDERSCORE	1
 #define PENRYN
-#define L1_CODE_SIZE 32768
-#define L1_CODE_ASSOCIATIVE 8
-#define L1_CODE_LINESIZE 64
 #define L1_DATA_SIZE 32768
-#define L1_DATA_ASSOCIATIVE 8
 #define L1_DATA_LINESIZE 64
-#define L2_SIZE 2097152
-#define L2_ASSOCIATIVE 8
+#define L2_SIZE 1048576
 #define L2_LINESIZE 64
-#define L3_SIZE 16777216
-#define L3_ASSOCIATIVE 16
-#define L3_LINESIZE 64
-#define DTB_DEFAULT_ENTRIES 32
+#define DTB_DEFAULT_ENTRIES 256
+#define DTB_SIZE 4096
 #define HAVE_CMOV
 #define HAVE_MMX
 #define HAVE_SSE
@@ -28,11 +21,6 @@
 #define HAVE_SSE3
 #define HAVE_SSSE3
 #define HAVE_SSE4_1
-#define HAVE_SSE4_2
-#define HAVE_AVX
-#define HAVE_CFLUSH
-#define NUM_SHAREDCACHE 1
-#define NUM_CORES 1
 #define CORE_PENRYN
 #define CHAR_CORENAME "PENRYN"
 #define SLOCAL_BUFFER_SIZE	32768

Makefile.conf

vbraun@catalina-osx openblas-test % diff -u src-segfault/Makefile.conf src-working/Makefile.conf 
--- src-segfault/Makefile.conf	2021-11-01 15:01:57.000000000 -0700
+++ src-working/Makefile.conf	2021-11-01 15:02:16.000000000 -0700
@@ -12,14 +12,20 @@
 CORE=PENRYN
 LIBCORE=penryn
 NUM_CORES=8
+PENRYN=1
+L1_DATA_SIZE=32768
+L1_DATA_LINESIZE=64
+L2_SIZE=1048576
+L2_LINESIZE=64
+DTB_DEFAULT_ENTRIES=256
+DTB_SIZE=4096
+HAVE_CMOV=1
 HAVE_MMX=1
 HAVE_SSE=1
 HAVE_SSE2=1
 HAVE_SSE3=1
 HAVE_SSSE3=1
 HAVE_SSE4_1=1
-HAVE_SSE4_2=1
-HAVE_AVX=1
 MAKE += -j 8
 SBGEMM_UNROLL_M=8
 SBGEMM_UNROLL_N=4

comment:78 Changed 13 months ago by Volker Braun

Its weird that HAVE_AVX isn't set in the working case, but I'm pretty sure AVX isn't the reason. In particular, building with OPENBLAS_CONFIGURE="NO_AVX=1" segfaults as well, and building with OPENBLAS_CONFIGURE="TARGET=PENRYN HAVE_AVX=1" works.

comment:79 Changed 13 months ago by Volker Braun

The culprit seems to be

#define DTB_DEFAULT_ENTRIES 32

increasing it to 64, say, fixes the segfault

comment:80 Changed 13 months ago by Matthias Köppe

Report upstream?

comment:81 Changed 13 months ago by Volker Braun

I've made a branch at u/vbraun/packages/openblas/0.3.18-test that patches DTB_DEFAULT_ENTRIES=32 when forcing TARGET=PENRYN, with that I can reproduce the same segfault on linux when running

OPENBLAS_CONFIGURE="TARGET=PENRYN" ./sage -p openblas
./sage -t --long  src/sage/tests/books/computational-mathematics-with-sagemath/linsolve_doctest.py

comment:82 Changed 13 months ago by Volker Braun

$ valgrind ./local/bin/python
[...]
>>> solve = factorized(Acsc) # LU factorization
==727003== Invalid write of size 8
==727003==    at 0x143AC900: dgemv_n (dgemv_n.S:233)
==727003==    by 0xFFFFFFFFFFFFFFFE: ???
==727003==    by 0x1F: ???
==727003==    by 0x1E: ???
==727003==    by 0xA75C8947: ???
==727003==    by 0x9F0E438F: ???
==727003==    by 0x4CF: ???
==727003==    by 0xBFEFFFFFFFFFFFFF: ???
==727003==    by 0xFFFFFFFFFFE00009: ???
==727003==    by 0x1F: ???
==727003==    by 0x9F0DAA87: ???
==727003==    by 0x98: ???
==727003==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Though dgemv_n.S hasn't been touched in 8 years ;)

comment:83 Changed 13 months ago by Volker Braun

This is https://github.com/xianyi/OpenBLAS/issues/3421, patch is in the linked PR

comment:84 Changed 13 months ago by Volker Braun

Branch: u/mkoeppe/packages/openblas/0.3.18u/vbraun/packages/openblas/0.3.18

comment:85 Changed 13 months ago by Matthias Köppe

Commit: 18044f326508a898561fb787a902279b1a527ee62f5f1957034aee0d9b130dae3f4e52f0e855986e
Reviewers: Matthias Koeppe, Dima PasechnikMatthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/1429845185, https://github.com/mkoeppe/sage/actions/runs/1429845182
Status: needs_workneeds_review

Testing with a bunch of other upgrade tickets


New commits:

2f5f195Add upstream fix for segfault in factorize

comment:86 Changed 13 months ago by Matthias Köppe

Reviewers: Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/1429845185, https://github.com/mkoeppe/sage/actions/runs/1429845182Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/1430376304, https://github.com/mkoeppe/sage/actions/runs/1430376300

comment:87 Changed 13 months ago by Matthias Köppe

Reviewers: Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/1430376304, https://github.com/mkoeppe/sage/actions/runs/1430376300Matthias Koeppe, Dima Pasechnik
Status: needs_reviewpositive_review

Tests look good

comment:88 Changed 13 months ago by Matthias Köppe

Authors: Isuru Fernando, Matthias KoeppeIsuru Fernando, Matthias Koeppe, Volker Braun

comment:89 Changed 13 months ago by Samuel Lelièvre

Priority: criticalblocker

comment:90 Changed 13 months ago by Volker Braun

Branch: u/vbraun/packages/openblas/0.3.182f5f1957034aee0d9b130dae3f4e52f0e855986e
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.