Stream: t-compiler/wg-mir-opt

Topic: preserving debuginfo in optimizations


bjorn3 (Feb 08 2020 at 11:45, on Zulip):

For line debuginfo generate a unique SourceLoc for each mir::SourceLoc. To map from Inst to address range, I just use func.inst_offsets on the function after compilation.

eddyb (Feb 08 2020 at 11:49, on Zulip):

that's super cool that Cranelift can give you access to all that

eddyb (Feb 08 2020 at 11:50, on Zulip):

I kind of wish the cranelift backend was in-tree and I had more time to help on it, but I'm glad someone's doing it :D

eddyb (Feb 08 2020 at 11:50, on Zulip):

@bjorn3 btw did you see the clusterfudge that LLVM's SSA value debuginfo is? https://github.com/rust-lang/rust/issues/68817

eddyb (Feb 08 2020 at 11:51, on Zulip):

AFAICT there's no way to force LLVM into a mode of "I really want this value computed and kept around because I'm in debug mode" without throwing it onto the stack

eddyb (Feb 08 2020 at 11:52, on Zulip):

Cranelift looks much better positioned to allow this

bjorn3 (Feb 08 2020 at 11:53, on Zulip):

Yes I saw that. Cranelift currently is not much better, as it also discards values after their last use. Adding a instruction which pretends to read a value to Cranelift may fix it though.

bjorn3 (Feb 08 2020 at 11:54, on Zulip):

I haven't spent much time on local debuginfo yet. cg_clif does have some rudimentary support currently, but I disabled it as it doesn't handle much types and value locations.

eddyb (Feb 08 2020 at 11:54, on Zulip):

yeah I think you need to find the last instruction that's in a lexical scope that's nested inside the scope of a variable and add some sort of "debuginfo fence"

eddyb (Feb 08 2020 at 11:54, on Zulip):

@bjorn3 IMO, all of the type debuginfo should be shared between LLVM and Cranelift

bjorn3 (Feb 08 2020 at 11:55, on Zulip):

Yeah, I have been planning to do that some day, but I haven't done it yet. I think I will give it a shot today though.

eddyb (Feb 08 2020 at 11:55, on Zulip):

I have some vague ideas of how we might be able to do this, we just need to find an abstraction (probably based around gimli)

bjorn3 (Feb 08 2020 at 11:56, on Zulip):

(Should we split this out in a new topic)

eddyb (Feb 08 2020 at 11:56, on Zulip):

shrug

bjorn3 (Feb 08 2020 at 11:58, on Zulip):

I was thinking about creating a new datastructure, which has the same shape as the output DWARF. I think using gimli will be hard to translate back to LLVM.

eddyb (Feb 08 2020 at 11:59, on Zulip):

I was thinking some kind of API which is easy to emit both LLVM and gimli from. not literally using gimli, that would indeed not be convertable back to LLVM

bjorn3 (Feb 08 2020 at 11:59, on Zulip):

You mean a trait? That should be possible

eddyb (Feb 08 2020 at 11:59, on Zulip):

yeah like the Builder trait

bjorn3 (Feb 08 2020 at 12:04, on Zulip):

/me is waiting for git pull to complete so that I can work on this.

bjorn3 (Feb 08 2020 at 15:13, on Zulip):

At https://github.com/rust-lang/rust/blob/6cad7542da2f10e2110f942de4db59716bacb3df/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs#L67 I don't understand why skipping the creation of a nested scope can cause shadowing problems.

eddyb (Feb 08 2020 at 15:15, on Zulip):

because you could have the same name twice in the same scope

bjorn3 (Feb 08 2020 at 15:16, on Zulip):

How? let a; let a; creates the second a in a subscope, right?

bjorn3 (Feb 08 2020 at 15:16, on Zulip):

That code is only for scopes without variables defined

eddyb (Feb 08 2020 at 15:17, on Zulip):

hmm

eddyb (Feb 08 2020 at 15:18, on Zulip):

@bjorn3 I think that code might be from a time when argument scope wasn't the outermost scope

bjorn3 (Feb 08 2020 at 15:19, on Zulip):

So I can always perform an early return when there are no variables in a scope?

eddyb (Feb 08 2020 at 15:20, on Zulip):

we should test it

bjorn3 (Feb 08 2020 at 15:21, on Zulip):

What should be the test?

eddyb (Feb 08 2020 at 15:21, on Zulip):

just change it and see if debuginfo tests pass

eddyb (Feb 08 2020 at 15:21, on Zulip):

I could probably do it right now myself

bjorn3 (Feb 08 2020 at 15:21, on Zulip):

:+1:

eddyb (Feb 08 2020 at 15:54, on Zulip):

@bjorn3 seems to work fine :)

bjorn3 (Feb 08 2020 at 15:55, on Zulip):

Thanks for testing. I will remove it then.

eddyb (Feb 08 2020 at 17:51, on Zulip):

@bjorn3 I included it in https://github.com/rust-lang/rust/pull/68960

eddyb (Feb 08 2020 at 17:51, on Zulip):

but what I was working on is https://github.com/rust-lang/rust/pull/68961

eddyb (Feb 08 2020 at 17:52, on Zulip):

which I'm excited for, especially if we start doing MIR inlining in debug mode (just very limited)

eddyb (Feb 08 2020 at 17:53, on Zulip):

@Wesley Wiser I think we should seriously consider recording calls being inlined, in MIR scope trees

eddyb (Feb 08 2020 at 17:55, on Zulip):

I don't see any reason we can't generate what LLVM inlining generates, in terms of debuginfo

eddyb (Feb 08 2020 at 17:55, on Zulip):

it's not like LLVM can tell apart LLVM IR that its own passes generated vs anything else :P

bjorn3 (Feb 08 2020 at 18:53, on Zulip):

bjorn3 I included it in https://github.com/rust-lang/rust/pull/68960

I also included it in #68959 :) I will wait with marking it as ready until I you're PR is merged and I have some more refactorings towards moving most debuginfo code to cg_ssa.

eddyb (Feb 08 2020 at 19:29, on Zulip):

@Wesley Wiser @bjorn3 I'm now testing a rustc which remembers which MIR scopes came from inlining a call :D

eddyb (Feb 08 2020 at 19:29, on Zulip):

no codegen for it, but I might tackle that soon if it's not too asinine

eddyb (Feb 08 2020 at 19:32, on Zulip):

error: internal compiler error: src/librustc_mir/shim.rs:99: creating shims from intrinsics (Intrinsic(DefId(2:981 ~ core[9627]::intrinsics[0]::[1]::transmute[0]))) is unsupported

eddyb (Feb 08 2020 at 19:32, on Zulip):

lmao

Zoxc (Feb 08 2020 at 20:47, on Zulip):

@eddyb Btw I'd like a flag which disables/enables optimizations that hurt the debugging experience. We kind of need LLVM to obey it to be useful though (unless we get a lot of MIR opts)

eddyb (Feb 08 2020 at 20:47, on Zulip):

I definitely want a LLVM mode that makes llvm.dbg.value actually work :P

eddyb (Feb 08 2020 at 20:51, on Zulip):

taadaa https://github.com/rust-lang/rust/pull/68965

Wesley Wiser (Feb 08 2020 at 20:55, on Zulip):

@eddyb is on :fire: today! :slight_smile:

eddyb (Feb 08 2020 at 20:58, on Zulip):

it's a nice distraction-free weekend :D

eddyb (Feb 08 2020 at 21:47, on Zulip):

@Wesley Wiser heh cool that you tried. I wonder if this is a shim or something

eddyb (Feb 08 2020 at 21:48, on Zulip):
#0 [optimized_mir] processing
    `<<collections::vec_deque::VecDeque<T> as core::ops::Drop>::drop::Dropper<'a, T> as core::ops::Drop>::drop`
Wesley Wiser (Feb 08 2020 at 21:48, on Zulip):

It's the first thing I think to do when I see a pr touching the inliner lol

Wesley Wiser (Feb 08 2020 at 21:48, on Zulip):

We regress far too easily

Wesley Wiser (Feb 08 2020 at 21:49, on Zulip):

