code_warntype now shows unstable SSA useref values in red
#54501
Conversation
This file contains 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
|
Highlighting ssa values on the left is also useful; specially when
|
|
This is without left highlighting for comparison.
|
|
I like the left hightlighting |
|
Does this need tests ? |
|
@gbaraldi If this needs more discussion let me know. |
|
It's always good to have some tests yeah. You can probably base it on some of the tests in show.jl. Search for IR or code_typed |
fingolfin
reviewed
May 23, 2024
base/compiler/ssair/show.jl
Outdated
| @@ -44,7 +49,7 @@ function print_stmt(io::IO, idx::Int, @nospecialize(stmt), used::BitSet, maxleng | |||
| if !color && stmt isa PiNode | |||
| # when the outer context is already colored (green, for pending nodes), don't use the usual coloring printer | |||
| print(io, "π (") | |||
| show_unquoted(io, stmt.val, indent) | |||
| show_unquoted(io, stmt.val, indent, 0, (stmt.val.id in unstable_ssa) ? true : false) | |||
There was a problem hiding this comment.
Isn't ? true : false redundant here and elsewhere?
Suggested change
| show_unquoted(io, stmt.val, indent, 0, (stmt.val.id in unstable_ssa) ? true : false) | |
| show_unquoted(io, stmt.val, indent, 0, stmt.val.id in unstable_ssa) |
base/compiler/ssair/show.jl
Outdated
| print(io, "φᶜ (") | ||
| first = true | ||
| for v in stmt.values | ||
| first ? (first = false) : print(io, ", ") | ||
| show_unquoted(io, v, indent) | ||
| if v isa SSAValue | ||
| show_unquoted(io, v, indent, prec, (v in unstable_ssa) ? true : false) |
There was a problem hiding this comment.
Option suggestion, but: how about changing this and its many variants in this PR to
Suggested change
| show_unquoted(io, v, indent, prec, (v in unstable_ssa) ? true : false) | |
| show_unquoted(io, v, indent, prec, unstable_ssa) |
and then deal with it in the show_unquoted(::IO, ::SSAValue, ...) method.
That would then also make it trivial to use unstable_ssa::Union{BitSet,Nothing} = nothing everywhere to avoid creating a BitSet() if it is not needed. Dealing with the nothing case would only have to be done in one place, namely the show_unquoted method for SSAValue
There was a problem hiding this comment.
I remember I had to do that to avoid stackoverflow. But I will check again. Reasonable suggestion to invest time. Thanks.
base/compiler/ssair/show.jl
Outdated
Comment on lines
106
to
108
| show_unquoted(io′, stmt.values[i], indent, 0, (stmt.values[i].id in unstable_ssa) ? true : false) | ||
| else | ||
| show_unquoted(io′, stmt.values[i], indent) |
There was a problem hiding this comment.
Suggested change
| show_unquoted(io′, stmt.values[i], indent, 0, (stmt.values[i].id in unstable_ssa) ? true : false) | |
| else | |
| show_unquoted(io′, stmt.values[i], indent) | |
| show_unquoted(io′, val, indent, 0, (stmt.values[i].id in unstable_ssa) ? true : false) | |
| else | |
| show_unquoted(io′, val, indent) |
or even
Suggested change
| show_unquoted(io′, stmt.values[i], indent, 0, (stmt.values[i].id in unstable_ssa) ? true : false) | |
| else | |
| show_unquoted(io′, stmt.values[i], indent) | |
| show_unquoted(io′, val, indent, 0, stmt.values[i].id in unstable_ssa) | |
| else | |
| show_unquoted(io′, val, indent) |
base/compiler/ssair/show.jl
Outdated
| print(io, "π (") | ||
| show_unquoted(io, stmt.val, indent) | ||
| show_unquoted(io, stmt.val, indent, prec, (stmt.val.id in unstable_ssa) ? true : false) |
There was a problem hiding this comment.
So previously prec was ignored, now it is passed on. Is this a necessary for this patch? (I am not objecting, just wondering whether this fixes something, resp. where it might break something else)
This is partial commit which passes unstable_ssa statements to `show_ir_stmts`. Central Idea is to show/print type unstable ssa userefs in IR statements in color. To achieve that we need to modify following code segments. [*] determine unstable_ssa [*] pass unstable_ssa to `show_ir_stmts`. [ ] pass unstable_ssa to `show_ir_stmt`. [ ] pass unstable_ssa to `print_stmt` [ ] pass unstable_ssa to `show_unquoted` This PR covers second task alone
This is partial commit which passes unstable_ssa statements to `show_ir_stmts`. Central Idea is to show/print type unstable ssa userefs in IR statements in color. To achieve that we need to modify following code segments. [*] determine unstable_ssa [*] pass unstable_ssa to `show_ir_stmts`. [*] pass unstable_ssa to `show_ir_stmt`. [ ] pass unstable_ssa to `print_stmt` [ ] pass unstable_ssa to `show_unquoted` This PR covers third task alone
This is partial commit which passes unstable_ssa statements to `show_ir_stmts`. Central Idea is to show/print type unstable ssa userefs in IR statements in color. To achieve that we need to modify following code segments. [*] determine unstable_ssa [*] pass unstable_ssa to `show_ir_stmts`. [*] pass unstable_ssa to `show_ir_stmt`. [*] pass unstable_ssa to `print_stmt` [ ] pass unstable_ssa to `show_unquoted` This PR covers fourth task alone
This is partial commit which passes unstable_ssa statements to `show_ir_stmts`. Central Idea is to show/print type unstable ssa userefs in IR statements in color. To achieve that we need to modify following code segments. [*] determine unstable_ssa [*] pass unstable_ssa to `show_ir_stmts`. [*] pass unstable_ssa to `show_ir_stmt`. [*] pass unstable_ssa to `print_stmt` [*] pass unstable_ssa to `show_unquoted` This PR covers finished basic pipeline. Other ssa values appearing in nodes like `return` etc can be easily provided now.
ops like `x + y` are covered.
pass unstable_ssa to show_unquoted This is partial commit which passes unstable_ssa statements to `show_ir_stmts`. Central Idea is to show/print type unstable ssa userefs in IR statements in color. To achieve that we need to modify following code segments. [*] determine unstable_ssa [*] pass unstable_ssa to `show_ir_stmts`. [*] pass unstable_ssa to `show_ir_stmt`. [*] pass unstable_ssa to `print_stmt` [*] pass unstable_ssa to `show_unquoted` This PR covers finished basic pipeline. Other ssa values appearing in nodes like `return` etc can be easily provided now. unstable_ssa as optional in `show_unquoted` unstable ssa should be highlighted only when appropriate
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.
Addresses #54028
reraising previous PR after suggested code changes #54441
