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

Apply cpu-optimisation to Rust projects #15544

Merged
merged 17 commits into from
Jul 4, 2023
16 changes: 16 additions & 0 deletions Library/Homebrew/extend/os/mac/hardware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,20 @@ def self.oldest_cpu(version = nil)
generic_oldest_cpu
end
end

# Override
# Mirrors version-dependent logic of oldest_cpu
sig { params(version: T.nilable(Version)).returns(String) }
def self.rustflags_target_cpu(version = nil)
Copy link
Member

Choose a reason for hiding this comment

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

When would version be passed through here rather than just use the current MacOS.version?

version = if version
MacOSVersion.new(version.to_s)
else
MacOS.version
end
if Hardware::CPU.intel? && version >= :mojave
"'-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
"'-Ctarget-cpu=nehalem'"
"--codegen target-cpu=nehalem"
  1. Let callers handle the quoting, if it's needed.
  2. Use the long version of the flag.
  3. I'd still rather use oldest_cpu here instead of hard-coding nehalem.

else
generic_rustflags_target_cpu
end
end
end
3 changes: 2 additions & 1 deletion Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1543,7 +1543,8 @@ def std_configure_args
# Standard parameters for cargo builds.
sig { params(root: T.any(String, Pathname), path: String).returns(T::Array[T.any(String, Pathname)]) }
def std_cargo_args(root: prefix, path: ".")
["--locked", "--root", root, "--path", path]
["--locked", "--root", root, "--path", path, "--config",
"build.rustflags=[#{Hardware.rustflags_target_cpu}]"]
Copy link
Member

Choose a reason for hiding this comment

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

How does this interact with RUSTFLAGS set in the environment? Does it override them? Append to them?

I think we may prefer to set RUSTFLAGS in the environment as well as/instead of doing this, since not all rustc invocations happen through a cargo call that uses std_cargo_args. See, for example, gstreamer, or the many formulae that have a cryptography resource.

end

# Standard parameters for CMake builds.
Expand Down
12 changes: 12 additions & 0 deletions Library/Homebrew/hardware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,18 @@ def oldest_cpu(_version = nil)
end
end
alias generic_oldest_cpu oldest_cpu

# Returns a _full_ rustflag to set target cpu, if necessary;
# Falls back to empty string
# This mirrors the logic of `oldest_cpu`,
# But only where it is version dependent.
# Rust already defaults to the oldest supported cpu for the target-triple
# Including apple-m1 since 1.71.
sig { params(_version: T.nilable(Version)).returns(String) }
def rustflags_target_cpu(_version = nil)
""
end
alias generic_rustflags_target_cpu rustflags_target_cpu
end
end

Expand Down