Support `-Cforce-frame-pointers=non-leaf` #124733

workingjubilee · 2024-05-05T01:24:06Z

Why don't we already support this...?

Suggested impl for rust-lang/compiler-team#744

rustbot · 2024-05-05T01:24:13Z

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-05-05T01:24:15Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

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

Zalathar · 2024-05-05T03:13:20Z

compiler/rustc_session/src/config.rs

@@ -2468,6 +2468,15 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
        }
    }

+    if !nightly_options::is_unstable_enabled(matches)


Annoyingly, there seem to be multiple different ways of checking for -Zunstable-options when implementing these ad-hoc value checks:

Some code uses nightly_options::is_unstable_enabled(matches), which parses the value independently

Some code uses unstable_opts.unstable_options, which uses whatever has already been parsed into the UnstableOptions struct via normal means

In this context, we should be able to just check unstable_opts.unstable_options, which seems like the less hacky variant.

(I assume you probably just copied this from the code immediately below. 🤷)

I did check the unstable_options.unstable_options thing, but it says you shouldn't touch it directly, so I avoided poking it directly. idk lol?

I think there should be a different way of specifying "these values are unstable" that uses a pattern or pattern-guard of some kind.

I realize there's only a few cases of this so it seems like a waste, but I think a profusion of flags is partly the fault of it not being easy to add such patterns that determine stability.

I don’t want to derail this PR too much, but if I get a chance I’ll open a separate issue or thread to discuss improvements to unstable values of stable flags, because the status quo has been annoying me too.

compiler/rustc_session/src/options.rs

jsgf · 2024-05-14T18:50:09Z

compiler/rustc_session/src/config.rs

+        early_dcx.early_fatal(
+            "`-Cforce-frame-pointers=non-leaf` also requires `-Zunstable-options` \
+                and a nightly compiler",
+        )


Do we really need this? What's the concern? It's already de-facto enabled via --target=spec.json.

Target specs are also unstable.

Mind, we could land it as instantly stable, I agree it has strong precedent, but then it needs to wait on a full T-compiler FCP, so "this can happen right now without requiring everyone to agree" was why I proposed doing things "correctly".

compiler/rustc_session/src/options.rs

compiler/rustc_target/src/spec/mod.rs

workingjubilee · 2024-05-14T19:12:16Z

#125127 was opened which suggests "always" and "never" as other options. I think adding "always" to this is fine if there's no objection from the reviewer but I don't believe we need more options for saying "no" because "never" is even more confusing than "no" or "off" (because it is strong enough it might come off as "force frame pointers to NEVER HAPPEN!!!" which is not at all what will occur).

compiler/rustc_interface/src/tests.rs

workingjubilee · 2024-05-26T21:01:54Z

rust-lang/compiler-team#744 was accepted.

workingjubilee · 2024-06-11T00:15:45Z

r? T-compiler

compiler-errors · 2024-06-18T15:31:31Z

I have no context about this, sorry

r? compiler

workingjubilee · 2024-06-22T23:00:55Z

r? compiler

saethlin · 2024-06-23T02:23:00Z

r? saethlin
@bors r+

Requires -Zunstable-options as this is a -C flag already.

Also lands behind -Zunstable-options, for now. Take the opportunity to do some mild cleanup.

all of these currently force stronger frame pointers, and currently the CLI does not override the target

workingjubilee · 2024-06-23T07:42:03Z

there... should have rebased so that the build worked again locally to start...

workingjubilee · 2024-06-23T07:43:52Z

@bors r=saethlin

bors · 2024-06-23T07:43:55Z

📌 Commit c72a6ad has been approved by saethlin

It is now in the queue for this repository.

bors · 2024-06-23T07:45:16Z

⌛ Testing commit c72a6ad with merge 45af7d9...

…ave-this, r=saethlin Support `-Cforce-frame-pointers=non-leaf` Why don't we already support this...? Suggested impl for rust-lang/compiler-team#744

bors · 2024-06-23T08:55:54Z

💔 Test failed - checks-actions

workingjubilee · 2024-06-23T09:23:30Z

fuchsia...

@bors r=saethlin

bors · 2024-06-23T09:23:33Z

📌 Commit 761ba5b has been approved by saethlin

It is now in the queue for this repository.

bors · 2024-06-23T11:12:14Z

⌛ Testing commit 761ba5b with merge c3d7fb3...

bors · 2024-06-23T13:24:08Z

☀️ Test successful - checks-actions
Approved by: saethlin
Pushing c3d7fb3 to master...

rust-timer · 2024-06-23T14:52:02Z

