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

Math packages: givaro, fflas-ffpack, linbox #29997

Merged
merged 3 commits into from
Oct 30, 2021
Merged

Conversation

tornaria
Copy link
Contributor

@tornaria tornaria commented Apr 4, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me

Notes

  • This includes: givaro, fflas-ffpack, linbox.
  • Everything is tested and check pass in x86_64, x86_64-musl, i686.
  • Except gf2x, everything is nocross.
  • I tried to make sure everything works with baseline x86_64 as defined here, essentially it means SSE2 is ok, but SSE3 is not.
  • I added a general option native, which defaults to off. When turned on, it will let the libraries compile for the build machine which often results in significant improvements.

EDIT: split in smaller independent PR to ease review

@dkwo
Copy link
Contributor

dkwo commented Apr 4, 2021

Great job, I will test soon.

@tornaria
Copy link
Contributor Author

tornaria commented Apr 4, 2021

Nothing relevant changed: I cleaned up some leftover lines in common/shlib.

@tornaria tornaria changed the title Several math libraries Math packages: givaro, fflas-ffpack, linbox Apr 6, 2021
@dkwo
Copy link
Contributor

dkwo commented Apr 7, 2021

Looks good to me.


if [ ! "$build_option_native" ]; then
configure_args="--enable-sse --enable-sse2
--disable-sse3 --disable-ssse3 --disable-sse41 --disable-sse42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather remove option and make in unconditional. People are free to use XBPS_CFLAGS="-march=native" to get optimized build. Works for this package, should work for others too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the correct variable is XBPS_TARGET_CFLAGS? At least that's what's used in the build-profiles (the README there needs to be fixed, btw).

I agree that that's a better way of solving it, but it would be nice to make sure it's working. Also see #29732

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean XBPS_CFLAGS in etc/conf

Copy link
Contributor Author

@tornaria tornaria Apr 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might work in this case (assuming the only effect of the --enable-sse* configure flags is to add -msse* to CFLAGS, I'd have to check).

However, some math packages have custom written asm for different architectures so selecting build option "native" will not necessarily be equivalent to just using -march=native.

Another point is that using a build option will make an annotation in the xbps, e.g.:

$ xq givaro | grep native
build-options: ~native 

vs.

$ xq givaro | grep native
build-options: native 

@dkwo
Copy link
Contributor

dkwo commented Aug 19, 2021

@tornaria @ericonr Btw, don't forget this one ;)

@tornaria
Copy link
Contributor Author

@ericonr: rebased with only style changes. Tested with sage-9.4.rc2.

@dkwo dkwo mentioned this pull request Aug 20, 2021
@tornaria tornaria mentioned this pull request Aug 23, 2021
@tornaria
Copy link
Contributor Author

Latest changes:

@tornaria
Copy link
Contributor Author

@ericonr: let's do math stuff one at a time. I propose to start with this one which is the older one I've got. I think I applied all the style changes you told me in #30032.

@dkwo
Copy link
Contributor

dkwo commented Oct 29, 2021

@ericonr Do you think we can merge some of the Math packages PR, starting from this perhaps? otherwise, rebasing them can be a bit tiring exercise.

@ericonr ericonr merged commit 578d045 into void-linux:master Oct 30, 2021
@tornaria tornaria deleted the math1 branch October 30, 2021 19:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants