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

Rename configure option --enable-fat-binary to --enable-portable-binary #32363

Open
culler opened this issue Aug 10, 2021 · 6 comments
Open

Rename configure option --enable-fat-binary to --enable-portable-binary #32363

culler opened this issue Aug 10, 2021 · 6 comments

Comments

@culler
Copy link
Contributor

culler commented Aug 10, 2021

As far as I can tell this configuration flag affects only the gmp and givaro packages. There are two problems that I have run into with it.

  1. It is misnamed. What Apple means by a "fat binary" is a single mach-O file containing multiple binaries for different CPU architectures. That is not what this flag produces. The name was probably derived from the gmp configure flag --enable-fat, which is used when SAGE_FAT_BINARY is yes. That gmp flag causes gmp to include assembly language modules for all possible Intel variants, and to add code that selects which of those modules to use at run time.

  2. It does the wrong thing for givaro. Currently in the givaro package it disables all intel extensions (the opposite of making it "fat") by setting the configure flags: -mno-sse, -mno-sse2, -mno-avx, -mno-avx2, and -mno-bmi2. But sse and sse2 should not be disabled, at least not on macOS. All of the relevent Intel chips used by Apple support those instruction sets. (And conda does not disable them, by the way.) Sage will not build on Apple systems with older Intel chips for other reasons.

A better name might be ENABLE_INTEL_COMPATIBILITY.

CC: @kliem @jhpalmieri

Component: build: configure

Issue created by migration from https://trac.sagemath.org/ticket/32363

@culler culler added this to the sage-9.4 milestone Aug 10, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2021

comment:1

Yes, it's misnamed.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2021

comment:2

As (I think) this is not specific to intel architectures, we could rename the configure option --enable-fat-binary (introduced in #30375) to something like --enable-portable-binary.

numpy >= 1.20 has a more fine-grained flag for its use of processor intrinsics, something that we may want to emulate; see #32021

@culler
Copy link
Contributor Author

culler commented Aug 11, 2021

comment:3

Possibly the sense should also be inverted, meaning that maximum portability would be the default while optimizing for a specific CPU type would require additional configuration flags. Perhaps a flag like --optimize-for-native-cpu would make sense for people who are building sage themselves for their personal use.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 11, 2021

comment:4

Replying to @culler:

Possibly the sense should also be inverted, meaning that maximum portability would be the default while optimizing for a specific CPU type would require additional configuration flags.

-1 on this. Building a portable binary is clearly a more specialized use case than building an optimized binary.

@kliem
Copy link
Contributor

kliem commented Aug 11, 2021

comment:5

-1

Non-portable build appears to be the standard use case, when compiling something yourself from source.

@kliem
Copy link
Contributor

kliem commented Aug 11, 2021

comment:6

I agree that the name --enable-fat-binary is confusing and incorrect.

@mkoeppe mkoeppe changed the title issues with SAGE_FAT_BINARY Rename configure option --enable-fat-binary to --enable-portable-binary Aug 11, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 11, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants