Bug 231371
| Summary: | math/openblas: Update to 0.3.7 | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Yuri Victorovich <yuri> | ||||||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | Steve Wills <swills> | ||||||||||||||||||||||||
| Status: | Closed FIXED | ||||||||||||||||||||||||||
| Severity: | Affects Only Me | CC: | iblis.dif01, luzpaz, phd_kimberlite, pkubaj, salvadore, swills | ||||||||||||||||||||||||
| Priority: | --- | Flags: | phd_kimberlite:
maintainer-feedback+
|
||||||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||||||
| URL: | https://github.com/xianyi/OpenBLAS/issues/1763 | ||||||||||||||||||||||||||
| Bug Depends on: | 240937 | ||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
Created attachment 197103 [details]
patch
Thank you for approving! 0.3.5 is current stable good to go? (In reply to Iblis Lin from comment #4) I'll look at this this week, thanks. (In reply to Yuri Victorovich from comment #5) Ping? Q2 is almost upon us. The build fails now: https://github.com/xianyi/OpenBLAS/issues/2074 (In reply to Yuri Victorovich from comment #7) Now the above error is gone, but it fails to build in poudriere with the DYNAMIC_ARCH option. I am investigating. (In reply to Yuri Victorovich from comment #8) Pending: https://github.com/xianyi/OpenBLAS/issues/2076 Looks like it was merged in https://github.com/xianyi/OpenBLAS/pull/2080 Created attachment 205323 [details]
patch
I got this patch updated to 0.3.6 and working on powerpc64. Patches are currently waiting for upstream acceptance.
It still needs pkg-plist to be updated.
Sorry, I meant https://github.com/xianyi/OpenBLAS/pull/2169 Created attachment 205333 [details]
patch
Tested in Poudriere on 12.0-RELEASE/amd64 and CURRENT/powerpc64.
I optimize for POWER7 on powerpc64, because optimizing for PPC970 causes errors, and POWER7 is the last generation that is supported on big-endian systems (newer POWER CPU's are supported by openblas only in little-endian variant).
Please test.
Created attachment 205334 [details]
patch
Just noticed that files/patch-Makefile.system is not necessary anymore.
Created attachment 205335 [details]
patch
Also, ia64 is not supported anymore on FreeBSD.
Created attachment 205347 [details]
patch
Also update Mk/Uses/blaslapack.mk and rename library to libopenblas.so.0.
Any progress? Sorry, I wouldn't have time to look at this. Created attachment 208485 [details] updated patch for 0.3.7 Here's an updated patch that accounts for the changes committed in ports r511652 and also updates to the latest version, 0.3.7. (In reply to Steve Wills from comment #20) Note that this: ${REINPLACE_CMD} \ -e 's/defined(linux)/(defined(linux) || defined(__FreeBSD__))/g' \ -e 's/ifdef linux/if defined(linux) || defined(__FreeBSD__)/g' \ ${WRKSRC}/kernel/power/*.S shouldn't be needed with 0.3.7. I upstreamed those changes. Created attachment 208700 [details] updated patch Here's an updated version that takes into account the changes in ports r515970 and ports r515977 See also https://reviews.freebsd.org/D17333 Created attachment 208701 [details]
patch
I have the following questions:
1) is single threaded library really unnecessary?
2) is cmake build stable?
I suggest to consider updating to 0.3.7 and cmake migration separately.
The attached is just a suggestion.
(In reply to Eijiro Shibusawa from comment #24) There were a number of changes in the previous patches that I carried forward, such as using pkgconfig and fixes for POWER that you didn't comment on. Perhaps we should list everything to be done on this port and discuss separately rather than looking at individual patches. So: 1. Update to 0.3.7 2. Improve formatting 3. Add USES=pkgconfig 4. Optimize for POWER7 (TARGET=POWER7) 5. Switch to cmake 6. Introduce interface64 flavor Am I missing anything? Do you have objections to any of these? Your patch seems to only address the update to 0.3.7. I don't understand your question about the single threaded library. The cmake build should be stable, I don't know any reason to think it wouldn't be. My previous patch didn't address the cmake or flavor issue, I was just updating what was previously in this PR, haven't gotten to looking at the review and addressing those yet. (In reply to Steve Wills from comment #25) Our powerpc64 users are mainly on Macs, so I would still optimize for PPC970 by default. POWER7 option is for those who have newer hardware. I would like to introduce also options for POWER8 and POWER9, but it seems like optimizing for those works only in little-endian mode. Sorry, I misunderstood that the library is multi threaded. I approve your patch. Before you commit this, please remove the reference to "libopenblasp" from other ports and achieve consensus among users about POWER architecture. (In reply to Steve Wills from comment #25) I think you're mistaken in setting TARGET. You added: 56 MAKE_ENV+= ${MAKE_ENV_${ARCH}} 57 MAKE_ENV_powerpc64= TARGET=POWER7 But this is what TARGET option is for. We optimize by default for PPC970 and there's an option to optimize for POWER6. In OpenBLAS, POWER7 is an alias for POWER6, so it's literally the same. The lines above are absolutely necessary and actually harm. s/necessary/unnecessary/ It also looks like appending TARGET=${TARGET_CPU_ARCH} to BUILDFLAGS has no meaning with this version. Can you set TARGET like this when you commit this patch?
.if defined(TARGET_CPU_ARCH)
BUILDFLAGS+= TARGET=${TARGET_CPU_ARCH}
+MAKE_ENV+= TARGET=${TARGET_CPU_ARCH}
.endif
Optimizing for POWER8 on 0.3.7 works just fine (it still fails for POWER9). Since users nowadays generally use PPC970, POWER8 (Tyan) or POWER9 (Raptor), I think it's ok to replace POWER6 option with POWER8. Basically s/POWER6/POWER8/. I was in talks with OpenBLAS devs and while OpenBLAS builds for POWER8 on BE, it's only on ELFv2 and after additional testing, I found that it has endianness bugs, making it unsuitable for work. So we're back to default PPC970 and option for POWER6 in 0.3.7. Created attachment 210895 [details]
proposed patch
Minor change here, I'm going to build test all the things that depend on openblas and see if this can be committed. I already have a change to Mk/Uses/blaslapack.mk and math/py-numpy/Makefile to replace references to openblasp with openblas. Perhaps the build test will show more things to fix.
A commit references this bug: Author: swills Date: Tue Jan 21 22:03:23 UTC 2020 New revision: 523749 URL: https://svnweb.freebsd.org/changeset/ports/523749 Log: math/openblas: Update to 0.3.7 While here, improve formatting and fix build of dependant ports. PR: 231371 Submitted by: yuri (initial version), pkubaj (ppc fixes) Approved by: Eijiro Shibusawa <phd_kimberlite@yahoo.co.jp> (maintainer) Changes: head/Mk/Uses/blaslapack.mk head/biology/gemma/Makefile head/games/leela-zero/Makefile head/lang/julia06/Makefile head/lang/julia07/Makefile head/lang/julia10/Makefile head/lang/julia11/Makefile head/math/armadillo/Makefile head/math/hmat-oss/Makefile head/math/octave-forge-ltfat/Makefile head/math/openblas/Makefile head/math/openblas/distinfo head/math/openblas/files/patch-Makefile head/math/openblas/files/patch-Makefile.rule head/math/openblas/files/patch-Makefile.system head/math/openblas/files/patch-c_check head/math/openblas/files/patch-common__arm.h head/math/openblas/files/patch-common__power.h head/math/openblas/files/patch-cpuid__arm64.c head/math/openblas/files/patch-cpuid_ia64.c head/math/openblas/files/patch-cpuid_sparc.c head/math/openblas/files/patch-cpuid_x86.c head/math/openblas/files/patch-driver_others_memory.c head/math/openblas/files/patch-exports+Makefile head/math/openblas/files/patch-f_check head/math/openblas/files/patch-interface__ztrmv.c head/math/openblas/files/patch-param.h head/math/openblas/pkg-plist head/math/py-numpy/Makefile head/misc/mxnet/Makefile head/science/erkale/Makefile head/science/libcint/files/patch-CMakeLists.txt Committed, thanks! A commit references this bug: Author: jbeich Date: Wed Jan 22 12:52:56 UTC 2020 New revision: 523805 URL: https://svnweb.freebsd.org/changeset/ports/523805 Log: graphics/caffe: drop math/cblas dependency after r523749 PR: 231371 240937 Changes: head/graphics/caffe/Makefile A commit references this bug: Author: jbeich Date: Wed Jan 22 13:01:32 UTC 2020 New revision: 523806 URL: https://svnweb.freebsd.org/changeset/ports/523806 Log: misc/libpostal: unbreak CBLAS=on after r523749 =======================<phase: lib-depends >============================ ===> libpostal-1.1_1 depends on shared library: libopenblas.so - not found ===> Installing existing package /packages/All/openblas-0.3.7,1.txz Installing openblas-0.3.7,1... pkg-static: openblas-0.3.7,1 conflicts with cblas-1.0_12 (installs files into the same place). Problematic file: /usr/local/include/cblas.h Failed to install the following 1 package(s): /packages/All/openblas-0.3.7,1.txz *** Error code 70 PR: 231371 Changes: head/misc/libpostal/Makefile A commit references this bug: Author: jbeich Date: Wed Jan 22 16:58:38 UTC 2020 New revision: 523819 URL: https://svnweb.freebsd.org/changeset/ports/523819 Log: math/hmat-oss: disable BLAS multi-threading after r523749 Previously, openblas_set_num_threads wasn't available but libopenblas.so was single-threaded. PR: 231371 Changes: head/math/hmat-oss/Makefile A commit references this bug: Author: jbeich Date: Wed Jan 22 16:58:45 UTC 2020 New revision: 523820 URL: https://svnweb.freebsd.org/changeset/ports/523820 Log: games/leela-zero: disable BLAS multi-threading after r523749 Previously, openblas_set_num_threads wasn't available but libopenblas.so was single-threaded. PR: 231371 Changes: head/games/leela-zero/Makefile head/games/leela-zero/files/patch-src_Network.cpp A commit references this bug: Author: swills Date: Thu Jan 30 14:18:01 UTC 2020 New revision: 524642 URL: https://svnweb.freebsd.org/changeset/ports/524642 Log: Fix math/openblas and bump dependent ports * Fix build on i386 [1] * Fix science/code_saturne build with new openblas [2] * Avoid installing private headers [3] * Prevent build from optimizing for host by correcting build confg [4] * Bump portrevision of dependent ports [5] This is correcting issues from r523749 [1][2][4] and r515970 [3] PR: 231371 Reported by: build cluster [1] Reported by: Dima Pasechnik <dimpase+freebsd@gmail.com> [2] Reported by: many [5] Reviewed by: mat, bapt Approved by: implicit, since this is a build fix Changes: head/biology/bolt-lmm/Makefile head/biology/gemma/Makefile head/biology/treekin/Makefile head/cad/calculix-ccx/Makefile head/cad/gmsh/Makefile head/cad/pdnmesh/Makefile head/databases/grass7/Makefile head/devel/ocaml-lacaml/Makefile head/games/leela-zero/Makefile head/graphics/caffe/Makefile head/graphics/gimp-refocus-plugin/Makefile head/lang/julia06/Makefile head/lang/julia07/Makefile head/lang/julia10/Makefile head/lang/julia11/Makefile head/mail/rspamd/Makefile head/mail/rspamd-devel/Makefile head/math/R/Makefile head/math/adept/Makefile head/math/ambit/Makefile head/math/armadillo/Makefile head/math/blaze/Makefile head/math/blazeiterative/Makefile head/math/blocksolve95/Makefile head/math/bonmin/Makefile head/math/casadi/Makefile head/math/ceres-solver/Makefile head/math/clp/Makefile head/math/cminpack/Makefile head/math/coinmp/Makefile head/math/coinutils/Makefile head/math/couenne/Makefile head/math/dbcsr/Makefile head/math/deal.ii/Makefile head/math/elpa/Makefile head/math/g2o/Makefile head/math/giacxcas/Makefile head/math/hmat-oss/Makefile head/math/ipopt/Makefile head/math/jags/Makefile head/math/lapack++/Makefile head/math/lapack95/Makefile head/math/levmar/Makefile head/math/moab/Makefile head/math/octave/Makefile head/math/octave-forge-ltfat/Makefile head/math/openblas/Makefile head/math/openblas/pkg-plist head/math/openturns/Makefile head/math/osi/Makefile head/math/primme/Makefile head/math/py-numpy/Makefile head/math/py-pysparse/Makefile head/math/py-scs/Makefile head/math/qposases/Makefile head/math/scilab/Makefile head/math/scs/Makefile head/math/sdpa/Makefile head/math/suitesparse/Makefile head/math/sundials/Makefile head/math/superlu/Makefile head/math/taucs/Makefile head/math/tmv/Makefile head/misc/mxnet/Makefile head/science/PETSc/Makefile head/science/berkeleygw/Makefile head/science/chemps2/Makefile head/science/code_saturne/Makefile head/science/dalton/Makefile head/science/dlib-cpp/Makefile head/science/elmerfem/Makefile head/science/erkale/Makefile head/science/fleur/Makefile head/science/gamess-us/Makefile head/science/gromacs/Makefile head/science/harminv/Makefile head/science/hypre/Makefile head/science/iboview/Makefile head/science/lammps/Makefile head/science/latte/Makefile head/science/libcint/Makefile head/science/mbdyn/Makefile head/science/mpqc/Makefile head/science/multiwfn/Makefile head/science/opensim-core/Makefile head/science/py-dlib/Makefile head/science/py-gpaw/Makefile head/science/py-phono3py/Makefile head/science/qmcpack/Makefile head/science/simbody/Makefile A commit references this bug: Author: swills Date: Fri Jan 31 14:57:32 UTC 2020 New revision: 524713 URL: https://svnweb.freebsd.org/changeset/ports/524713 Log: math/openblas: fix plist with DYNAMIC_ARCH off This is further fallout from r523749 and r524642 While here, fix order of PORTREVISION. There is no PORTREVISION bump because this a build fix. PR: 231371 PR: 243739 Reported by: many Changes: head/math/openblas/Makefile head/math/openblas/pkg-plist |
Created attachment 197101 [details] patch