I believe it's coming from a change in your second commit FYI

Wesley Wiser (Feb 08 2020 at 21:49, on Zulip):

bisecting to be sure

eddyb (Feb 08 2020 at 21:50, on Zulip):

I mean, it's clearly inlining this :P https://github.com/rust-lang/rust/blob/master/src/liballoc/collections/vec_deque.rs#L153

eddyb (Feb 08 2020 at 21:51, on Zulip):

should be easy to repro

eddyb (Feb 08 2020 at 21:55, on Zulip):

ooooh the real_drop_in_place name is gone

eddyb (Feb 08 2020 at 21:58, on Zulip):

the param env is wrong here but I doubt that's it https://github.com/rust-lang/rust/blob/master/src/librustc/ty/instance.rs#L288

eddyb (Feb 08 2020 at 21:59, on Zulip):

oh this is giving me a headache

eddyb (Feb 08 2020 at 22:00, on Zulip):

@Wesley Wiser how hard would it be for you to test a small commit?

eddyb (Feb 08 2020 at 22:03, on Zulip):

actually, I take that back, the bug is in instance and/or shim

eddyb (Feb 08 2020 at 22:13, on Zulip):

@Wesley Wiser @varkor @oli okay so there's a dark invariant for the InstanceDef variants that have a Ty - all sorts of things can break if the type isn't monomorphic (which means the substs are actually pointless and should always be empty)

eddyb (Feb 08 2020 at 22:16, on Zulip):

actually, no, the substs should be valid for the DefId, but other than that, the monomorphic type restriction should still hold

eddyb (Feb 08 2020 at 22:17, on Zulip):

we could do better by extracting a "shallow skeleton", but it's a lot of work I guess

eddyb (Feb 08 2020 at 22:56, on Zulip):

@Aaron Hill o/

eddyb (Feb 08 2020 at 22:58, on Zulip):

@Aaron Hill not sure I did this Zulip subscribe thing right, does this notify you :P?

Aaron Hill (Feb 08 2020 at 23:20, on Zulip):

looks like it did

Aaron Hill (Feb 08 2020 at 23:21, on Zulip):

that mir inlining issue with #[tracked_caller] is actually the reason why I started working on the hygiene/span serialization fixes :)

eddyb (Feb 08 2020 at 23:21, on Zulip):

wow

eddyb (Feb 08 2020 at 23:22, on Zulip):

@Aaron Hill wait did you just remove the track_caller check, and tried that?

Aaron Hill (Feb 08 2020 at 23:22, on Zulip):

it was bothering me that we couldn't inline 'unwrap' without panic message regressions

Aaron Hill (Feb 08 2020 at 23:22, on Zulip):

yup

Aaron Hill (Feb 08 2020 at 23:22, on Zulip):

it seemed to work, otherwise

eddyb (Feb 08 2020 at 23:22, on Zulip):

okay, so, the reason that check is there, has nothing to do with cross-crate spans

Aaron Hill (Feb 08 2020 at 23:22, on Zulip):

but the printed message changed to a macro invocation for 'unwrap'

eddyb (Feb 08 2020 at 23:22, on Zulip):

there are a bunch of tests missing

eddyb (Feb 08 2020 at 23:22, on Zulip):

I just added them on my branch

Aaron Hill (Feb 08 2020 at 23:22, on Zulip):

well, I didn't do any further testing after I saw that message

eddyb (Feb 08 2020 at 23:22, on Zulip):

basically that check wasn't tested

eddyb (Feb 08 2020 at 23:23, on Zulip):

the reason is there is because you need to somehow remember that you inlined some code

Aaron Hill (Feb 08 2020 at 23:23, on Zulip):

what do you mean?

Aaron Hill (Feb 08 2020 at 23:23, on Zulip):

From what I saw, the actual passing of the caller location happens during codegen

eddyb (Feb 08 2020 at 23:23, on Zulip):

if you just inline then the Span won't be in the caller

eddyb (Feb 08 2020 at 23:23, on Zulip):

it will be in the callee

eddyb (Feb 08 2020 at 23:24, on Zulip):

