Bug 231371 - math/openblas: Update to 0.3.7
Summary: math/openblas: Update to 0.3.7
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Piotr Kubaj
URL: https://github.com/xianyi/OpenBLAS/is...
Keywords:
Depends on: 240937
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-15 04:24 UTC by Yuri Victorovich
Modified: 2019-12-10 09:04 UTC (History)
6 users (show)

See Also:


Attachments
patch (14.94 KB, patch)
2018-09-15 04:24 UTC, Yuri Victorovich
no flags Details | Diff
patch (15.17 KB, patch)
2018-09-15 08:37 UTC, Yuri Victorovich
no flags Details | Diff
patch (17.71 KB, patch)
2019-06-25 08:42 UTC, Piotr Kubaj
no flags Details | Diff
patch (18.62 KB, patch)
2019-06-25 16:23 UTC, Piotr Kubaj
no flags Details | Diff
patch (18.67 KB, patch)
2019-06-25 16:29 UTC, Piotr Kubaj
no flags Details | Diff
patch (19.43 KB, patch)
2019-06-25 16:31 UTC, Piotr Kubaj
no flags Details | Diff
patch (20.76 KB, patch)
2019-06-26 08:57 UTC, Piotr Kubaj
no flags Details | Diff
updated patch for 0.3.7 (17.20 KB, patch)
2019-10-21 15:43 UTC, Steve Wills
no flags Details | Diff
updated patch (19.93 KB, patch)
2019-10-30 14:01 UTC, Steve Wills
swills: maintainer-approval? (phd_kimberlite)
Details | Diff
patch (14.85 KB, patch)
2019-10-30 14:34 UTC, Eijiro Shibusawa
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer 2018-09-15 04:24:17 UTC
Created attachment 197101 [details]
patch
Comment 1 Yuri Victorovich freebsd_committer 2018-09-15 08:37:33 UTC
Created attachment 197103 [details]
patch
Comment 2 Yuri Victorovich freebsd_committer 2018-09-15 16:18:57 UTC
Thank you for approving!
Comment 3 kunda 2019-01-10 06:31:33 UTC
0.3.5 is current stable
Comment 4 Iblis Lin 2019-02-06 07:01:22 UTC
good to go?
Comment 5 Yuri Victorovich freebsd_committer 2019-02-06 07:26:21 UTC
(In reply to Iblis Lin from comment #4)

I'll look at this this week, thanks.
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2019-03-29 17:52:42 UTC
(In reply to Yuri Victorovich from comment #5)

Ping?  Q2 is almost upon us.
Comment 7 Yuri Victorovich freebsd_committer 2019-03-31 16:54:45 UTC
The build fails now: https://github.com/xianyi/OpenBLAS/issues/2074
Comment 8 Yuri Victorovich freebsd_committer 2019-03-31 21:38:05 UTC
(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.
Comment 9 Yuri Victorovich freebsd_committer 2019-03-31 22:34:58 UTC
(In reply to Yuri Victorovich from comment #8)

Pending: https://github.com/xianyi/OpenBLAS/issues/2076
Comment 10 kunda 2019-05-16 20:05:23 UTC
Looks like it was merged in https://github.com/xianyi/OpenBLAS/pull/2080
Comment 11 Piotr Kubaj freebsd_committer 2019-06-25 08:42:31 UTC
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.
Comment 12 Piotr Kubaj freebsd_committer 2019-06-25 09:02:22 UTC
https://github.com/xianyi/OpenBLAS/issues/1895
Comment 13 Piotr Kubaj freebsd_committer 2019-06-25 09:02:38 UTC
Sorry, I meant https://github.com/xianyi/OpenBLAS/pull/2169
Comment 14 Piotr Kubaj freebsd_committer 2019-06-25 16:23:52 UTC
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.
Comment 15 Piotr Kubaj freebsd_committer 2019-06-25 16:29:43 UTC
Created attachment 205334 [details]
patch

Just noticed that files/patch-Makefile.system is not necessary anymore.
Comment 16 Piotr Kubaj freebsd_committer 2019-06-25 16:31:14 UTC
Created attachment 205335 [details]
patch

Also, ia64 is not supported anymore on FreeBSD.
Comment 17 Piotr Kubaj freebsd_committer 2019-06-26 08:57:28 UTC
Created attachment 205347 [details]
patch

Also update Mk/Uses/blaslapack.mk and rename library to libopenblas.so.0.
Comment 18 kunda 2019-07-23 01:46:07 UTC
Any progress?
Comment 19 Yuri Victorovich freebsd_committer 2019-07-23 01:47:59 UTC
Sorry, I wouldn't have time to look at this.
Comment 20 Steve Wills freebsd_committer 2019-10-21 15:43:54 UTC
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.
Comment 21 Piotr Kubaj freebsd_committer 2019-10-28 19:41:50 UTC
(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.
Comment 22 Steve Wills freebsd_committer 2019-10-30 14:01:21 UTC
Created attachment 208700 [details]
updated patch

Here's an updated version that takes into account the changes in ports r515970 and ports r515977
Comment 23 Steve Wills freebsd_committer 2019-10-30 14:02:47 UTC
See also https://reviews.freebsd.org/D17333
Comment 24 Eijiro Shibusawa 2019-10-30 14:34:15 UTC
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.
Comment 25 Steve Wills freebsd_committer 2019-10-30 17:09:59 UTC
(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.
Comment 26 Piotr Kubaj freebsd_committer 2019-10-30 17:15:53 UTC
(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.
Comment 27 Eijiro Shibusawa 2019-10-30 21:13:54 UTC
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.
Comment 28 Piotr Kubaj freebsd_committer 2019-10-30 22:47:17 UTC
(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.
Comment 29 Piotr Kubaj freebsd_committer 2019-10-30 22:49:33 UTC
s/necessary/unnecessary/
Comment 30 Piotr Kubaj freebsd_committer 2019-10-30 23:05:52 UTC
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
Comment 31 Piotr Kubaj freebsd_committer 2019-10-31 13:21:04 UTC
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/.
Comment 32 Piotr Kubaj freebsd_committer 2019-11-05 06:39:41 UTC
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.