print: name and reuse inlined call site debuginfo "scopes".#18
Merged
Conversation
Firestar99
approved these changes
Jul 1, 2025
Member
Firestar99
left a comment
There was a problem hiding this comment.
I feel like as this is just debug print, and it massively helps you, we should just merge it without me explicitly reviewing every single part of it
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jul 1, 2025
…dren. (#19) Follow-up to: - #18 Overlooked an edge case (`scope.parent()` being in the `claimed` map, but at a later position than `scope` itself, i.e. part of the same "claim" batch) in the original PR, now the order is properly "outside-in": |Before|After| |-|-| |||
This was referenced Jul 7, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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_defalways 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 inConstattributes etc.).spirtline count
.spirt.htmlfile size
comparison
(EDIT: I've sadly had to force widths via explicit
<img>tags because GitHub started applyingoverflow-wrap: anywhereto 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.htmlafter 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::fmtinternals, 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.)