codegen has to be able to walk up the "chain" of inlines, going through #[track_caller] ones, until it finds the actual call site that matters

eddyb (Feb 08 2020 at 23:24, on Zulip):

it might be easier to show rather than tell :P

eddyb (Feb 08 2020 at 23:25, on Zulip):

(since I was supposed to implement this, but got distracted by the bug you found)

Aaron Hill (Feb 08 2020 at 23:25, on Zulip):

oh, I see

Aaron Hill (Feb 08 2020 at 23:25, on Zulip):

expansion_cause would resolve to the macro invocation location in unwrap

eddyb (Feb 08 2020 at 23:26, on Zulip):

but you don't even want to do expansion_cause for any span inside the unwrap, you want to first find the real callsite

eddyb (Feb 08 2020 at 23:26, on Zulip):

which only the inliner knew, and it threw it away

Aaron Hill (Feb 08 2020 at 23:26, on Zulip):

I see

eddyb (Feb 08 2020 at 23:26, on Zulip):

that's where https://github.com/rust-lang/rust/pull/68965 comes in

eddyb (Feb 08 2020 at 23:27, on Zulip):

(I've been meaning to do this for a while, and the current climate of "let's turn the inliner on" is a good excuse to do it for debuginfo)

eddyb (Feb 08 2020 at 23:27, on Zulip):

@Aaron Hill anyway this would've been obvious if tests were added which actually turned the inliner on lol

Aaron Hill (Feb 08 2020 at 23:28, on Zulip):

so, it sounds like the hygiene change might not even be necessary for #[track_caller], lol

eddyb (Feb 08 2020 at 23:28, on Zulip):

@Aaron Hill these tests now fail if you remove the check for no track_caller, from the inliner https://github.com/rust-lang/rust/pull/68965/commits/0f373ea4648e1a8334662fc7580f5b6d20607f10

eddyb (Feb 08 2020 at 23:29, on Zulip):

well, you do need the hygiene change to make it work correctly in many other cases, but it's mostly codegen rather than MIR inlining, that is affected

eddyb (Feb 09 2020 at 00:41, on Zulip):

@Wesley Wiser sorry, got distracted, but I just pushed a commit which should solve the ICE you hit

eddyb (Feb 09 2020 at 00:41, on Zulip):

/me doesn't actually know how to test it reliably

Wesley Wiser (Feb 09 2020 at 01:02, on Zulip):

@eddyb Sorry, missed the notification.

Wesley Wiser (Feb 09 2020 at 01:02, on Zulip):

I'll test it now

eddyb (Feb 09 2020 at 01:02, on Zulip):

nah your timing is great :D

eddyb (Feb 09 2020 at 01:05, on Zulip):

I'm now testing the MIR-inliner-aware #[track_caller] codegen

Wesley Wiser (Feb 09 2020 at 01:11, on Zulip):

@eddyb Stage 1 builds successfully!

eddyb (Feb 09 2020 at 01:11, on Zulip):

:D

eddyb (Feb 09 2020 at 01:12, on Zulip):

@Wesley Wiser did you see this btw? https://github.com/rust-lang/rust/pull/67662#discussion_r376725183

Wesley Wiser (Feb 09 2020 at 01:12, on Zulip):

Yeah but I haven't had a chance to circle back to it yet

eddyb (Feb 09 2020 at 01:43, on Zulip):

lol had a bit of an "opposite day" moment, if all the inlined scopes were #[track_caller], I was using the MIR Call span. but that's innermost, like, in the definition of Location::caller, and I have to find the outermost inlined callsite instead

eddyb (Feb 09 2020 at 01:45, on Zulip):

huh, const-caller-location.rs will force me to implement the same loop for miri, this is a pleasant surprise

Wesley Wiser (Feb 09 2020 at 02:12, on Zulip):

A full bootstrap completes successfully and only the "expected" ui tests fail

eddyb (Feb 09 2020 at 02:13, on Zulip):

you mean the track_caller stuff?

Wesley Wiser (Feb 09 2020 at 02:13, on Zulip):

The latest changes in #68965

eddyb (Feb 09 2020 at 02:14, on Zulip):

well they ain't gonna fail now :P

Wesley Wiser (Feb 09 2020 at 02:14, on Zulip):

oh sorry

eddyb (Feb 09 2020 at 02:14, on Zulip):

just pushed codegen+miri support for inlined #[track_caller]

Wesley Wiser (Feb 09 2020 at 02:14, on Zulip):

There's a small number of ui tests that fail even on master if you enable the inliner pass

eddyb (Feb 09 2020 at 02:15, on Zulip):

aaah I see

eddyb (Feb 09 2020 at 02:15, on Zulip):

OOOOH so that's what you do. I feel dumb now. okay that makes this easier to test heh

Wesley Wiser (Feb 09 2020 at 02:15, on Zulip):

Which is just to say, your PR looks good to me

eddyb (Feb 09 2020 at 02:15, on Zulip):

are they like, backtraces? because that means debuginfo is next on the menu :P

Wesley Wiser (Feb 09 2020 at 02:16, on Zulip):

Lol, yeah it's literally just:

diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs
index 1a057500dae..26da57e41f1 100644
--- a/src/librustc_mir/transform/inline.rs
+++ b/src/librustc_mir/transform/inline.rs
@@ -37,7 +37,7 @@ struct CallSite<'tcx> {

 impl<'tcx> MirPass<'tcx> for Inline {
     fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) {
-        if tcx.sess.opts.debugging_opts.mir_opt_level >= 2 {
+        if tcx.sess.opts.debugging_opts.mir_opt_level >= 1 {
             Inliner { tcx, source }.run_pass(body);
         }
     }
Wesley Wiser (Feb 09 2020 at 02:16, on Zulip):

I know at least one of them is

eddyb (Feb 09 2020 at 02:20, on Zulip):

I can play with that while the try build/perf run is going

Wesley Wiser (Feb 09 2020 at 02:20, on Zulip):
failures:
    [ui] ui/async-await/async-closure.rs
    [ui] ui/backtrace-debuginfo.rs
    [ui] ui/issues/issue-22638.rs
    [ui] ui/type_length_limit.rs

test result: FAILED. 9565 passed; 4 failed; 50 ignored; 0 measured; 0 filtered out
eddyb (Feb 09 2020 at 02:20, on Zulip):

ugh the "type length" limit is such a silly thing

eddyb (Feb 09 2020 at 02:20, on Zulip):

I don't even know if it protects us from anything real anymore

Wesley Wiser (Feb 09 2020 at 02:21, on Zulip):

It looks like it's complaining because the test is passing now?

eddyb (Feb 09 2020 at 02:21, on Zulip):

lmao

Wesley Wiser (Feb 09 2020 at 02:21, on Zulip):
---- [ui] ui/type_length_limit.rs stdout ----

error: ui test compiled successfully!
status: exit code: 0
command: "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/home/wesley/code/rust/rust3/src/test/ui/type_length_limit.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "--out-dir" "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/ui/type_length_limit" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/ui/type_length_limit/auxiliary" "-A" "unused"
eddyb (Feb 09 2020 at 02:21, on Zulip):

needs more #[inline(never)]

eddyb (Feb 09 2020 at 02:22, on Zulip):

feel free to play around with the non-debuginfo ones, I'm also seeing codegen-units tests also failing because of too much inlining :P

Wesley Wiser (Feb 09 2020 at 02:23, on Zulip):

Could be yeah. I haven't looked at the rest of the test suite yet

eddyb (Feb 09 2020 at 02:24, on Zulip):

I mean, if it's not a debuginfo/backtrace test, it likely wants some #[inline(never)]

Wesley Wiser (Feb 09 2020 at 02:26, on Zulip):

Is type_length_limit enforced at codegen time or something?

eddyb (Feb 09 2020 at 02:26, on Zulip):

sort of. it's a silly thing

eddyb (Feb 09 2020 at 02:26, on Zulip):

we should just remove it

eddyb (Feb 09 2020 at 02:27, on Zulip):

I think it made more sense before we stopped giving LLVM things names by default

eddyb (Feb 09 2020 at 02:27, on Zulip):

there are a bunch of issues around it too, it's not DAG-like enough and so has false positives

eddyb (Feb 09 2020 at 11:07, on Zulip):

@Wesley Wiser love to write a loop and worry about it being a measurable slowdown and be right

eddyb (Feb 09 2020 at 18:22, on Zulip):

that took me far too long, I guess this is what I get for tiring myself out yesterday

eddyb (Feb 10 2020 at 01:27, on Zulip):

@bjorn3 @Wesley Wiser okay I realized too late, but to avoid things getting too ugly and confusing, I'll have to move some code from cg_llvm to cg_ssa. I'll have to finish this tomorrow, and I'll try to keep the diff small

eddyb (Feb 10 2020 at 01:27, on Zulip):

overall I'm pretty confident I can pull it off, it's just a matter of passing all the right data around

eddyb (Feb 10 2020 at 01:27, on Zulip):

(this is for inlining debuginfo)

eddyb (Feb 10 2020 at 18:12, on Zulip):

@Wesley Wiser split the Instance::resolve fix into https://github.com/rust-lang/rust/pull/69036

Wesley Wiser (Feb 10 2020 at 18:14, on Zulip):

I'll review tonight after I can play with it a bit

eddyb (Feb 10 2020 at 18:16, on Zulip):

frankly idk who should review, but it's basically being conservative, and codegen can't hit that case today so I don't think it cause any issues even if it's stricter than it needs to be

Wesley Wiser (Feb 10 2020 at 18:23, on Zulip):

We can get somebody else to review as well, that's fine :)

eddyb (Feb 10 2020 at 18:28, on Zulip):

I guess what I'm worried about is @nikomatsakis finding out about this after the fact or something, and having objections

Wesley Wiser (Feb 10 2020 at 19:11, on Zulip):

We could hold off merging until after the compiler team triage meeting. Nominating or bringing it up as an announcement would at least give @nikomatsakis and others a heads up that they might want to take a look.

eddyb (Feb 10 2020 at 19:11, on Zulip):

feel free to do it shrug

eddyb (Feb 10 2020 at 22:34, on Zulip):

hmm I don't think I need to move anything, just expose DILocation properly

eddyb (Feb 10 2020 at 22:49, on Zulip):

okay I did find a reason but it's not really necessary I don't think?

eddyb (Feb 11 2020 at 00:01, on Zulip):

@Wesley Wiser first test run with the inliner enabled and debuginfo support, I'm hyped :D

eddyb (Feb 11 2020 at 00:01, on Zulip):

it was pretty easy once all the pieces were in place, but reducing the refactors needed broke my brain yesterday

Wesley Wiser (Feb 11 2020 at 00:02, on Zulip):

Woohoo! :tada:

Wesley Wiser (Feb 11 2020 at 00:03, on Zulip):

Here's hoping for lots of green on perf.rlo

eddyb (Feb 11 2020 at 00:06, on Zulip):

@Wesley Wiser backtrace test passed!!!

eddyb (Feb 11 2020 at 00:06, on Zulip):

all the other mostly unrelated nonsense is still there ofc, I think I'll let you handle it and try to go back to whatever I was doing lol

Wesley Wiser (Feb 11 2020 at 00:06, on Zulip):

Haha

Wesley Wiser (Feb 11 2020 at 00:06, on Zulip):

Oh, is there a way to see what async closures desugar to?

eddyb (Feb 11 2020 at 00:07, on Zulip):

I have a few local FIXMEs for speeding up the inliner, I guess I can do them tomorrow but now I just want to go home and sleep

eddyb (Feb 11 2020 at 00:07, on Zulip):

@Wesley Wiser --unpretty=hir, maybe?

Wesley Wiser (Feb 11 2020 at 00:07, on Zulip):

Ah ok

eddyb (Feb 11 2020 at 00:07, on Zulip):

if it still even exists anymore

Wesley Wiser (Feb 11 2020 at 00:07, on Zulip):

Go sleep :)

eddyb (Feb 11 2020 at 00:09, on Zulip):

(the FIXMEs I have are about the BasicBlock, SourceScope and Local mappings - the latter two use an IndexVec<T, T> but it should really be a single e.g. first_inlined_local: Local, perhaps inlined_locals: RangeFrom<Local> for semantic clarity, and then mapping is just addition)

Wesley Wiser (Feb 11 2020 at 00:10, on Zulip):

and then mapping is just addition

Oh excellent! That's awesome

eddyb (Feb 11 2020 at 00:11, on Zulip):

we already do it for blocks :P

eddyb (Feb 11 2020 at 00:11, on Zulip):

just very haphazardly

Wesley Wiser (Feb 11 2020 at 00:11, on Zulip):

Yeah

eddyb (Feb 11 2020 at 00:24, on Zulip):

@Wesley Wiser pushed, knock yourself out :D

eddyb (Feb 12 2020 at 12:17, on Zulip):

@bjorn3 do you happen to know if there is a way to compact DWARF units together, kind of like an LTO thing, but cheaper because it's DWARF-only?

eddyb (Feb 12 2020 at 12:17, on Zulip):

like maybe some tool or something

eddyb (Feb 12 2020 at 12:18, on Zulip):

I don't know who to ask lol, this debuginfo journey led me to https://github.com/rust-lang/rust/pull/69080 and it feels like there's still a lot more to cut down from it

bjorn3 (Feb 12 2020 at 12:42, on Zulip):

You may want to read section 3.1 of the DWARF5 spec. It describes DW_TAG_partial_unit and DW_TAG_type_unit which are as I understad it are used to share debuginfo between multiple compilation units.

bjorn3 (Feb 12 2020 at 12:46, on Zulip):

They are introduced in DWARF5, with a similar thing (.debug_types) introduced in DWARF4 and removed again in DWARF5.

eddyb (Feb 12 2020 at 12:51, on Zulip):

we generate .debug_types heh

bjorn3 (Feb 12 2020 at 12:55, on Zulip):

I thought we targeted DWARF3 for MacOS.

eddyb (Feb 12 2020 at 12:55, on Zulip):

I mean, I'm not on macOS, but I did see that in the source code last night by happenstance and it's gated on the target so it only affects macOS builds

eddyb (Feb 12 2020 at 12:58, on Zulip):

@bjorn3 so the problem IMO is that there's no step where debuginfo from different codegen units is combined together in any way, so even if "partial unit" exists as a concept, there's nothing creating it. am I confused here? all I see in the llvm-dwarfdump output for librustc_driver-*.so is a gazillion 5267 instances of DW_TAG_compile_unit

eddyb (Feb 12 2020 at 12:59, on Zulip):

arguably a smart enough linker could take those 5267 instances of DW_TAG_compile_unit and make a single DW_TAG_compile_unit, right?

bjorn3 (Feb 12 2020 at 13:00, on Zulip):

I think the purpose of partial units is that you emit common types once, and then reference them from compilation units; avoiding duplicates at the codegen level, instead of linking level.

bjorn3 (Feb 12 2020 at 13:01, on Zulip):

arguably a smart enough linker could take those 5267 instances of DW_TAG_compile_unit and make a single DW_TAG_compile_unit, right?

On non macOS systems the linker doesn't know anything about debuginfo. DWARF is made to have concatable sections for this reason.

eddyb (Feb 12 2020 at 13:04, on Zulip):

ah but in this case I got rid of the types, so the only duplication is from things like function names, I think?

eddyb (Feb 12 2020 at 13:04, on Zulip):

and file names

eddyb (Feb 12 2020 at 13:05, on Zulip):

On non macOS systems the linker doesn't know anything about debuginfo.

right, I mean a hypothetical linker. or some other tool

bjorn3 (Feb 12 2020 at 13:35, on Zulip):

ah but in this case I got rid of the types, so the only duplication is from things like function names, I think?

I think so, strings and DW_TAG_subprocedure and lineinfo for functions that are instantiated multiple times are duplicated. Though in case of DW_TAG_subprocedure and lineinfo the exact contents will differ, as they refer to a different instantiation of the function. I don't think any otber things are duplicated.

Last update: Feb 25 2020 at 03:25UTC