rustc_mir: track inlined callees in SourceScopeData. #68965

eddyb · 2020-02-08T20:51:42Z

We now record which MIR scopes are the roots of other (inlined) functions's scope trees, which allows us to generate the correct debuginfo in codegen, similar to what LLVM inlining generates.
This PR makes the ui test backtrace-debuginfo pass, if the MIR inliner is turned on by default.

Also, #[track_caller] is now correct in the face of MIR inlining (cc @anp).

Fixes #76997.

r? @rust-lang/wg-mir-opt

src/librustc_mir/transform/inline.rs

eddyb · 2020-02-08T22:55:08Z

If a cross-crate #[track_caller] function is inlined into a non-#[track_caller] function

The #[track_caller]'s function's entire body can be ignored, since the "caller location" span will be that of the call site inside the non-#[track_caller] function.

However, you're right there is a problem, it's just not in inlining: #47809 (comment)

eddyb · 2020-02-09T02:18:31Z

I want to see if the #[track_caller] changes add too much overhead:
@bors try @rust-timer queue

rust-timer · 2020-02-09T02:18:32Z

Awaiting bors try build completion

bors · 2020-02-09T02:18:42Z

⌛ Trying commit c43d529 with merge 5f38376...

@anp

rustc_mir: track inlined callees in SourceScopeData. This would be a prerequisite for generating the correct debuginfo in codegen, which would look similar to what LLVM inlining generates (we should be able to "fake it" as well). Also, `#[track_caller]` is now correct in the face of MIR inlining (cc @anp). However, the debuginfo side isn't implemented yet. I might take a stab at it soon, not sure. r? @rust-lang/wg-mir-opt

wesleywiser · 2020-02-09T03:01:20Z

src/librustc_mir/interpret/intrinsics/caller_location.rs

