Re: [PATCH] final: Improve output for -dp and -fverbose-asm

[Martin Sebor <msebor@gmail.com>]

On 11/29/2017 04:13 PM, Segher Boessenkool wrote:
> This improves the assembler output (for -dp and -fverbose-asm) in
> several ways.  It always prints the insn_cost.  It does not print
> "[length = NN]" but "[c=NN l=NN]", to save space.  It does not add one
> to the instruction alternative number (everything else starts counting
> those at 0, too).  And finally, it tries to keep things lined up in
> columns a bit better.
>
> Tested on powerpc64-linux {-m32,-m64}; is this okay for trunk?

[c=NN l=NN] will be meaningless to those without some deeper
insight into/experience with what's actually being printed.
It might as well say [NN NN].  But such extreme terseness would
seem to run counter to the documented purpose of -fverbose-asm
to "Put extra commentary information in the generated assembly
code to make it more readable."

Looking further in the manual I don't see the [length=NN] bit
mentioned (nor does your patch add a mention of it) so while
the meaning of [length=NN] isn't exactly obvious, using [l=NN]
instead certainly won't make it easier to read.  Does the manual
need updating?

Martin

>
>
> Segher
>
>
> 2017-11-29  Segher Boessenkool  <segher@kernel.crashing.org>
>
> 	* final.c (output_asm_name): Print insn_cost.  Shorten output.  Print
> 	which_alternative instead of which_alternative + 1.
> 	(output_asm_insn): Print an extra tab if the template is short.
>
> ---
>  gcc/final.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/final.c b/gcc/final.c
> index fdae241..afb6906 100644
> --- a/gcc/final.c
> +++ b/gcc/final.c
> @@ -3469,16 +3469,20 @@ output_asm_name (void)
>  {
>    if (debug_insn)
>      {
> -      int num = INSN_CODE (debug_insn);
> -      fprintf (asm_out_file, "\t%s %d\t%s",
> -	       ASM_COMMENT_START, INSN_UID (debug_insn),
> -	       insn_data[num].name);
> -      if (insn_data[num].n_alternatives > 1)
> -	fprintf (asm_out_file, "/%d", which_alternative + 1);
> +      fprintf (asm_out_file, "\t%s %d\t",
> +	       ASM_COMMENT_START, INSN_UID (debug_insn));
>
> +      fprintf (asm_out_file, "[c=%d",
> +	       insn_cost (debug_insn, optimize_insn_for_speed_p ()));
>        if (HAVE_ATTR_length)
> -	fprintf (asm_out_file, "\t[length = %d]",
> +	fprintf (asm_out_file, " l=%d",
>  		 get_attr_length (debug_insn));
> +      fprintf (asm_out_file, "]  ");
> +
> +      int num = INSN_CODE (debug_insn);
> +      fprintf (asm_out_file, "%s", insn_data[num].name);
> +      if (insn_data[num].n_alternatives > 1)
> +	fprintf (asm_out_file, "/%d", which_alternative);
>
>        /* Clear this so only the first assembler insn
>  	 of any rtl insn will get the special comment for -dp.  */
> @@ -3824,6 +3828,10 @@ output_asm_insn (const char *templ, rtx *operands)
>  	putc (c, asm_out_file);
>        }
>
> +  /* Try to keep the asm a bit more readable.  */
> +  if ((flag_verbose_asm || flag_print_asm_name) && strlen (templ) < 9)
> +    putc ('\t', asm_out_file);
> +
>    /* Write out the variable names for operands, if we know them.  */
>    if (flag_verbose_asm)
>      output_asm_operand_names (operands, oporder, ops);
>