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.

Sign up for GitHub

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

Jump to bottom

Apply cpu-optimisation to Rust projects #15544

Merged
merged 17 commits into from
Jul 4, 2023
Merged

Apply cpu-optimisation to Rust projects #15544

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
c344a3f
Apply cpu-optimisation to Rust projects (hardcoded)
Tokarak Jun 13, 2023
a072508
Revert "Apply cpu-optimisation to Rust projects (hardcoded)"
Tokarak Jun 13, 2023
066a888
Add rustflags_target_cpu method
Tokarak Jun 13, 2023
b4dd639
Use rustflags_target_cpu
Tokarak Jun 13, 2023
d4bfbb1
Remove FIXMEs
Tokarak Jun 13, 2023
2bd20ea
Fix
Tokarak Jun 13, 2023
c698174
Fix error on empty target-cpu
Tokarak Jun 13, 2023
cccad3e
Line break
Tokarak Jun 14, 2023
261f55a
Prepare to migrate to RUSTFLAGS
Tokarak Jun 14, 2023
2133da8
Set target-cpu through RUSTFLAGS
Tokarak Jun 14, 2023
9a99b93
Add RUSTFLAGS to ENV/super
Tokarak Jun 17, 2023
9a19f5b
Refactor to use oldest_cpu
Tokarak Jun 17, 2023
73c339d
Style changes
Tokarak Jun 17, 2023
40aa419
More cpus in rust_optimisation_flags
Tokarak Jun 17, 2023
87158ac
Improve logic (code review)
Tokarak Jun 20, 2023
55bdbab
brew style --fix
Tokarak Jun 20, 2023
521fdcb
Wording
Tokarak Jun 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Library/Homebrew/extend/ENV/shared.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module SharedEnvExtension
CMAKE_PREFIX_PATH CMAKE_INCLUDE_PATH CMAKE_FRAMEWORK_PATH
GOBIN GOPATH GOROOT PERL_MB_OPT PERL_MM_OPT
LIBRARY_PATH LD_LIBRARY_PATH LD_PRELOAD LD_RUN_PATH
RUSTFLAGS
].freeze
private_constant :SANITIZED_VARS

Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/extend/ENV/std.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_a
self["PKG_CONFIG_LIBDIR"] = determine_pkg_config_libdir

self["MAKEFLAGS"] = "-j#{make_jobs}"
self["RUSTFLAGS"] = Hardware.rustflags_target_cpu

if HOMEBREW_PREFIX.to_s != "/usr/local"
# /usr/local is already an -isystem and -L directory so we skip it
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/extend/ENV/super.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_a

self["HOMEBREW_ENV"] = "super"
self["MAKEFLAGS"] ||= "-j#{determine_make_jobs}"
self["RUSTFLAGS"] = Hardware.rustflags_target_cpu
self["PATH"] = determine_path
self["PKG_CONFIG_PATH"] = determine_pkg_config_path
self["PKG_CONFIG_LIBDIR"] = determine_pkg_config_libdir
Expand Down
17 changes: 17 additions & 0 deletions Library/Homebrew/hardware.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ def optimization_flags
end
alias generic_optimization_flags optimization_flags

# Only give values where it is an improvement over rust cpu defaults
# Rust already defaults to the oldest supported cpu for each target-triple
# Including apple-m1 since Rust 1.71 for aarch64-apple-darwin.
Copy link
Member

Choose a reason for hiding this comment

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

I'd really rather be explicit here rather than bake in assumptions about the behaviour of rustc. None of us track the development of rustc that carefully, so if they decide to bump the minimum of core2 before we do, then this method will no longer behave as expected, and that's likely to go unnoticed for some time.

The reverse (i.e. if we decide to bump the minimum of core2 before Rust does) is also an issue, albeit a smaller one.

def rust_optimisation_flags
@rust_optimisation_flags ||= {
native: "-Ctarget-cpu=native",
nehalem: "-Ctarget-cpu=nehalem",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
native: "-Ctarget-cpu=native",
nehalem: "-Ctarget-cpu=nehalem",
native: "--codegen target-cpu=native",
nehalem: "--codegen target-cpu=nehalem",

Please use the long flags for better readability.

}.freeze
end

sig { returns(Symbol) }
def arch_32_bit
if arm?
Expand Down Expand Up @@ -214,6 +224,13 @@ def oldest_cpu(_version = nil)
end
end
alias generic_oldest_cpu oldest_cpu

# Returns a _full_ rustflag to set target cpu, if necessary;
# Defaults to empty string
sig { returns(String) }
def rustflags_target_cpu
CPU.rust_optimisation_flags.fetch(oldest_cpu, "")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sig { returns(String) }
def rustflags_target_cpu
CPU.rust_optimisation_flags.fetch(oldest_cpu, "")
sig { returns(T.nilable(String)) }
def rustflags_target_cpu
CPU.rust_optimisation_flags.fetch(oldest_cpu, nil)

Also: we should try to support users doing something like

brew install --build-bottle --bottle-arch=ivybridge

which this doesn't currently do.

end
end
end

Expand Down