-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
openblas: add initial package #15685
Conversation
@bhack will try to add support for this in numpy |
looks like i have a few things to iron out on this; |
It should not link against 300kB GOMP, at least not on compact embedded architectures. |
POWER4 is a 64bit CPU, probably that needs to be omitted, 32bit PPC (Apple G4) is long unmaintained, it might need some tuning to accommodate configuration with only generic C. That shall also help support other less mainstream arches. |
AFAICT, your PPC builds currently fail as they are missing the OpenMP headers, and the x86_64 build struggles to find a matching SGEMM kernel as the "TARGET=ATHLON" you gave it is strictly 32bit (suggest using TARGET=OPTERON or CORE2 if you want a tiny bit more performance than the safest, TARGET=GENERIC while avoiding the size penalty of DYNAMIC_ARCH) |
wow, that's a lot of useful feedback :) so, it may take me a bit to re-spin things; obviously, this will get tweaked even after being merged; if anyone feels like they want to be a co-maintainer on this, i am happy to add/list them; |
You can also build dynamic lib on x86_64 with few cores present in hardware + generic. Further down the road, though. |
0b76a2c
to
7685dcf
Compare
Changelog v1 -> v2:
|
so, we may tweak things by using the this will be future-work; |
i may have missed what this comment; |
ARM targets: OpenMP:
It is all written in |
You need to tweak COMMON_OPT="" and maybe CCOMMON_OPT=$YOUR_CFLAGS to avoid overrriding intended |
many thanks for the
in general in OpenWrt packaging we don't run tests; usually these tests are more in the responsibility of the package (itself)
Should I remove these? |
I'm seeing that
|
Changelog v2 -> v3:
|
Thread errors are because of strange threading flags , remove that USE_OPENMP=0 and add NUM_THREADS=2, seems that CI build happens on various machines and messages pop on or off depending on that. |
3473afd
to
76af978
Compare
I still get the same build errors after removing USE_OPENMP=0 and adding NUM_THREADS=2. And adding So, I removed Changelog v3 -> v4:
|
anything from my part to do here? |
I suppose it is usable as numpy backend on supplied platforms. And reasonable minimised representation of OpenBLAS. Youre always free to ping upstream if you find an inconsistency, or plainly get stuck and confused. |
I ran this code on a numpy instalation:
Output:
|
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
created PR #16054 for numpy with OpenBLAS right now, i just want to slowly move forward; |
libs/openblas/Makefile
Outdated
|
||
PKG_NAME:=OpenBLAS | ||
PKG_VERSION:=0.3.15 | ||
PKG_RELEASE:=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use $(AUTORELEASE) here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack;
libs/openblas/Makefile
Outdated
based on GotoBLAS2 1.13 BSD version. | ||
endef | ||
|
||
ifneq (,$(findstring $(ARCH) , aarch64 mips64 mips64el x86_64 )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be shortened to one line with CONFIG_ARCH_64BIT
libs/openblas/Makefile
Outdated
endif | ||
endif # ifeq ($(OPENBLAS_TARGET),) | ||
|
||
ifneq (,$(findstring $(ARCH) , aarch64 mips64 mips64el x86_64 )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a duplicate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep;
sorry about that; will remove;
|
||
MAKE_FLAGS += \ | ||
CROSS=1 \ | ||
HOSTCC=$(HOSTCC) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is suspicious :). I bet it breaks with ccache on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm; no idea about ccache here;
Initial draft PR is: openwrt#11894 This one is a bit more complete, and follows packaging practices. For now, disabling builds on ARC and PowerPC. Will require more work to get them going. Explicitly disabling OpenMP support, so that it doesn't get picked by accident. Later we may use the `CPU_TYPE` parameter to tweak things a little further. Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Changelog v4 -> v5:
|
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com> Signed-off-by: Tianling Shen <cnsztl@immortalwrt.org>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Also bump Cython version to 0.29.23. And add support for OpenBLAS. Currently optional, but will be enabled by default on some architectures later. Depends on PR openwrt/packages#15685 Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Maintainer: me
Compile tested: x86 openwrt/openwrt@ddcb970
Run tested: x86 openwrt/openwrt@ddcb970
Initial draft PR is:
#11894
This one is a bit more complete, and follows packaging practices.
For now, disabling builds on ARC and PowerPC. Will require more work to get
them going.
Explicitly disabling OpenMP support, so that it doesn't get picked by
accident.
Later we may use the
CPU_TYPE
parameter to tweak things a little further.Signed-off-by: Alexandru Ardelean ardeleanalex@gmail.com