Extend `-Cdebuginfo` with new options and named aliases #83947

jdtatz · 2021-04-06T20:11:25Z

The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options.

Fix #60020
Fix #64405

rust-highfive · 2021-04-06T20:11:28Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

lcnr · 2021-04-12T10:46:41Z

not something i am familiar with, maybe

r? @nagisa

compiler/rustc_session/src/options.rs

nagisa · 2021-04-12T20:45:23Z

This overall seems good to me, but it should be accompanied with some tests I think.

jdtatz · 2021-04-23T18:30:20Z

This overall seems good to me, but it should be accompanied with some tests I think.

One issue with at the moment is that debug-info tests are not being ran #47163

nagisa · 2021-04-24T13:58:09Z

The test could be a codegen test verifying the kind of settings LLVM receives and the kind of debug info statements that do or do not show up in the IR. I suspect, for instance, that the optimized IR does not need to contain any @llvm.dbg.declarecalls when just line-tables/directives are being emitted.

(Also see https://rustc-dev-guide.rust-lang.org/tests/intro.html if you haven't yet)

compiler/rustc_codegen_ssa/src/back/linker.rs

nagisa · 2021-05-26T22:29:49Z

Sorry for taking so long to take another look at this!

This looks great, but there is a test failure that needs to be fixed :(

Could you also squash the commits together into 1 or 2 while removing commits such as "Formatting"? Thanks!

nagisa · 2021-05-28T15:36:59Z

During the meeting we had on Thrusday a number of T-compiler members have expressed that they would like to see an MCP for this change. However given that this is already implemented, we could also run an FCP, wdyt @pnkfelix @michaelwoerister?

michaelwoerister · 2021-06-10T12:23:16Z

Question: What exactly is "line-directives-only"?

crlf0710 · 2021-06-26T16:39:35Z

@jdtatz Ping from triage, CI is still red. Would you mind fixing that? Thanks!

JohnCSimon · 2021-07-12T00:13:50Z

@jdtatz Ping again from triage, build is still broken.

rfcbot · 2021-08-19T18:00:26Z

🔔 This is now entering its final comment period, as per the review above. 🔔

apiraino · 2021-08-26T14:37:13Z

The main thing blocking this from landing, from the viewpoint of T-compiler, is the need for improvements to the documentation, especially user-facing stuff.

cc @jdtatz in case the previous notification went lost

@rustbot label -S-waiting-on-team +S-waiting-on-author

rfcbot · 2021-08-29T18:16:50Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

`-Cdebuginfo=1` was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for line tables only was added. Additionally an option for line info directives only was added, which is well needed for some targets. The debug info options should now behave the same as clang's debug info options.

nagisa · 2021-09-24T10:33:02Z

src/doc/rustc/src/codegen-options/index.md

-* `1`: line tables only.
-* `2`: full debug info.
+* `0` or `none`: no debug info at all (the default).
+* `line-directives-only`: line info directives only, (For the nvptx* targets this enables [profiling](https://reviews.llvm.org/D46061), but on other targets the behavior is unspecified).


Hm, claiming unspecified behaviour in our flags unnecessarily feels… bad? Could we just say "enables profiling for some targets" (also what kind of profiling? sampling? instrumented?) for now?

JohnCSimon · 2021-11-08T04:51:57Z

ping from triage:
@jdtatz Can you please address the comment from the reviewer?

bors · 2021-12-30T02:15:34Z

☔ The latest upstream changes (presumably #88354) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2022-01-30T04:45:06Z

Ping from triage:
@jdtatz
I'm closing this due to inactivity, Please reopen when you are ready to continue with this. Thank you.

@rustbot label: +S-inactive

…oerister Extend -Cdebuginfo with new options and named aliases This is a rebase of rust-lang#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below. Note that the changes in this PR have already been through FCP: rust-lang#83947 (comment) Closes rust-lang#109311. Helps with rust-lang#104968. r? `@michaelwoerister` cc `@cuviper` --- The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options. Fix rust-lang#60020 Fix rust-lang#64405

Extend -Cdebuginfo with new options and named aliases This is a rebase of rust-lang/rust#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below. Note that the changes in this PR have already been through FCP: rust-lang/rust#83947 (comment) Closes rust-lang/rust#109311. Helps with rust-lang/rust#104968. r? `@michaelwoerister` cc `@cuviper` --- The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options. Fix rust-lang/rust#60020 Fix rust-lang/rust#64405

rust-highfive assigned lcnr Apr 6, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2021

This comment has been minimized.

Sign in to view

rust-highfive assigned nagisa and unassigned lcnr Apr 12, 2021

nagisa reviewed Apr 12, 2021

View reviewed changes

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

nagisa reviewed Apr 12, 2021

View reviewed changes

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

This comment has been minimized.

Sign in to view

nagisa reviewed May 9, 2021

View reviewed changes

compiler/rustc_codegen_ssa/src/back/linker.rs Outdated Show resolved Hide resolved

jdtatz force-pushed the debuginfo-fix branch from a8dca88 to 8571677 Compare May 9, 2021 23:13

This comment has been minimized.

Sign in to view

jdtatz force-pushed the debuginfo-fix branch from 8571677 to 17cc86b Compare May 12, 2021 01:23

pnkfelix added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 27, 2021

apiraino removed the I-nominated label May 27, 2021

crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2021

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Aug 19, 2021

wesleywiser added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Aug 26, 2021

rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Aug 29, 2021

rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 29, 2021

apiraino removed the to-announce Announce this issue on triage meeting label Sep 2, 2021

jdtatz force-pushed the debuginfo-fix branch from 068c479 to c4f33f5 Compare September 3, 2021 15:21

jdtatz added 2 commits September 3, 2021 11:27

More in-depth documentation for the new debuginfo options

8bfc641

jdtatz force-pushed the debuginfo-fix branch from c4f33f5 to 8bfc641 Compare September 3, 2021 17:02

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 20, 2021

nagisa reviewed Sep 24, 2021

View reviewed changes

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 19, 2021

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2021

JohnCSimon closed this Jan 30, 2022

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jan 30, 2022

jyn514 mentioned this pull request Mar 20, 2023

Add a new -Z force-full-debuginfo flag #109311

Closed

jyn514 mentioned this pull request Mar 31, 2023

Extend -Cdebuginfo with new options and named aliases #109808

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend `-Cdebuginfo` with new options and named aliases #83947

Extend `-Cdebuginfo` with new options and named aliases #83947

jdtatz commented Apr 6, 2021 •

edited

Loading

rust-highfive commented Apr 6, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Apr 12, 2021

nagisa commented Apr 12, 2021

jdtatz commented Apr 23, 2021

nagisa commented Apr 24, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

nagisa commented May 26, 2021 •

edited

Loading

nagisa commented May 28, 2021

michaelwoerister commented Jun 10, 2021

crlf0710 commented Jun 26, 2021

JohnCSimon commented Jul 12, 2021

rfcbot commented Aug 19, 2021

apiraino commented Aug 26, 2021

rfcbot commented Aug 29, 2021

nagisa Sep 24, 2021

JohnCSimon commented Nov 8, 2021

bors commented Dec 30, 2021

JohnCSimon commented Jan 30, 2022

Extend -Cdebuginfo with new options and named aliases #83947

Extend -Cdebuginfo with new options and named aliases #83947

Conversation

jdtatz commented Apr 6, 2021 • edited Loading

rust-highfive commented Apr 6, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Apr 12, 2021

nagisa commented Apr 12, 2021

jdtatz commented Apr 23, 2021

nagisa commented Apr 24, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

nagisa commented May 26, 2021 • edited Loading

nagisa commented May 28, 2021

michaelwoerister commented Jun 10, 2021

crlf0710 commented Jun 26, 2021

JohnCSimon commented Jul 12, 2021

rfcbot commented Aug 19, 2021

apiraino commented Aug 26, 2021

rfcbot commented Aug 29, 2021

nagisa Sep 24, 2021

Choose a reason for hiding this comment

JohnCSimon commented Nov 8, 2021

bors commented Dec 30, 2021

JohnCSimon commented Jan 30, 2022

Extend `-Cdebuginfo` with new options and named aliases #83947

Extend `-Cdebuginfo` with new options and named aliases #83947

jdtatz commented Apr 6, 2021 •

edited

Loading

nagisa commented Apr 24, 2021 •

edited

Loading

nagisa commented May 26, 2021 •

edited

Loading