Message ID | 1471029210-47845-1-git-send-email-dmalcolm@redhat.com |
---|---|
State | New |
Headers | show |
On 08/12/2016 01:13 PM, David Malcolm wrote: > On Thu, 2016-08-11 at 20:00 -0600, Sandra Loosemore wrote: >> On 08/11/2016 02:34 PM, David Malcolm wrote: >>> I sometimes find myself scouring assembler output from the compiler >>> and trying to figure out which instructions correspond to which >>> lines of source code; I believe this is a common activity for some >>> end-users. >>> >>> The following patch adds a new -fasm-show-source option, which >>> emits comments into the generated asm showing the pertinent >>> line of source code, whenever it changes. It uses the same logic >>> as debug_hooks->source_line for tracking this (for handling >>> line-based breakpoints). >>> >>> An example can be seen in the invoke.texi part of the patch. As >>> noted there, it's aimed at end-users, rather than gcc developers. >>> The example shows a relatively short function; the option is >>> likely to be much more useful for longer functions. >>> >>> I think it would further improve usability if this option were >>> enabled >>> by default when the final output is .s (either via -S, or by "-o >>> foo.s"). >>> Ideas on how to implement that (in the driver) would be welcome - I >>> started looking at the spec-handling code, but thought I'd post the >>> idea here first, before diving in too deeply. >>> >>> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; adds >>> 2 PASS results to gcc.sum. >>> >>> Thoughts? OK for trunk as-is? >> >> Why not extend the existing -fverbose-asm to do this? E.g. >> -fverbose-asm=source, or something like that. Otherwise you need to >> cross-reference the documentation for the two options and explain how >> they interact (or don't interact, as the case may be). >> >> -Sandra > > Thanks. > > With the patch as-is, if I pass both -fverbose-asm and > -fasm-show-source, I get the following: > > # test.c:7: int total = 0; > xorl %eax, %eax # <retval> > # test.c:9: for (i = 0; i < n; i++) > xorl %edx, %edx # i > .L2: > # test.c:9: for (i = 0; i < n; i++) > cmpl %edi, %edx # n, i > jge .L5 #, > # test.c:10: total += i * i; > movl %edx, %ecx # i, tmp92 > imull %edx, %ecx # i, tmp92 > # test.c:9: for (i = 0; i < n; i++) > incl %edx # i > # test.c:10: total += i * i; > addl %ecx, %eax # tmp92, <retval> > jmp .L2 # > .L5: > # test.c:13: } > ret > .cfi_endproc > > I find the above pleasing, as it shows both the source, and the > variable names associated with the asm insn arguments. The source > line information works well with jump-to-source, in Emacs, at least. > > -fverbose-asm also adds some metadata to the top of the dump, that I > wasn't interested in from a see-the-source point of view, but I can live > with that. > > Currently -fverbose-asm is documented in common.opt as: > "Add extra commentary to assembler output" > > and in invoke.texi as: > @opindex fverbose-asm > Put extra commentary information in the generated assembly code to > make it more readable. This option is generally only of use to those > who actually need to read the generated assembly code (perhaps while > debugging the compiler itself). > > @option{-fno-verbose-asm}, the default, causes the > extra information to be omitted and is useful when comparing two > assembler files. > > Given that the precise output format for -fverbose-asm isn't > documented, and it already can be thought of as our option for > "make the asm readable please", my preference would be to extend it > to print the source lines, without adding any "=source" > complications: if the user was interested in the relationship of ins > arguments to source expressions, they're likely also interested in > seeing the corresponding source lines. > > Following is a rewritten version of the patch that does this, adding > a description of the output to invoke.texi (along with a caveat that > one should not try to parse the comments). > > OK for trunk if it survives bootstrap®ression testing? > > As mentioned before, I'd also like to make this more discoverable, > by automatically adding the option in the driver if the user has > specified asm output (via -S or via "-o something.s") - is this > latter idea also reasonable? > > Dave > > gcc/ChangeLog: > * doc/invoke.texi (fverbose-asm): Note that source code lines > are emitted, and provide an example. > * final.c (asm_show_source): New function. > (final_scan_insn): Call asm_show_source. > > gcc/testsuite/ChangeLog: > * gcc.dg/verbose-asm-2.c: New test case. I don't think we have any kind of requirement to maintain a format for -fverbose-asm. So I think adding the new stuff under the -fverbose-asm flag is the way to go. Whether or not to add it to -S or not is a distinct question and I don't think the answer is as clear cut... OK on the updated patch. jeff
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 22001f9..f2f8931 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -11414,6 +11414,89 @@ debugging the compiler itself). extra information to be omitted and is useful when comparing two assembler files. +The added comments include: + +@itemize @bullet + +@item +information on the compiler version and command-line options, + +@item +the source code lines associated with the assembly instructions, +in the form FILENAME:LINENUMBER:CONTENT OF LINE, + +@item +hints on which high-level expressions correspond to +the various assembly instruction operands. + +@end itemize + +For example, given this C source file: + +@smallexample +int test (int n) +@{ + int i; + int total = 0; + + for (i = 0; i < n; i++) + total += i * i; + + return total; +@} +@end smallexample + +compiling to (x86_64) assembly via @option{-S} and emitting the result +direct to stdout via @option{-o} @option{-} + +@smallexample +gcc -S test.c -fverbose-asm -Os -o - +@end smallexample + +gives output similar to this: + +@smallexample + .file "test.c" +# GNU C11 (GCC) version 7.0.0 20160809 (experimental) (x86_64-pc-linux-gnu) + [...snip...] +# options passed: + [...snip...] + + .text + .globl test + .type test, @@function +test: +.LFB0: + .cfi_startproc +# test.c:4: int total = 0; + xorl %eax, %eax # <retval> +# test.c:6: for (i = 0; i < n; i++) + xorl %edx, %edx # i +.L2: +# test.c:6: for (i = 0; i < n; i++) + cmpl %edi, %edx # n, i + jge .L5 #, +# test.c:7: total += i * i; + movl %edx, %ecx # i, tmp92 + imull %edx, %ecx # i, tmp92 +# test.c:6: for (i = 0; i < n; i++) + incl %edx # i +# test.c:7: total += i * i; + addl %ecx, %eax # tmp92, <retval> + jmp .L2 # +.L5: +# test.c:10: @} + ret + .cfi_endproc +.LFE0: + .size test, .-test + .ident "GCC: (GNU) 7.0.0 20160809 (experimental)" + .section .note.GNU-stack,"",@@progbits +@end smallexample + +The comments are intended for humans rather than machines and hence the +precise format of the comments is subject to change. + @item -frecord-gcc-switches @opindex frecord-gcc-switches This switch causes the command line used to invoke the diff --git a/gcc/final.c b/gcc/final.c index 5b04311..eccc3d8 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -2140,6 +2140,26 @@ call_from_call_insn (rtx_call_insn *insn) return x; } +/* Print a comment into the asm showing FILENAME, LINENUM, and the + corresponding source line, if available. */ + +static void +asm_show_source (const char *filename, int linenum) +{ + if (!filename) + return; + + int line_size; + const char *line = location_get_source_line (filename, linenum, &line_size); + if (!line) + return; + + fprintf (asm_out_file, "%s %s:%i: ", ASM_COMMENT_START, filename, linenum); + /* "line" is not 0-terminated, so we must use line_size. */ + fwrite (line, 1, line_size, asm_out_file); + fputc ('\n', asm_out_file); +} + /* The final scan for one insn, INSN. Args are same as in `final', except that INSN is the insn being scanned. @@ -2563,8 +2583,12 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, note in a row. */ if (!DECL_IGNORED_P (current_function_decl) && notice_source_line (insn, &is_stmt)) - (*debug_hooks->source_line) (last_linenum, last_filename, - last_discriminator, is_stmt); + { + if (flag_verbose_asm) + asm_show_source (last_filename, last_linenum); + (*debug_hooks->source_line) (last_linenum, last_filename, + last_discriminator, is_stmt); + } if (GET_CODE (body) == PARALLEL && GET_CODE (XVECEXP (body, 0, 0)) == ASM_INPUT) diff --git a/gcc/testsuite/gcc.dg/verbose-asm-2.c b/gcc/testsuite/gcc.dg/verbose-asm-2.c new file mode 100644 index 0000000..747bff1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/verbose-asm-2.c @@ -0,0 +1,15 @@ +/* Ensure that the -fverbose-asm leads to source code information in the generated asm. */ +/* { dg-options "-fverbose-asm" } */ + +int test (int n) +{ + int i; + int total = 0; + + for (i = 0; i < n; i++) + total += i * i; + + return total; +} + +/* { dg-final { scan-assembler "total = 0" } } */