+            if let Some(source_info) = frame.current_source_info() {
+                // Walk up the `SourceScope`s, in case some of them are from MIR inlining.
+                let mut scope = source_info.scope;
+                loop {


Would be great if we could deduplicate this logic.

Probably a method on mir::Body?

bors · 2020-02-09T04:45:38Z

☀️ Try build successful - checks-azure
Build commit: 5f38376 (5f3837675c20728121185495e89d73dc1ac55dfa)

rust-timer · 2020-02-09T04:45:40Z

Queued 5f38376 with parent a19edd6, future comparison URL.

rust-timer · 2020-02-09T10:04:39Z

Finished benchmarking try commit 5f38376, comparison URL.

eddyb · 2020-02-09T10:24:42Z

What this tells me is that walking up the scope tree for every get_caller_location is wasteful.
I'm wondering whether it would be better to add a separate "inlined call tree" or just another Option<SourceScope> for the "inline root" (a scope with inlined: Some(...)).

eddyb · 2020-02-09T18:21:54Z

Added a second Option<SourceScope> parent chain that skips directly to the inlined root scope.
@bors try @rust-timer queue

rust-timer · 2020-02-09T18:21:55Z

Awaiting bors try build completion

bors · 2020-02-09T18:22:04Z

⌛ Trying commit c8c1f61e4bdb0bea14c6df1ff69dd3a93f0fd149 with merge 43b730a168b06d9629d3e3a4a42a885eb565c2be...

bors · 2020-02-09T20:55:16Z

☀️ Try build successful - checks-azure
Build commit: 43b730a168b06d9629d3e3a4a42a885eb565c2be (43b730a168b06d9629d3e3a4a42a885eb565c2be)

rust-timer · 2020-02-09T20:55:18Z

Queued 43b730a168b06d9629d3e3a4a42a885eb565c2be with parent 6dff769, future comparison URL.

rust-timer · 2020-02-10T08:10:24Z

Finished benchmarking try commit 43b730a168b06d9629d3e3a4a42a885eb565c2be, comparison URL.

bjorn3 · 2020-02-10T09:40:36Z

syn-opt clean build has a 9% regression in mir->llvm translation (~3% overall regression) The rest of the regressions don't seem to have a noticable change in the self-profiling data.

eddyb · 2020-02-10T12:20:24Z

This looks very similar to the last run except that there should be be no inlined_parent_scope for the loop to even be entered, this time, I'm confused.
The whole point of this second run was that it should be boring.

bjorn3 · 2020-02-10T12:56:28Z

The regressions are smaller this time.

eddyb · 2020-02-10T17:55:10Z

The regressions are smaller this time.

Only for syn-opt, which is the noise generator.
This time there's even more 1% regressions.
Oh and if you look past the red, on both of them, there's a bunch of max: 0.6% and max: 0.4%.
So something is certainly going wrong.

eddyb · 2020-10-23T16:10:14Z

r? @nagisa for LLVM (I wish we had GH teams specifically for review roles so I don't have to guess)

nagisa

There are a number of tests for MIR / at MIR level that have changed, which is fine; I'd love to see a test against LLVM-IR too.

From what I've seen in MIR tests, we might also want to have a way to run only a specific MIR optimisation at least for test purposes… sometime down the line.

See comments, but otherwise the changes LGTM (though I didn't pay super close attention to MIR ones, trusting Oli's review).

nagisa · 2020-10-25T23:19:02Z

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

+        span: Span,
+    ) -> S {
+        // FIXME(eddyb) this should never be `None`.
+        let dbg_scope = self.dbg_scope.unwrap();


expect and/or unwrap_or_else(|| bug!(...))?

nagisa · 2020-10-25T23:25:15Z

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

+
+                        let arg_ty = self.monomorphize(&decl.ty);
+
+                        let span = self.adjust_span_for_debugging(decl.source_info.span);


This pattern appears to be repeated multiple times throughout the codebase. It might make sense to just have a method that does these three steps in a single location? Not sure what'd it be named tho… adjusted_span_and_dbg_scope maybe?

oli-obk · 2020-10-26T12:46:14Z

From what I've seen in MIR tests, we might also want to have a way to run only a specific MIR optimisation at least for test purposes… sometime down the line.

The way mir opts are being run is being reworked, and I think something like overwriting the optimization pipeline on a per-opt basis was also on the menu

nagisa · 2020-10-26T14:54:35Z

@bors r=nagisa,oli-obk

bors · 2020-10-26T14:54:37Z

📌 Commit 2b3f009 has been approved by nagisa,oli-obk

bors · 2020-10-26T18:50:27Z

⌛ Testing commit 2b3f009 with merge 0da6d42...

bors · 2020-10-26T21:16:24Z

☀️ Test successful - checks-actions
Approved by: nagisa,oli-obk
Pushing 0da6d42 to master...

This went unused in commit 88d874d, part of rust-lang#68965.

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2020

wesleywiser reviewed Feb 8, 2020

View reviewed changes

src/librustc_mir/transform/inline.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

eddyb mentioned this pull request Feb 8, 2020

Tracking issue for RFC 2091: Implicit caller location #47809

Closed

3 tasks

eddyb force-pushed the mir-inline-scope branch from 0f373ea to 5c3ec91 Compare February 9, 2020 00:40

wesleywiser reviewed Feb 9, 2020

View reviewed changes

eddyb force-pushed the mir-inline-scope branch from c43d529 to c8c1f61 Compare February 9, 2020 18:21

rust-highfive assigned nagisa Oct 23, 2020

nagisa approved these changes Oct 25, 2020

View reviewed changes

oli added 4 commits October 26, 2020 12:49

Prefer bug! over unwrap()

f8aae50

Deduplicate span and dbg_scope adjustment

7101ce9

Add an inlining debuginfo test

7ae8bc3

Ignore long lines in test

2b3f009

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 Oct 26, 2020

wesleywiser added the A-mir-opt Area: MIR optimizations label Oct 26, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2020

bors merged commit 0da6d42 into rust-lang:master Oct 26, 2020

rustbot added this to the 1.49.0 milestone Oct 26, 2020

This was referenced Oct 26, 2020

[mir-opt] Allow debuginfo to be generated for a constant or a Place #73210

Merged

I can't stop writing copy propagation passes #76723

Closed

This was referenced Oct 29, 2020

type parameter V/#2 (V/2) out of range when substituting with MIR inlining enabled #78529

Closed

Infinite loop when inlining #78573

Closed

eddyb deleted the mir-inline-scope branch October 31, 2020 13:38

cuviper added a commit to cuviper/rust that referenced this pull request Jan 13, 2021

Remove the unused context from CreateDebugLocation

0342fd1

This went unused in commit 88d874d, part of rust-lang#68965.

cuviper mentioned this pull request Jan 13, 2021

llvm: Remove the unused context from CreateDebugLocation #80990

Merged

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 14, 2021

Rollup merge of rust-lang#80990 - cuviper:unused-debug-context, r=nagisa

800aa61

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 14, 2021

Rollup merge of rust-lang#80990 - cuviper:unused-debug-context, r=nagisa

6ae4d31

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 14, 2021

Rollup merge of rust-lang#80990 - cuviper:unused-debug-context, r=nagisa

14d78d8

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 14, 2021

Rollup merge of rust-lang#80990 - cuviper:unused-debug-context, r=nagisa

896757f

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 14, 2021

Rollup merge of rust-lang#80990 - cuviper:unused-debug-context, r=nagisa

494cb3c

llvm: Remove the unused context from CreateDebugLocation This went unused in commit 88d874d, part of rust-lang#68965.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_mir: track inlined callees in SourceScopeData. #68965

rustc_mir: track inlined callees in SourceScopeData. #68965

eddyb commented Feb 8, 2020 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Feb 8, 2020 •

edited

Loading

eddyb commented Feb 9, 2020

rust-timer commented Feb 9, 2020

bors commented Feb 9, 2020

wesleywiser Feb 9, 2020

eddyb Feb 9, 2020

bors commented Feb 9, 2020

rust-timer commented Feb 9, 2020

rust-timer commented Feb 9, 2020

eddyb commented Feb 9, 2020

eddyb commented Feb 9, 2020

rust-timer commented Feb 9, 2020

bors commented Feb 9, 2020

bors commented Feb 9, 2020

rust-timer commented Feb 9, 2020

rust-timer commented Feb 10, 2020

bjorn3 commented Feb 10, 2020

eddyb commented Feb 10, 2020

bjorn3 commented Feb 10, 2020

eddyb commented Feb 10, 2020

eddyb commented Oct 23, 2020

nagisa left a comment

nagisa Oct 25, 2020

nagisa Oct 25, 2020

oli-obk commented Oct 26, 2020

nagisa commented Oct 26, 2020

bors commented Oct 26, 2020

bors commented Oct 26, 2020

bors commented Oct 26, 2020


		let arg_ty = self.monomorphize(&decl.ty);

		let span = self.adjust_span_for_debugging(decl.source_info.span);

rustc_mir: track inlined callees in SourceScopeData. #68965

rustc_mir: track inlined callees in SourceScopeData. #68965

Conversation

eddyb commented Feb 8, 2020 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Feb 8, 2020 • edited Loading

eddyb commented Feb 9, 2020

rust-timer commented Feb 9, 2020

bors commented Feb 9, 2020

wesleywiser Feb 9, 2020

Choose a reason for hiding this comment

eddyb Feb 9, 2020

Choose a reason for hiding this comment

bors commented Feb 9, 2020

rust-timer commented Feb 9, 2020

rust-timer commented Feb 9, 2020

eddyb commented Feb 9, 2020

eddyb commented Feb 9, 2020

rust-timer commented Feb 9, 2020

bors commented Feb 9, 2020

bors commented Feb 9, 2020

rust-timer commented Feb 9, 2020

rust-timer commented Feb 10, 2020

bjorn3 commented Feb 10, 2020

eddyb commented Feb 10, 2020

bjorn3 commented Feb 10, 2020

eddyb commented Feb 10, 2020

eddyb commented Oct 23, 2020

nagisa left a comment

Choose a reason for hiding this comment

nagisa Oct 25, 2020

Choose a reason for hiding this comment

nagisa Oct 25, 2020

Choose a reason for hiding this comment

oli-obk commented Oct 26, 2020

nagisa commented Oct 26, 2020

bors commented Oct 26, 2020

bors commented Oct 26, 2020

bors commented Oct 26, 2020

eddyb commented Feb 8, 2020 •

edited

Loading

eddyb commented Feb 8, 2020 •

edited

Loading