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

[RFC]: Upgrade OpenBLAS dependency #2228

Open
3 tasks done
rreusser opened this issue May 3, 2024 · 4 comments · May be fixed by #2236
Open
3 tasks done

[RFC]: Upgrade OpenBLAS dependency #2228

rreusser opened this issue May 3, 2024 · 4 comments · May be fixed by #2236
Labels
RFC Request for comments. Feature requests and proposed changes.

Comments

@rreusser
Copy link
Member

rreusser commented May 3, 2024

Description

OpenBLAS 0.2.19 is almost eight years out of date.

As a result, I ran into issues running make install-deps-openblas. See truncated error logs below:
... 
Makefile:488: warning: undefined variable `CCBLASOBJS'
Makefile:488: warning: undefined variable `CCBLASOBJS_P'
Makefile:488: warning: undefined variable `CZBLASOBJS'
Makefile:488: warning: undefined variable `CZBLASOBJS_P'
Makefile:488: warning: undefined variable `CXBLASOBJS'
Makefile:488: warning: undefined variable `CXBLASOBJS_P'

<output truncated>

gemm.c:403:103: error: use of undeclared identifier 'GEMM_DEFAULT_OFFSET_B'
  sb = (XFLOAT *)(((BLASLONG)sa + ((GEMM_P * GEMM_Q * COMPSIZE * SIZE + GEMM_ALIGN) & ~GEMM_ALIGN)) + GEMM_OFFSET_B);
                                                                                                      ^   
../common_param.h:972:23: note: expanded from macro 'GEMM_OFFSET_B'
#define GEMM_OFFSET_B   GEMM_DEFAULT_OFFSET_B
                        ^   
6 errors generated.
make[2]: *** [sgemm.o] Error 1
make[1]: *** [libs] Error 1
make: *** [deps-install-openblas] Error 2

Upgrading to 0.3.27 resulted in successful installation. I propose to upgrade the repo URL, checksum, and version to result in successful OpenBLAS installation.

Related Issues

No response

Questions

  • Do we need to lock down more build flags along with this? See:

    stdlib/tools/make/common.mk

    Lines 406 to 480 in 466b5aa

    # Define the OpenBLAS version:
    DEPS_OPENBLAS_VERSION ?= 0.2.19
    # Generate a version slug:
    deps_openblas_version_slug := $(subst .,_,$(DEPS_OPENBLAS_VERSION))
    # Define the output path when building OpenBLAS:
    DEPS_OPENBLAS_BUILD_OUT ?= $(DEPS_BUILD_DIR)/openblas_$(deps_openblas_version_slug)
    # Host architecture:
    DEPS_OPENBLAS_ARCH := $(shell $(CC) -dumpmachine | sed "s/\([^-]*\).*$$/\1/")
    # Target binary (32-bit or 64-bit):
    DEPS_OPENBLAS_BINARY ?= 64
    # Target architecture (cross-compiling):
    DEPS_OPENBLAS_TARGET_ARCH ?=
    # Host C compiler (cross-compiling):
    DEPS_OPENBLAS_HOSTCC ?=
    # C compiler flags:
    DEPS_OPENBLAS_CFLAGS ?=
    # Fortran compiler flags:
    DEPS_OPENBLAS_FFLAGS ?= -O3 $(fPIC)
    # Flag indicating whether to use clang:
    DEPS_OPENBLAS_USE_CLANG ?=
    # Specify stack alignment on Windows.
    #
    # [1]: https://gcc.gnu.org/onlinedocs/gcc-4.5.3/gcc/i386-and-x86_002d64-Options.html
    ifeq ($(OS), WINNT)
    ifneq ($(DEPS_OPENBLAS_ARCH), x86_64)
    ifneq ($(DEPS_OPENBLAS_USE_CLANG), 1)
    DEPS_OPENBLAS_CFLAGS += -mincoming-stack-boundary=2
    endif
    DEPS_OPENBLAS_FFLAGS += -mincoming-stack-boundary=2
    endif
    endif
    # Unless for distribution (i.e., a need exists for supporting multiple architectures in a single binary), disable building for all architectures:
    DEPS_OPENBLAS_DYNAMIC_ARCH ?= 0
    # Define whether to compile a debug build:
    DEPS_OPENBLAS_DEBUG ?= 0
    # Specify whether to build a 64-bit (8 byte integers) BLAS interface (not all Fortran compilers support this; safe to disable):
    DEPS_OPENBLAS_USE_BLAS64 ?= 0
    # When building a 64-bit BLAS interface, add a prefix and/or suffix to all exported symbol names in the shared library. Doing so helps avoid conflicts with other BLAS libraries, especially when using 64-bit integer interfaces in OpenBLAS. Note that the same prefix and suffix are added to the library name: `lib$(SYMBOLPREFIX)openblas$(SYMBOLSUFFIX)` rather than `libopenblas`.
    DEPS_OPENBLAS_SYMBOLSUFFIX ?=
    DEPS_OPENBLAS_SYMBOLPREFIX ?=
    # Define whether to use threading (determined automatically if not specified):
    DEPS_OPENBLAS_USE_THREAD ?=
    # Specify whether to use the AVX kernel on Sandy Bridge.
    DEPS_OPENBLAS_NO_AVX ?= 1
    # Specify whether to use Haswell optimizations if binutils is too old (e.g. RHEL6):
    DEPS_OPENBLAS_NO_AVX2 ?= 1
    # Specify whether to compile CBLAS:
    DEPS_OPENBLAS_NO_CBLAS ?= 0
    # Specify whether to only compile CBLAS:
    DEPS_OPENBLAS_ONLY_CBLAS ?= 0
    # Specify whether to compile LAPACK (also disables compiling the C interface to LAPACK):
    DEPS_OPENBLAS_NO_LAPACK ?= 0
    # Specify whether to compile the C interface to LAPACK:
    DEPS_OPENBLAS_NO_LAPACKE ?= 0

  • Are there other compatibility issues/concerns/validations which would need to accompany this change?

Other

No.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@kgryte
Copy link
Member

kgryte commented May 3, 2024

@rreusser This would be a welcome addition. And indeed, upgrading OpenBLAS has been a TODO.

@kgryte
Copy link
Member

kgryte commented May 3, 2024

@rreusser I originally based everything on how Julia manages OpenBLAS: https://github.com/JuliaLang/julia/blob/master/deps/openblas.mk

@kgryte
Copy link
Member

kgryte commented May 3, 2024

You can probably cross-reference there to see if we are missing anything in terms of build flags.

@kgryte kgryte added the RFC Request for comments. Feature requests and proposed changes. label May 3, 2024
@kgryte
Copy link
Member

kgryte commented May 3, 2024

...and no backward compatibility concerns we need to work about. Previous OpenBLAS support via stdlib was always experimental.

rreusser added a commit that referenced this issue May 7, 2024
This commit upgrade OpenBLAS from 0.2.19, which is now eight years out
of date, to 0.3.27. Note that OpenBLAS support is experimental in the
first place.

Fixes: #2228
@rreusser rreusser linked a pull request May 7, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants