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

Don't force-enable frame pointers when generating debug info #48785

Closed
nikomatsakis opened this Issue Mar 6, 2018 · 4 comments

Comments

Projects
None yet
6 participants
@nikomatsakis
Contributor

nikomatsakis commented Mar 6, 2018

In #47152, we decided to add a flag to force enable frame pointers (that is, to prevent the compiler from optimizing them away). This flag was to be insta-stable because it is needed, e.g. to help profilers like perf that need to reconstruct the stack. @dotdash then came back with a few proposed changes. Unfortunately, they were never able to finalize the rebase, and the PR was closed by triage for inactivity. This is where you come in, dear reader! Perhaps you can help us!

There was some dispute about what the final interface ought to be though. @dotdash proposed -Comit-frame-pointer=[yes|no], but @Mark-Simulacrum proposed -Cframe-pointers=[auto, yes, no], which seems logical enough.

However we say it, the default behavior (when user says nothing) probably wants to be:

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 6, 2018

cc @rust-lang/compiler -- we should settle on a plan here, no?

@nagisa

This comment has been minimized.

Contributor

nagisa commented Mar 6, 2018

-Cframe-pointers=val matches our current direction of CLI design. We should change to that ASAP if we want to keep the compatibility.

The defaults can always be changed later, though. The default should be specified in the target files.

I’ll fiddle with this.

@Centril Centril added the A-debuginfo label Mar 7, 2018

kennytm added a commit to kennytm/rust that referenced this issue Mar 21, 2018

Rollup merge of rust-lang#48786 - nagisa:fp, r=nikomatsakis
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of rust-lang#47152 plus some changes suggested by rust-lang#48785.

Fixes rust-lang#11906

r? @nikomatsakis

kennytm added a commit to kennytm/rust that referenced this issue Mar 22, 2018

Rollup merge of rust-lang#48786 - nagisa:fp, r=nikomatsakis
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of rust-lang#47152 plus some changes suggested by rust-lang#48785.

Fixes rust-lang#11906

r? @nikomatsakis

bors added a commit that referenced this issue Apr 4, 2018

Auto merge of #48786 - nagisa:fp, r=nikomatsakis
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of #47152 plus some changes suggested by #48785.

Fixes #11906

r? @nikomatsakis

bors added a commit that referenced this issue Apr 18, 2018

Auto merge of #48786 - nagisa:fp, r=<try>
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of #47152 plus some changes suggested by #48785.

Fixes #11906

r? @nikomatsakis

bors added a commit that referenced this issue Apr 18, 2018

Auto merge of #48786 - nagisa:fp, r=<try>
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of #47152 plus some changes suggested by #48785.

Fixes #11906

r? @nikomatsakis

bors added a commit that referenced this issue May 1, 2018

Auto merge of #48786 - nagisa:fp, r=nikomatsakis
Add force-frame-pointer flag to allow control of frame pointer ommision

Rebase of #47152 plus some changes suggested by #48785.

Fixes #11906

r? @nikomatsakis
@jonhoo

This comment has been minimized.

Contributor

jonhoo commented Sep 21, 2018

Should this be closed given that #48786 has landed?

@sfackler

This comment has been minimized.

Member

sfackler commented Sep 21, 2018

I think so!

@sfackler sfackler closed this Sep 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment