rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

llvm.dbg.value should be used instead of llvm.dbg.declare wherever possible. #68817

Open eddyb opened 4 years ago

eddyb commented 4 years ago

(I'm opening this issue because I can't find a pre-existing one)

llvm.dbg.declare describes the location in memory of a debuginfo variable, whereas llvm.dbg.value describes the value itself directly. Today we only use the former, which means that in debug mode, all named variables are placed on the stack, even if they're simple scalars (e.g. integers).

Ideally, we'd use llvm.dbg.value in the same situations where we skip creating a stack slot (e.g. LLVM alloca), i.e. for scalars, vectors and pairs of scalars.


However, this is much harder than I expected, mostly due to LLVM being too eager to throw away llvm.dbg.value if computing the value can be avoided (i.e. it's not used by anything else).

I think llvm.dbg.declare only fares better because at opt-level=0, the lack of SROA means allocas are kept around pretty much completely intact.

FastISel (used at opt-level=0) throws away any non-trivial llvm.dbg.value, but disabling it with -C llvm-args=-fast-isel=0 only helps some simple cases (such as a reference to another stack variable - IMO FastISel should be improved to handle those, I don't see why it couldn't).

In general, it looks like Instruction Selection ("ISel") in LLVM ignores llvm.dbg.values while building all of the machine instructions for a BB, and only afterwards does it come back to the llvm.dbg.values and lower them to DBG_VALUE, using the value only if it already exists (i.e. something else needs that same value, at runtime), and otherwise it leads to <optimized out> vars.

Maybe the upcoming GlobalISel approach handles llvm.dbg.value better, but I would need to target AArch64 to even try it out, from what I hear (I might still do it out of curiosity).


While investigating this I also came across https://github.com/rust-lang/rust/pull/8855#discussion_r374392128 - but setting AlwaysPreserve to true only keeps the debuginfo variable around, it doesn't help at all with keeping any value alive (so you end up with <optimized out> in the debugger, instead of the variable missing).

I haven't seen anything that would make llvm.dbg.value keep the value alive so it's guaranteed to be available to the debugger, but if there is such a thing, it's probably the way to go, if we want to avoid relying on the stack so much.

cc @nikomatsakis @michaelwoerister @hanna-kruppe

eddyb commented 4 years ago

Here's a more detailed example of how even simple cases break down: https://github.com/rust-lang/rust/blob/8417d68de5e063426ab6bb7f383df6117d1beeed/src/test/debuginfo/borrowed-struct.rs#L77-L79 That snippet results in this (LLVM) Machine IR (with -fast-isel=0):

DBG_VALUE %stack.0, $noreg, !"stack_val_ref",
    !DIExpression(),
    debug-location !31; src/test/debuginfo/borrowed-struct.rs:77:8 line no:77

DBG_VALUE $noreg, $noreg, !"stack_val_interior_ref_1",
     !DIExpression(),
     debug-location !33; src/test/debuginfo/borrowed-struct.rs:78:8 line no:78

DBG_VALUE $noreg, $noreg, !"stack_val_interior_ref_2",
     !DIExpression(DW_OP_plus_uconst, 8, DW_OP_stack_value),
     debug-location !35; src/test/debuginfo/borrowed-struct.rs:79:8 line no:79

(NB: $noreg is effectively undef, and %stack.0 is stack_val's stack slot)

stack_val_interior_ref_1 should be the same value as stack_val_ref, but LLVM somehow manages to lose track of %stack.0, even though it would need no extra machine instructions, just %stack.0 instead of $noreg for the first DBG_VALUE operand.

LLVM even went through the trouble of turning an offset of 8 bytes (to get to the y field) into !DIExpression(DW_OP_plus_uconst, 8, DW_OP_stack_value) for stack_val_interior_ref_2, but it's all for naught without %stack.0 as the base of that offset.

The result is that stack_val_interior_ref_{1,2} have no DW_AT_location in DWARF (dumped by llvm-dwarfdump), and in a debugger they would show up as <optimized out>, and/or cause errors.

eddyb commented 4 years ago

I tried an experiment, namely using inline assembly to keep the input to llvm.dbg.value alive.

However, what seems to happen is that a register is used to hold the value, but as soon as that register is needed for anything else (e.g. the return value of a call), the value is gone, and it's not saved anywhere.

Relying on the stack with llvm.dbg.declare, as we do today, appears to be the only foolproof way of keeping arbitrary values around, sadly (kind of obvious in retrospect).

There are some exceptions, such as constants, pointers to globals or to other variables on the stack, which should be encodable in DWARF (and indeed LLVM does so, but it's somewhat unreliable).

I suspect I can emit the inline assembly around the point of the StorageDead or so, but I've spent enough time on this already, and it would be somewhat tricky to do so.

I've opened #68902 with the branch I experimented on, to get some perf numbers, but it's only as a rough estimate and unlikely to lead to anything.

eddyb commented 4 years ago

There are some exceptions, such as constants, pointers to globals or to other variables on the stack, which should be encodable in DWARF (and indeed LLVM does so, but it's somewhat unreliable).

Long term we might want to handle some of these ourselves, up to and including encoding them into mir::VarDebugInfo (constants would be easy to justify, no need to use a MIR local).

eddyb commented 4 years ago

One way we might be able to get some of the wins from llvm.dbg.value is, for cases where we wouldn't use the stack if we could use llvm.dbg.value, to keep the stack usage but instead of loading from the stack every time the value is used, refer to the original value that was stored on the stack.

So instead of:

%x = alloca i32
llvm.dbg.declare %x, !DIVariable("x")
; ...
%3 = ...
store %3, %x
; ...
%10 = load %x
%11 = add %10, 42
; ...
%20 = load %x
%21 = add %20, 42
; ...
%30 = load %x
%31 = add %30, 42
; ...

we'd be able to get this:

%x = alloca i32
llvm.dbg.declare %x, !DIVariable("x")
; ...
%3 = ...
store %3, %x
%4 = add %3, 42
; ...

(more realistically, this would be helpful for e.g. accessing a field through a reference)

eddyb commented 4 years ago

@hanna-kruppe pointed me to https://lists.llvm.org/pipermail/llvm-dev/2020-February/139376.html, which looks really promising (even if it doesn't solve everything, it's a step in the right direction).

Also, something in there that peaked my attention was:

Of these categories, the first 3 will become salvageable after this work is implemented, and Select Insts will also be salvageable with some follow-up work to enable conditional branching in complex expressions.

This is exciting, because the lack of conditional branching makes fragmenting enums variables that have debuginfo attached to them impossible in #48300. (Or more accurately, the most straightforward implementation of fragmented enum debuginfo would branch on the discriminant - I have an alternative that involves crafting type debuginfo in a way that variants don't overlap from the perspective of the debugger, but that's harder and more dubious)

programmerjake commented 2 months ago

LLVM is redoing debug info so hopefully the issues go away with the new IR representation for debug info: https://llvm.org/docs/RemoveDIsDebugInfo.html