rustc_codegen_ssa: only "spill" SSA-like values to the stack for debuginfo. #68961
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit e34b66075282e9a3d29da0a15b83a1744b93ff3d with merge 9366841f40c1b378839d842909d2708a1e361923... |
|
☀️ Try build successful - checks-azure |
|
@rust-timer build 9366841f40c1b378839d842909d2708a1e361923 |
|
Queued 9366841f40c1b378839d842909d2708a1e361923 with parent 85ffd44, future comparison URL. |
| bx.set_var_name(spill_slot.llval, &(name + ".dbg.spill")); | ||
| } | ||
| operand.val.store(bx, spill_slot); | ||
| spill_slot |
There was a problem hiding this comment.
Is this code triggered anywhere? Do we have a test for related behaviours? In particular that the value and name as seen by debugger is right and remains in scope for as long as necessary (i.e. the problem that appears to be preventing our use to llvm-dbg.value)
Might be worth adding a couple now if they don’t yet exist so that we catch issues when we move to llvm.dbg.value.
There was a problem hiding this comment.
Most of the debuginfo tests fail with llvm.dbg.value, which is how I found all the issues with it.
A common pattern with debuginfo tests is that they breakpoint at the end of a function, so all the variables are as close to the end of their scope as possible, stressing even llvm.dbg.value+inline asm (which only gives you the value if you breakpoint earlier).
|
overall r=me. |
|
Finished benchmarking try commit 9366841f40c1b378839d842909d2708a1e361923, comparison URL. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bors
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
bors
added
S-waiting-on-author
|
@bors r=nagisa |
|
📌 Commit 1a8f5ef has been approved by |
bors
added
S-waiting-on-bors
|
@bors rollup=never (suggested by @hanna-kruppe in case perf results change, but I also want to run debug-mode |
|
@bors p=1 |
rustc_codegen_ssa: only "spill" SSA-like values to the stack for debuginfo. This is an implementation of the idea described in #68817 (comment). In short, instead of debuginfo forcing otherwise-SSA-like MIR locals into `alloca`s, and requiring a `load` for each use (or two, for scalar pairs), the `alloca` is now *only* used for attaching debuginfo with `llvm.dbg.declare`: the `OperandRef` is stored to the `alloca`, but *never loaded* from it. Outside of `debug_introduce_local`, nothing cares about the debuginfo-only `alloca`, and instead works with `OperandRef` the same as MIR locals without debuginfo before this PR. This should have some of the benefits of `llvm.dbg.value`, while working today. cc @nagisa @nikomatsakis
|
☀️ Test successful - checks-azure |
|
FWIW, this is the comparison for the actual merged build (dc4242d...b6690a8). Doesn't really look any different from the PR run, which is nice. |
This is an implementation of the idea described in #68817 (comment).
In short, instead of debuginfo forcing otherwise-SSA-like MIR locals into
allocas, and requiring aloadfor each use (or two, for scalar pairs), theallocais now only used for attaching debuginfo withllvm.dbg.declare: theOperandRefis stored to thealloca, but never loaded from it.Outside of
debug_introduce_local, nothing cares about the debuginfo-onlyalloca, and instead works withOperandRefthe same as MIR locals without debuginfo before this PR.This should have some of the benefits of
llvm.dbg.value, while working today.cc @nagisa @nikomatsakis