Finished benchmarking commit (c3d7fb3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary 2.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.1%, 2.6%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (secondary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Bootstrap: 691.495s -> 693.516s (0.29%)
Artifact size: 326.88 MiB -> 326.77 MiB (-0.04%)

rustbot assigned TaKO8Ki May 5, 2024

rustbot added 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 5, 2024

workingjubilee added the S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. label May 5, 2024

Zalathar reviewed May 5, 2024

View reviewed changes

jieyouxu mentioned this pull request May 14, 2024

Add support for -Cforce-frame-poiners=non-leaf #125127

Closed

jsgf reviewed May 14, 2024

View reviewed changes

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

jsgf reviewed May 14, 2024

View reviewed changes

compiler/rustc_session/src/options.rs Show resolved Hide resolved

jsgf reviewed May 14, 2024

View reviewed changes

compiler/rustc_target/src/spec/mod.rs Show resolved Hide resolved

workingjubilee force-pushed the cant-beleaf-we-dont-have-this branch from d3204f4 to f5ad116 Compare May 14, 2024 19:43

This comment has been minimized.

Sign in to view

workingjubilee force-pushed the cant-beleaf-we-dont-have-this branch 2 times, most recently from bbd9930 to f705368 Compare May 14, 2024 19:59

jsgf reviewed May 16, 2024

View reviewed changes

compiler/rustc_interface/src/tests.rs Show resolved Hide resolved

workingjubilee removed the S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. label May 26, 2024

rustbot assigned compiler-errors and unassigned TaKO8Ki Jun 11, 2024

rustbot assigned TaKO8Ki and unassigned compiler-errors Jun 18, 2024

rustbot assigned estebank and unassigned TaKO8Ki Jun 22, 2024

workingjubilee added 4 commits June 23, 2024 00:36

test: for frame-pointer=non-leaf codegen opts

f301d08

compiler: Support nightly -Cforce-frame-pointers=non-leaf

7d160ae

Requires -Zunstable-options as this is a -C flag already.

compiler: Accept -Cforce-frame-pointers=always

598e265

Also lands behind -Zunstable-options, for now. Take the opportunity to do some mild cleanup.

test: ignore force-frame-pointers test on some targets

c72a6ad

all of these currently force stronger frame pointers, and currently the CLI does not override the target

workingjubilee force-pushed the cant-beleaf-we-dont-have-this branch from ed9bb9f to c72a6ad Compare June 23, 2024 07:41

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 23, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 23, 2024

compiler: parse the empty frame-pointer

761ba5b

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 23, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 23, 2024

bors merged commit c3d7fb3 into rust-lang:master Jun 23, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jun 23, 2024

This was referenced Jun 23, 2024

Move ZST ABI handling to rustc_target #125854

Merged

debug-fmt-detail option #123940

Merged

sess: stabilize -C stack-protector=all #121742

Open

workingjubilee deleted the cant-beleaf-we-dont-have-this branch June 23, 2024 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `-Cforce-frame-pointers=non-leaf` #124733

Support `-Cforce-frame-pointers=non-leaf` #124733

workingjubilee commented May 5, 2024

rustbot commented May 5, 2024

rustbot commented May 5, 2024

Zalathar May 5, 2024

workingjubilee May 5, 2024

workingjubilee May 5, 2024

Zalathar May 5, 2024

jsgf May 14, 2024

workingjubilee May 14, 2024

workingjubilee May 14, 2024

workingjubilee commented May 14, 2024 •

edited

Loading

This comment has been minimized.

workingjubilee commented May 26, 2024

workingjubilee commented Jun 11, 2024

compiler-errors commented Jun 18, 2024

workingjubilee commented Jun 22, 2024

saethlin commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

rust-timer commented Jun 23, 2024

Support -Cforce-frame-pointers=non-leaf #124733

Support -Cforce-frame-pointers=non-leaf #124733

Conversation

workingjubilee commented May 5, 2024

rustbot commented May 5, 2024

rustbot commented May 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

workingjubilee commented May 14, 2024 • edited Loading

This comment has been minimized.

workingjubilee commented May 26, 2024

workingjubilee commented Jun 11, 2024

compiler-errors commented Jun 18, 2024

workingjubilee commented Jun 22, 2024

saethlin commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

workingjubilee commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

bors commented Jun 23, 2024

rust-timer commented Jun 23, 2024

Overall result: no relevant changes - no action needed

Support `-Cforce-frame-pointers=non-leaf` #124733

Support `-Cforce-frame-pointers=non-leaf` #124733

workingjubilee commented May 14, 2024 •

edited

Loading