print: name and reuse inlined call site debuginfo "scopes". #18
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses the visual (and functional) bloat caused by accurate (wrt inlining) debuginfo from Rust-GPU,
which on the motivating example, results in these changes:
(middle column obtained by making
DbgScope::maybe_print_name_as_ref_or_def
always returnNone
, which should not be considered likely output after this PR, it's only kept around as a fallback, e.g. if intra-function debuginfo ends up used inConst
attributes etc.).spirt
line count
.spirt.html
file size
comparison
(EDIT: I've sadly had to force widths via explicit
<img>
tags because GitHub started applyingoverflow-wrap: anywhere
to the entire markdown render output, which means individual letters in the text cells are competing against whole<img>
s, during table layout, and the letters end up inevitably all in one vertical line)Initially I was hoping for some kind of "stateful" comment system, and/or showing "debug scopes" using braces within the debuginfo comments, but the
// d123 = ...
notation added in this PR was an algorithmic nightmare as-is. (mainly because of multi-version support, where uses of debuginfo can easily disappear/move as nodes get simplified/wholly removed/etc. in various passes, making it hard so share common placements for such debuginfo "definitions")Also, it's been aesthetically tricky to handle the transition from "location of (inlined) call site" to "location inside (inlined) callee", and this PR ended up flipping the order, from inside-out to outside-in. (you might be able to tell between the first and second column, it used to look like a backtrace that keeps "zooming out", from leaf function to entry-point, whereas now it "zooms in", keeping the leaf-most location closest to whatever follows the debuginfo comments)
That 33.5 MiB
.spirt.html
after this PR still makes browsers a bit unhappy. (I was going to share more comparisons but even that single one took minutes per screenshot just to scroll with the Firefox screenshot tool!)(specific HTML size/complexity issues might be fixable, but it's hard to tell what browsers are struggling with most, without actually testing a tentative change)
However, it's a far cry from what it used to be, and any visual and UX improvements are welcome.
(let alone the 216 MiB file I started from, before fixing the
format_args!
"decompiler" on my Rust-GPU branch, as the output was full ofcore::fmt
internals, esp. kept alive by function pointers/vtables)Not sure how to handle reviews, but I would gladly walk someone through all of this mess if anyone cares.
(Thought, just like the other recent pretty-printer PRs, this is purely about aesthetics/ergonomics, so anything being "wrong" or "imprecise" shouldn't affect the correctness of actual SPIR-T passes etc.)