Skip to content

compiler: set Apple frame pointers by architecture #141797

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
Merged
merged 1 commit into from
Jun 7, 2025
Merged

compiler: set Apple frame pointers by architecture #141797

merged 1 commit into from
Jun 7, 2025
+23 −36

Conversation

workingjubilee
Copy link
Member

All Apple targets stop overriding this configuration and instead use the default base of FramePointer::NonLeaf, which means some Apples will have less frame pointers in leaf functions.

r? @madsmtm

cc @thomcc

@rustbot rustbot added O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 30, 2025

These commits modify compiler targets.
(See the Target Tier Policy.)

@workingjubilee
Copy link
Member Author

workingjubilee commented May 30, 2025 •
edited
Loading

This mostly affects x86_64-apple-darwin and related targets, which now use non-leaf frame pointers. This was implemented in #86652 despite non-leaf frame pointers becoming available then. It was not clear to me if there was a specific motivation for preserving x86-64's "full" frame-pointers or not.

An obvious alternative to this PR is to make the default frame-pointer setting something arch-dependent instead.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@workingjubilee workingjubilee force-pushed the apple-likes-frame-pointers-but-not-that-much branch from 6822314 to 57b04e7 Compare May 30, 2025 21:57
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@workingjubilee workingjubilee force-pushed the apple-likes-frame-pointers-but-not-that-much branch from 57b04e7 to cca230a Compare May 30, 2025 22:36
madsmtm
madsmtm reviewed Jun 5, 2025
View reviewed changes
Copy link
Contributor

@madsmtm madsmtm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

To be clear, the affected targets as far as I can tell are:

  • aarch64-apple-watchos (likely a correct change, though might not be, watchOS is kinda weird sometimes)
  • armv7s-apple-ios
  • armv7k-apple-watchos
  • arm64_32-apple-watchos
  • x86_64-apple-darwin
  • x86_64h-apple-darwin
  • x86_64-apple-ios-macabi
  • x86_64-apple-ios
  • x86_64-apple-tvos
  • x86_64-apple-watchos-sim
  • i686-apple-darwin
  • i386-apple-ios

Are all of these intentional?

An obvious alternative to this PR is to make the default frame-pointer setting something arch-dependent instead.

Do you know what Clang's logic is? I'd be much more comfortable following that, whatever it is.

@workingjubilee
Copy link
Member Author

workingjubilee commented Jun 5, 2025 •
edited
Loading

Are all of these intentional?

Yes.

Previously, they would default to full frame pointers, which seems strictly worse for codegen on the more binary-size-constrained targets.

@workingjubilee
Copy link
Member Author

workingjubilee commented Jun 5, 2025 •
edited
Loading

Do you know what Clang's logic is? I'd be much more comfortable following that, whatever it is.

It seems on x86 platforms, clang indeed uses full frame pointers.
It is also the case for armv7.
clang does use non-leaf frame pointers for aarch64-apple-watchos, as you suspected.
As well as arm64_32-apple-watchos, too.

Clang will also reject -fomit-frame-pointer for the relevant armv7 targets, but supports -target armv7s-apple-ios -emit-llvm -momit-leaf-frame-pointer.

@workingjubilee
Copy link
Member Author

@madsmtm Part of my reasoning, aside from it being a simplifying assumption, is that since -momit-leaf-frame-pointer is supported on all of these targets, then it seems reasonable to allow Rust code to omit these frame pointers by default because we do not have that CLI option. -Cforce-frame-pointers only will give you more frame pointers and never less, even if you pass -Cforce-frame-pointers=off.

@workingjubilee
Copy link
Member Author

Anyways, that said, I'm equally happy to just make the value of FramePointer be instead a match on the arch.

@madsmtm
Copy link
Contributor

madsmtm commented Jun 6, 2025 •
edited
Loading

Anyways, that said, I'm equally happy to just make the value of FramePointer be instead a match on the arch.

Yeah, I'd prefer that (so that we'd only be changing the value for aarch64-apple-watchos and arm64_32-apple-watchos).

@workingjubilee workingjubilee force-pushed the apple-likes-frame-pointers-but-not-that-much branch from cca230a to 8d4031e Compare June 6, 2025 16:00
@workingjubilee workingjubilee changed the title compiler: Use FramePointer::NonLeaf for Apples compiler: set Apple frame pointers by architecture Jun 6, 2025
@workingjubilee
Copy link
Member Author

I have implemented the change mentioned.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@workingjubilee
compiler: set Apple frame pointers by architecture
b25aa26 
Apple targets can now overriding this configuration and instead use the
default based on their architecture, which means aarch64 targets now
have less frame pointers in leaf functions.
madsmtm
madsmtm approved these changes Jun 6, 2025
View reviewed changes
Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Thanks! Feel free to r=me after the test change has been reverted:
@bors delegate+ rollup

@workingjubilee workingjubilee force-pushed the apple-likes-frame-pointers-but-not-that-much branch from 8d4031e to b25aa26 Compare June 6, 2025 17:14
@workingjubilee
Copy link
Member Author

literally was just pushing that :^)

@bors r=madsmtm

@bors
Copy link
Collaborator

bors commented Jun 6, 2025

📌 Commit b25aa26 has been approved by madsmtm

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 7, 2025
@jhpratt
Rollup merge of rust-lang#141797 - workingjubilee:apple-likes-frame-p…
5260e7c 
…ointers-but-not-that-much, r=madsmtm

compiler: set Apple frame pointers by architecture

All Apple targets stop overriding this configuration and instead use the default base of FramePointer::NonLeaf, which means some Apples will have less frame pointers in leaf functions.

r? `@madsmtm`

cc `@thomcc`
This was referenced Jun 7, 2025
Closed
Merged
bors added a commit that referenced this pull request Jun 7, 2025
@bors
Auto merge of #142142 - jhpratt:rollup-frlezq2, r=jhpratt
c57119b 
Rollup of 8 pull requests

Successful merges:

 - #137992 (Stabilise `os_string_pathbuf_leak`)
 - #141558 (Limit the size of cgu names when using the `-Zhuman-readable-cgu-name…)
 - #141797 (compiler: set Apple frame pointers by architecture)
 - #141857 (coretests: move float tests from num to floats module and use a more flexible macro to generate them)
 - #142045 (Make obligation cause code suggestions verbose)
 - #142076 (Check documentation of bootstrap in PR CI)
 - #142110 (Add solaris targets to build-manifest)
 - #142131 (Make cast suggestions verbose)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1c8b096 into rust-lang:master Jun 7, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 7, 2025
rust-timer added a commit that referenced this pull request Jun 7, 2025
@rust-timer
Unrolled build for #141797
87953ef 
Rollup merge of #141797 - workingjubilee:apple-likes-frame-pointers-but-not-that-much, r=madsmtm

compiler: set Apple frame pointers by architecture

All Apple targets stop overriding this configuration and instead use the default base of FramePointer::NonLeaf, which means some Apples will have less frame pointers in leaf functions.

r? ``@madsmtm``

cc ``@thomcc``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madsmtm madsmtm madsmtm approved these changes

Assignees

@madsmtm madsmtm

Labels
O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
5 participants
@workingjubilee @rustbot @rust-log-analyzer @madsmtm @bors