[PATCH] Add source information to -fverbose-asm

[Jeff Law]

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&regrtested 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&regression 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