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

cc-wrapper: -march= is not allowed on powerpc #205242

Merged
merged 1 commit into from Jan 10, 2023
Merged

cc-wrapper: -march= is not allowed on powerpc #205242

merged 1 commit into from Jan 10, 2023

Conversation

ghost
Copy link

@ghost ghost commented Dec 9, 2022

Description of changes

Gcc does not allow -march= on PowerPC:

https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options

Instead, -mcpu= should be used to set the minimum instruction set and -mtune= is used to optimize instruction scheduling for a specific processor. Both flags take the same set of valid values, which includes native.

This commit causes isGccArchSupported to return false for PowerPC targets so we never pass an -march= flag, since that will always be rejected by gcc.

Things done
  • Built on platform(s)
    • powerpc64le-linux (full workstation rebuild)
  • Fits CONTRIBUTING.md.

Gcc does not allow `-march=` on PowerPC:

  https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options

Instead, `-mcpu=` should be used to set the minimum instruction set
and `-mtune=` is used to optimize instruction scheduling for a
specific processor.  Both flags take the same set of valid values,
which includes `native`.

This commit causes `isGccArchSupported` to return `false` for PowerPC
targets so we never pass an `-march=` flag, since that will always be
rejected by gcc.
@ghost ghost requested a review from Ericson2314 as a code owner December 9, 2022 04:08
Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

From what I could find, the premise is true and the diff lgtm.

@Atemu Atemu requested a review from a team December 9, 2022 08:55
@SuperSandro2000
Copy link
Member

@ofborg eval

@Atemu Atemu added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Dec 9, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Dec 9, 2022
@sternenseemann sternenseemann merged commit dfa3f14 into NixOS:master Jan 10, 2023
@ghost ghost deleted the ccwrapper-powerpc-march branch January 10, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants