RFC: prettier IR-show for line number and inlining information #28390
Conversation
|
We already have a julia ir mode that shows the linetable in verbose mode. I really think printing all this line information by default is way too verbose. |
|
I love the assembly and llvm view. |
|
E.g. compare the amount of information that fits on one screen with the full linetable information compared to the compressed representation we have right now
I really think the full linetable cannot be the default. Sometimes it's useful, |
|
and note that the full representation we have right now shorter than the representation suggested in this PR (because it doesn't use a line to end a scope). |
|
This PR restores the v0.6 capabilities for line-info printing that regressed when the new IR PR was merged. It then also adds additional formatting marks to coordinate it with the updates to the
I strongly disagree. Most of the time, it's pretty much the only anchor I have for figuring out why this code was generated. I find that first screenshot to be basically worthless, since it fails to represent most location information or represents it ambiguously. I agree it obscures somewhat the optimization information – but that's why I don't print this more accurate metadata for IRCode (or you can also do |
Well, I cut off the linetable information in that screenshot (here it is)
When I designed that representation I tried pretty hard to make it maximally informative without taking up any extra lines. If you really, really feel that all the location information is necessary, I think the current verbose printing is better, because it at least doesn't need an extra line to close the scope and doesn't clutter up the LHS. |
It's not necessary here either. I included it to give this the same layout as the |
Sure, but I think putting the additional indicators on the LHS of the screen clutters up the control flow information, while there's plenty of space on the RHS. If you don't use lines for closing the scope, then you need some other way to indicate it, as the current verbose printing does. I really don't think consistency with the LLVM printing is all that valuable here, because that has other constraints (i.e. we can't put any formatting on content lines, because we want it to be copy-pasteable to a .ll file). |
|
I guess what I'm saying is this.
If you make those changes, I'll be ok. |
ararslan
added
the
display and printing
|
I can do all of that except 1 (already not changed) and 3 (I think we should make this a Function argument – like LLVM – not just a boolean). |
|
Timing and priorities, folks. |
|
Seriously, folks. Please do not spend time on pretty-printing when we have release-blocking bugs that need to be fixed yesterday. |
Use box-drawing characters and indentation to make the output readable more rapidly.
…_native For consistency of user experience, reduce the variance in our IR printing across formats. This also now shows inlining and line number information even if the output device might not support color, which was previously impossible (a regression since v0.6).
Here we make the observation that it's somewhat common to have chains of methods of one function that recursively handle arguments in some fashion (for example, map-tuple or +). However, since they all have the same method name, it's possible to represent these on a single line.
|
Changes requested above by Keno have been made. Additionally, I now compress recursion to take a single line (the effect of which is most apparent with tailcall-like functions such as |
|
I extremely much prefer the old printing. Please bring that back with an option or something. I can't use this at all. |
|
Yeah, I'm sorry, but this printing just looks messy. It's great if I care about the line information, but near useless if I want to actually look at the code:
It loses the compactness of the previous printing as well as the alignment of the SSA values (which was useful to quickly be able to go to an operand). We also need verbosity controls on the macros themselves. Previous printing for comparison:
I'm also not convinced it's any better than the verbose printing we had before:
Lastly, I'm unhappy about the process by which this was merged (or rather the lack thereof), but I've already mentioned so offline, so I won't repeat that here. |
|
Agree. Just merging this despite all the objections is pretty antisocial behavior. This is yet another step in an apparent project that you've undertaken for unclear reasons to make the output of all the code introspection tools so verbose and illegible that no one will ever use them again. Please stop. |
|
I'm currently looking at an issue (#29117), where the inlining information is pretty much the only part of the IR that matters, so yes, it's useful.
There's a flag for this in the code. Right now, it's set on for code_typed and code_llvm, but off for code_native. I find it helps most with code_llvm readability, but we can toggle that off for the other two.
Is the thought just that we should indent it more heavily, so that the code is more visibly left-aligned? That's something we could change to be consistent across our tooling too. |
I didn't say it was useless, I said it's near useless if I want to look at the code. There's clearly situations where you care about the line info, but I'd say in most cases you care about what the code is doing. That's why I've been requesting a simple macro flag here. |
As suggested by maleadt in the original LineNumber info PR, this uses box-drawing characters and indentation to improve readability of
code_llvm(and should trigger detection in certain terminals of line number hyperlinking). As a second step, it uses the same line number formatting across all ofcode_lowered/code_typed/code_warntype/code_llvm/code_native, for a more consistent experience.A sample example:
(note that in the terminal, there is additional usage of color to further improve visibility of various elements)
fixes #27526