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

shared: only fall back to -march=native on supported architectures #18759

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

alebcay
Copy link
Member

@alebcay alebcay commented Nov 12, 2024

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

On Linux, -march=native is added in the optflags when building from source (and not for a bottle). This works fine for the case where users install brew in a custom prefix on a supported architecture, but breaks the case where the user is building from source for running under Linux on an unsupported architecture for which the -march=native flag is not valid (I specifically ran into this issue while tinkering with brew under Ubuntu on RISC-V).

Introduce a new optimization flag option none and adjust effective_arch to only fall back on :native if running on Intel or ARM (which are not the only architectures which support -march=native, but it does cover all existing supported use cases and leaves room for current development/experimentation happening with brew on Linux arm64), and falls back on the more conservative :none option if not running one of those architectures.

@MikeMcQuaid
Copy link
Member

On Linux, -march=native is added in the optflags when building from source (and not for a bottle).

Just putting it out there: everyone who felt very strongly about this being the case has left the project. Other maintainers may still feel strongly about this but: I think it's worth reopening the discussion about removing this behaviour. Reasoning:

  • breakage like this
  • it's inconsistent with macOS (that doesn't use -march=native like this)
  • it makes source builds inconsistent with bottles which harms debugging

Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

  • breakage like this

This kind of breakage (using brew on RISC-V) seems exotic enough to not be worth making decisions that will affect many other users. If there are other types of more common breakage, we can discuss those, but I don't want to base it on this one.

  • it's inconsistent with macOS (that doesn't use -march=native like this)

macOS and Linux have different baselines -- on macOS, we do -march=westmere or -march=nehalem, which are reasonable. The baseline for Linux is -march=core2, which is pretty bad.

  • it makes source builds inconsistent with bottles which harms debugging

I don't think this has really caused us any significant difficulty in the past few years.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @alebcay!

@MikeMcQuaid MikeMcQuaid merged commit 5a2c264 into Homebrew:master Nov 13, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants