Re: [Buildroot] [PATCH v4 3/3] package/armadillo: allows to select between clapack, lapack or openblas

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Arnout, All, 

On 2021-07-24 23:45 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly:
> From: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
> 
> armadillo can use clapack, lapack or openblas as BLAS provider and
> clapack or lapack as LAPACK provider.
> 
> This patch
> - adds hidden variable to check dependencies/requirement for each of them
> - adds an option to use openblas as BLAS provider
> 
> The choice is required since applications may potentially need
> lapack/clapack.
> 
> Note that a choice between lapack and clapack is hard to do in Kconfig,
> because it inevitably leads to a circular dependency.
> 
> Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> Changes v3 -> v4: [Done by Arnout]
> - Split off lapack/clapack arch depends in separate patches
> - Simplify the comments (no need for the powerpc complexity)
> - Remove the choices because clapack/lapack choice doesn't work. Keep a
>   single config for openblas.
> Changes v2 -> v3:
> - drop all default statements for choice (Thomas)
> - add explicit -l since blas libary is called liblas for (c)lapack and
>   libopenblas for openblas (Thomas)
> - add a choice for lapack selection between lapack, clapack or none
> Changes v1 -> v2:
> - add openblas as blas provider
> ---
>  package/armadillo/Config.in    | 32 +++++++++++++++++++++++---------
>  package/armadillo/armadillo.mk | 28 +++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/package/armadillo/Config.in b/package/armadillo/Config.in
> index b2b61a3233..710da0314f 100644
> --- a/package/armadillo/Config.in
> +++ b/package/armadillo/Config.in
> @@ -1,20 +1,34 @@
> +config BR2_PACKAGE_ARMADILLO_LAPACK_BLAS_SUPPORTS
> +	bool
> +	default y if BR2_PACKAGE_CLAPACK_ARCH_SUPPORTS
> +	default y if BR2_PACKAGE_LAPACK_ARCH_SUPPORTS && BR2_TOOLCHAIN_HAS_FORTRAN
> +	default y if BR2_PACKAGE_OPENBLAS_ARCH_SUPPORTS
> +
>  comment "armadillo needs a toolchain w/ C++"
> +	depends on BR2_PACKAGE_ARMADILLO_LAPACK_BLAS_SUPPORTS
>  	depends on !BR2_INSTALL_LIBSTDCPP
> -	depends on !BR2_powerpc
> -	depends on !BR2_m68k_cf
> -
> -comment "armadillo needs a glibc toolchain w/ C++"
> -	depends on BR2_powerpc
> -	depends on !BR2_INSTALL_LIBSTDCPP || BR2_TOOLCHAIN_USES_UCLIBC
>  
>  config BR2_PACKAGE_ARMADILLO
>  	bool "armadillo"
> +	depends on BR2_PACKAGE_ARMADILLO_LAPACK_BLAS_SUPPORTS
>  	depends on BR2_INSTALL_LIBSTDCPP
> -	depends on !BR2_powerpc || BR2_TOOLCHAIN_USES_GLIBC # clapack
> -	depends on !BR2_m68k_cf # clapack
> -	select BR2_PACKAGE_CLAPACK
> +	select BR2_PACKAGE_CLAPACK if !BR2_PACKAGE_LAPACK && !BR2_PACKAGE_ARMADILLO_BLAS_OPENBLAS
>  	help
>  	  Armadillo: An Open Source C++ Linear Algebra Library for
>  	  Fast Prototyping and Computationally Intensive Experiments.
>  
>  	  http://arma.sourceforge.net/
> +
> +if BR2_PACKAGE_ARMADILLO
> +
> +config BR2_PACKAGE_ARMADILLO_BLAS_OPENBLAS

Note the option name here ^^^ [...]

> +	bool "use openblas"
> +	depends on BR2_PACKAGE_OPENBLAS_ARCH_SUPPORTS
> +	select BR2_PACKAGE_OPENBLAS
> +	help
> +	  Use OpenBLAS as BLAS library. Without this option, clapack or lapack
> +	  will be used.
> +
> +endchoice
> +
> +endif
> diff --git a/package/armadillo/armadillo.mk b/package/armadillo/armadillo.mk
> index 624b842ef6..82df7602be 100644
> --- a/package/armadillo/armadillo.mk
> +++ b/package/armadillo/armadillo.mk
> @@ -7,11 +7,37 @@
>  ARMADILLO_VERSION = 9.900.2
>  ARMADILLO_SOURCE = armadillo-$(ARMADILLO_VERSION).tar.xz
>  ARMADILLO_SITE = https://downloads.sourceforge.net/project/arma
> -ARMADILLO_DEPENDENCIES = clapack
>  ARMADILLO_INSTALL_STAGING = YES
>  ARMADILLO_LICENSE = Apache-2.0
>  ARMADILLO_LICENSE_FILES = LICENSE.txt
>  
>  ARMADILLO_CONF_OPTS = -DDETECT_HDF5=false
>  
> +# blas support may be provided by lapack, clapack or openblas
> +# blas library from (c)lapack is libblas.a, libopenblas.a otherwise
> +ARMADILLO_CONF_OPTS += -DBLAS_FOUND=ON
> +ifeq ($(BR2_PACKAGE_ARMADILLO_OPENBLAS),y)

[...] so I guess you meant BR2_PACKAGE_ARMADILLO_BLAS_OPENBLAS here, no?

> +ARMADILLO_CONF_OPTS += -DBLAS_LIBRARIES=-lopenblas
> +ARMADILLO_DEPENDENCIES = openblas
> +else
> +ARMADILLO_CONF_OPTS += -DBLAS_LIBRARIES=-lblas
> +ifeq ($(BR2_PACKAGE_CLAPACK), y)

We don't usually add a space after the comma in an ifeq, especially in
cases like this simple test.

Why duplicate the dependency on lapack/clapack in the !openblas case,
when the same dependencies already exist, below?

I mean: in the !openblas case, we know that either lapack or clapack are
enabled, so we know will hit either case in the block [...]

> +ARMADILLO_DEPENDENCIES = clapack
> +else
> +ARMADILLO_DEPENDENCIES = lapack
> +endif
> +endif

[...] here:

> +# lapack support may be provided by lapack or clapack
> +# but not by openblas
> +ifeq ($(BR2_PACKAGE_CLAPACK),y)
> +ARMADILLO_CONF_OPTS += -DLAPACK_FOUND=ON
> +ARMADILLO_DEPENDENCIES += clapack
> +else ifeq ($(BR2_PACKAGE_LAPACK),y)
> +ARMADILLO_CONF_OPTS += -DLAPACK_FOUND=ON
> +ARMADILLO_DEPENDENCIES += lapack
> +else
> +ARMADILLO_CONF_OPTS += -DLAPACK_FOUND=OFF
> +endif

Additionally, this block will be hit even in the openblas case. Is this
expected?

Regards,
Yann E. MORIN.

>  $(eval $(cmake-package))
> -- 
> 2.31.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot