reliable line numbers in code info printing #29893
Conversation
|
This seems nice, but can we leave the default non-verbose? I'm worried about new users trying to use these and getting a mess of line info thrown at them (this is already happening with code_native as implemented right now). |
|
This is an absurd bikeshed comment, but the brackets look weird to me. The closing |
vtjnash
force-pushed
the
jn/codeinfo-lineno
branch
from
c2bda2c to
8cab827
Compare
This just needs someone to adapt it to the framework here. If it had gotten done by now by someone, I was figuring that we could merge it into this PR. But we can merge that on master separately whenever it's ready. For now, I’m most interested in finally recovering the capability for this code to present information completely and correctly.
I have great confidence in users abilities—they aren’t stupid. Regardless, I expect that I'll appreciate having this information available by default in bug reports, in much the same way that I want backtraces to be overly information-laden by default. Btw, @Keno you mentioned that you'd like the bars on the right. I'm generally for that too, since otherwise the right-hand-size seems to be under-utilized, and I don't yet know what else we would need to put there. Once this general framework is merged, I'm hopeful that we can find some way to get that changed where supported. @JeffBezanson sure, done. It was a tossup for me whether it was drawing a strict grouping or some more abstract margin line design. Going with the bracket interpretation seems good. |
vtjnash
force-pushed
the
jn/codeinfo-lineno
branch
from
8cab827 to
9f72610
Compare
|
May I suggest for the purpose of getting this merged, you switch the default to non-verbose, since everybody seems to be in favor of that part of this change, and then we have a separate debate on that part of the change? I don't think coupling the two is necessary. |
|
And FWIW, I'd be fine if "non-verbose" means "none" here until we re-implement the RHS information. |
|
Can we just recommend that those folks edit their True, that doesn't really need to block merging, since I can just set my default too, so I've switched that default in my latest commit: Base.IRShow.debuginfo[:default] = Base.IRShow.debuginfo[:source] # or :none |
vtjnash
force-pushed
the
jn/codeinfo-lineno
branch
from
86ed2fa to
a0885fb
Compare
|
Should we call this |
Maybe not too visible in the diff, but the currently allowed values are explained in every doc-string for each code_* function and macro (and observations about possible future work to add more values is mentioned in the PR text above). These doc-string additions ended up somewhere near to the lower 3/4th of the github changes view. Looking at the doc-string, it could also be called
The alias |
|
Perhaps I'm missing it but I don't see My objection is that the name |
|
In prose, I write the option name without the leading colon.
Seriously, you're worried that it's a single character too long? What is it redundant with? If you're going to bike-shed the name, it'd be helpful to at least have a more concrete reason than "comments=2" is "better and less awkward". Most of the options are single words, and avoid the underscore question. But typically across Julia, we
We could permit integers as aliases, for ease of typing, but even C has had enums for ages for better readability. |
|
On this branch, if you set |
|
It works fine, but specifies something slightly different. In particular, they can independently specify a choice to filter out available debug information. We may synchronize those options in more cases in the future. But right now, master doesn't give you any choice, so this is at least an improvement. |
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.
These levels are intended to roughly correspond approximately to compiler options -g0 (none) through -g2 (including local variables). Currently we do not implement the ability to represent local variable information, so the default is to only show line number information suitable for identifying the source line of a given statement (but not the types).
Only for Julia IR, for now. Later, this should be threaded into code_llvm and enabled there also. I chose green for new nodes (like new life), and used yellow for metadata (because the strong contrast helps to hide it visually, and similar to sticky notes), and used grey for basic blocks (because they are minor information), and used cyan/gray/Base.warn_color() for type information (because that's what we've always used).
vtjnash
force-pushed
the
jn/codeinfo-lineno
branch
from
a0885fb to
4c13c65
Compare
|
Is it just me or does this option not have any effect? julia> function collatz(n)
k = 0
while n > 1
n = iseven(n) ? n >> 1 : 3n + 1
k += 1
end
return k
end
collatz (generic function with 1 method)
julia> @code_native debuginfo=:none collatz(100)
.section __TEXT,__text,regular,pure_instructions
; ┌ @ REPL[9]:2 within `collatz'
xorl %eax, %eax
; │ @ REPL[9]:3 within `collatz'
; │┌ @ operators.jl:286 within `>'
; ││┌ @ int.jl:49 within `<'
cmpq $2, %rdi
; │└└
jl L45
nopl (%rax,%rax)
; │ @ REPL[9]:4 within `collatz'
; │┌ @ int.jl:448 within `>>' @ int.jl:441
L16:
movq %rdi, %rcx
sarq %rcx
; │└
testb $1, %dil
; │┌ @ int.jl:53 within `+'
leaq 1(%rdi,%rdi,2), %rdi
; │└
cmoveq %rcx, %rdi
; │ @ REPL[9]:5 within `collatz'
; │┌ @ int.jl:53 within `+'
addq $1, %rax
; │└
; │ @ REPL[9]:3 within `collatz'
; │┌ @ operators.jl:286 within `>'
; ││┌ @ int.jl:49 within `<'
cmpq $2, %rdi
; │└└
jge L16
; │ @ REPL[9]:7 within `collatz'
L45:
retq
nop
; └ |
|
It works for |
|
|
|
Something seems wrong with the lines to the left? They are cut everywhere there are instructions.
|
|
Wasn't the intention to default to not showing all this line info? |
|
@vtjnash does not want to do that even though there does seem to be fairly broad consensus behind it. Personally, my preferred state of affairs would be having three levels of debug info:
Where |
|
@KristofferC It's intentional: LLVM really does not not want us printing on the same line (so that the output is valid code). We can hack around it as follows, but then it's not a valid file anymore, so on balance it doesn't seem to be worthwhile. diff --git a/src/disasm.cpp b/src/disasm.cpp
index 418643d568..04f6465059 100644
--- a/src/disasm.cpp
+++ b/src/disasm.cpp
@@ -806,6 +806,7 @@ static void jl_dump_asm_internal(
// start out with a raw stream, and create formatted stream from it here.
// LLVM will desctruct the formatted stream, and we keep the raw stream.
auto ustream = llvm::make_unique<formatted_raw_ostream>(rstream);
+ llvm::formatted_raw_ostream *private_ustream = ustream.get();
Streamer.reset(TheTarget->createAsmStreamer(Ctx, std::move(ustream), /*asmverbose*/true,
/*useDwarfDirectory*/ true,
IP, CE, MAB, /*ShowInst*/ false));
@@ -860,7 +861,8 @@ static void jl_dump_asm_internal(
uint64_t nextLineAddr = -1;
DILineInfoTable::iterator di_lineIter = di_lineinfo.begin();
DILineInfoTable::iterator di_lineEnd = di_lineinfo.end();
- DILineInfoPrinter dbgctx{"; ", true};
+ DILineInfoPrinter dbgctx{"", true};
+ dbgctx.SetVerbosity(debuginfo);
if (pass != 0) {
if (di_ctx && di_lineIter != di_lineEnd) {
// Set up the line info
@@ -907,6 +909,7 @@ static void jl_dump_asm_internal(
if (symbol)
Streamer->EmitLabel(symbol);
}
+ *private_ustream << dbgctx.inlining_indent("│");
MCInst Inst;
MCDisassembler::DecodeStatus S;
No. The intention of the PR was to improve formatting. If you want to change the defaults, make your own PR. |
|
I came here trying to figure out why using Suppressor
macro no_comments(ex)
quote
full_output = split((@capture_out $(esc(ex))), '\n')
without_comments = filter(s -> !startswith(s, ';'), full_output)
foreach(println, without_comments)
end
endBefore: julia> code_native(x -> sqrt(x), (Float64,); syntax=:intel)
.section __TEXT,__text,regular,pure_instructions
; ┌ @ REPL[6]:1 within `#17'
; │┌ @ math.jl:492 within `sqrt'
; ││┌ @ REPL[6]:1 within `<'
push rax
vxorps xmm1, xmm1, xmm1
vucomisd xmm1, xmm0
; ││└
ja L17
; ││ @ math.jl:493 within `sqrt'
vsqrtsd xmm0, xmm0, xmm0
; │└
pop rax
ret
; │┌ @ math.jl:492 within `sqrt'
L17:
movabs rax, offset throw_complex_domainerror
movabs rdi, 4363115776
call rax
ud2
nop dword ptr [rax]
; └└After: julia> @no_comments code_native(x -> sqrt(x), (Float64,); syntax=:intel)
.section __TEXT,__text,regular,pure_instructions
push rax
vxorps xmm1, xmm1, xmm1
vucomisd xmm1, xmm0
ja L17
vsqrtsd xmm0, xmm0, xmm0
pop rax
ret
L17:
movabs rax, offset throw_complex_domainerror
movabs rdi, 4363115776
call rax
ud2
nop dword ptr [rax]¡A blissful sight! |
|
@vtjnash: It's pretty ridiculous that we still cannot suppress all this junk in the output of |
|
This is fixed by #30223 |
| @@ -122,11 +122,11 @@ calls and expand argument types automatically: | |||
| ```julia-repl | |||
| julia> @code_llvm +(1,1) | |||
|
|
|||
| ; Function Attrs: sspreq | |||
| define i64 @"julia_+_130862"(i64, i64) #0 { | |||
| ; @ int.jl:53 within `+' | |||
There was a problem hiding this comment.
What's the reason for printing this as `+' instead of `+`? It messes up syntax highlighting sometimes when pasting output.
There was a problem hiding this comment.
I'd say just fix it to be paired backticks, which, imo looks better anyway.
fixes #27149
This implements a uniform and flexible printing of line number and inlining information across all of our compiler introspection passes (*see comparison screen shots at the bottom). The goal here is to expose all information which may be necessary to the user, but to also provide a uniform framework for letting the user augment or limit that information. As such, all
code_functions and macros now take adebuginfoargument.This PR utilizes a flexible callback-based design (loosely based on the LLVM design of the same that has proved very useful to us) so that users can create new designs which render less or more information than the default (and to also set those to the default(*), if they wish). Currently the only modes for
:debuginfothat are implemented are:source(showing only line number information) and:none(showing no debug information). Also, all provide:default(user-customizable(*)). In the future, I’m hoping users will implement new modes to suit other specific interests. Such examples, ordered by verbosity, include:Example snippets below. This function was chosen because it has unusually complicated line information, even at the source level, because of the macro. This is not true of most functions: typically unoptimized code will not contain any inlining bar delineating any statements as belonging to some foreign frame. I’m also hopeful to see if someone—maybe me—can move the bar to the right, while still preserving the accuracy that I've worked hard over the years to ensure.) Note: these were taken using the Solarized Dark theme, which sometimes shifts colors pretty far from their defaults to much more muted tones, so YMMV.
input
code_lowered
code_warntype
code_typed
code_llvm
code_native
* user-customization is handled via an open dictionary
Base.IRShow.debuginfothat maps symbol name (such as:default) to a callback annotation printer. Users can alter or add mappings in their.juliarc.jlfiles and create new packages to create novel new modes of their own!