(v2) Add a "compact" mode to print_rtx_function
diff mbox

Message ID 1479829122.7673.83.camel@redhat.com
State New
Headers show

Commit Message

David Malcolm Nov. 22, 2016, 3:38 p.m. UTC
On Tue, 2016-11-22 at 15:45 +0100, Jakub Jelinek wrote:
> On Tue, Nov 22, 2016 at 03:38:04PM +0100, Bernd Schmidt wrote:
> > On 11/22/2016 02:37 PM, Jakub Jelinek wrote:
> > > Can't it be done only if xloc.file contains any fancy characters?
> > 
> > Sure, but why? Strings generally get emitted with quotes around
> > them, I
> > don't see a good reason for filenames to be different, especially
> > if it
> > makes the output easier to parse.
> 
> Because printing common filenames matches what we emit in
> diagnostics,
> what e.g. sanitizers emit at runtime diagnostics, what we emit as
> locations
> in gimple dumps etc.

It sounds like a distinction between human-readable vs machine
-readable.

How about something like the following, which only adds the quotes if
outputting the RTL FE's input format?

Does this fix the failing tests?

Comments

Dominik Vogt Nov. 25, 2016, 4:37 p.m. UTC | #1
On Tue, Nov 22, 2016 at 10:38:42AM -0500, David Malcolm wrote:
> On Tue, 2016-11-22 at 15:45 +0100, Jakub Jelinek wrote:
> > On Tue, Nov 22, 2016 at 03:38:04PM +0100, Bernd Schmidt wrote:
> > > On 11/22/2016 02:37 PM, Jakub Jelinek wrote:
> > > > Can't it be done only if xloc.file contains any fancy characters?
> > > 
> > > Sure, but why? Strings generally get emitted with quotes around
> > > them, I
> > > don't see a good reason for filenames to be different, especially
> > > if it
> > > makes the output easier to parse.
> > 
> > Because printing common filenames matches what we emit in
> > diagnostics,
> > what e.g. sanitizers emit at runtime diagnostics, what we emit as
> > locations
> > in gimple dumps etc.
> 
> It sounds like a distinction between human-readable vs machine
> -readable.
> 
> How about something like the following, which only adds the quotes if
> outputting the RTL FE's input format?
> 
> Does this fix the failing tests?

Yep.

> --- a/gcc/print-rtl.c
> +++ b/gcc/print-rtl.c
> @@ -371,7 +371,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
>        if (INSN_HAS_LOCATION (in_insn))
>  	{
>  	  expanded_location xloc = insn_location (in_insn);
> -	  fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
> +	  if (m_compact)
> +	    fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
> +	  else
> +	    fprintf (m_outfile, " %s:%i", xloc.file, xloc.line);

Looks sensible to me.

Ciao

Dominik ^_^  ^_^
Dominik Vogt Dec. 1, 2016, 10:12 a.m. UTC | #2
On Tue, Nov 22, 2016 at 10:38:42AM -0500, David Malcolm wrote:
> On Tue, 2016-11-22 at 15:45 +0100, Jakub Jelinek wrote:
> > On Tue, Nov 22, 2016 at 03:38:04PM +0100, Bernd Schmidt wrote:
> > > On 11/22/2016 02:37 PM, Jakub Jelinek wrote:
> > > > Can't it be done only if xloc.file contains any fancy characters?
> > > 
> > > Sure, but why? Strings generally get emitted with quotes around
> > > them, I
> > > don't see a good reason for filenames to be different, especially
> > > if it
> > > makes the output easier to parse.
> > 
> > Because printing common filenames matches what we emit in
> > diagnostics,
> > what e.g. sanitizers emit at runtime diagnostics, what we emit as
> > locations
> > in gimple dumps etc.
> 
> It sounds like a distinction between human-readable vs machine
> -readable.
> 
> How about something like the following, which only adds the quotes if
> outputting the RTL FE's input format?
> 
> Does this fix the failing tests?

> From 642d511fdba3a33fb18ce46c549f7c972ed6b14e Mon Sep 17 00:00:00 2001
> From: David Malcolm <dmalcolm@redhat.com>
> Date: Tue, 22 Nov 2016 11:06:41 -0500
> Subject: [PATCH] print-rtl.c: conditionalize quotes for filenames
> 
> gcc/ChangeLog:
> 	* print-rtl.c (rtx_writer::print_rtx_operand_code_i): Only use
> 	quotes for filenames when in compact mode.
> ---
>  gcc/print-rtl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
> index 77e6b05..5370602 100644
> --- a/gcc/print-rtl.c
> +++ b/gcc/print-rtl.c
> @@ -371,7 +371,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
>        if (INSN_HAS_LOCATION (in_insn))
>  	{
>  	  expanded_location xloc = insn_location (in_insn);
> -	  fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
> +	  if (m_compact)
> +	    fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
> +	  else
> +	    fprintf (m_outfile, " %s:%i", xloc.file, xloc.line);
>  	}
>  #endif
>      }
> -- 
> 1.8.5.3

I'd like to get our test failure fixed, either by changing
print-rtl.c or our test case.  Is the above patch good for trunk?
It does fix the s390 test failure.

Ciao

Dominik ^_^  ^_^
Bernd Schmidt Dec. 1, 2016, 12:27 p.m. UTC | #3
On 12/01/2016 11:12 AM, Dominik Vogt wrote:
>>
>> diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
>> index 77e6b05..5370602 100644
>> --- a/gcc/print-rtl.c
>> +++ b/gcc/print-rtl.c
>> @@ -371,7 +371,10 @@ rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
>>        if (INSN_HAS_LOCATION (in_insn))
>>  	{
>>  	  expanded_location xloc = insn_location (in_insn);
>> -	  fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
>> +	  if (m_compact)
>> +	    fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
>> +	  else
>> +	    fprintf (m_outfile, " %s:%i", xloc.file, xloc.line);
>>  	}
>>  #endif
>>      }
>> --
>> 1.8.5.3
>
> I'd like to get our test failure fixed, either by changing
> print-rtl.c or our test case.  Is the above patch good for trunk?
> It does fix the s390 test failure.

I still don't see a strong reason not to print the quotes, so I'd 
suggest changing the testcase.


Bernd
Andreas Krebbel Dec. 2, 2016, 12:35 p.m. UTC | #4
On Thu, Dec 01, 2016 at 01:27:55PM +0100, Bernd Schmidt wrote:
> On 12/01/2016 11:12 AM, Dominik Vogt wrote:
...
> >I'd like to get our test failure fixed, either by changing
> >print-rtl.c or our test case.  Is the above patch good for trunk?
> >It does fix the s390 test failure.
> 
> I still don't see a strong reason not to print the quotes, so I'd
> suggest changing the testcase.

Ok. I've just committed
https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00084.html

Bye,

-Andreas-

Patch
diff mbox

From 642d511fdba3a33fb18ce46c549f7c972ed6b14e Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 22 Nov 2016 11:06:41 -0500
Subject: [PATCH] print-rtl.c: conditionalize quotes for filenames

gcc/ChangeLog:
	* print-rtl.c (rtx_writer::print_rtx_operand_code_i): Only use
	quotes for filenames when in compact mode.
---
 gcc/print-rtl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 77e6b05..5370602 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -371,7 +371,10 @@  rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
       if (INSN_HAS_LOCATION (in_insn))
 	{
 	  expanded_location xloc = insn_location (in_insn);
-	  fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
+	  if (m_compact)
+	    fprintf (m_outfile, " \"%s\":%i", xloc.file, xloc.line);
+	  else
+	    fprintf (m_outfile, " %s:%i", xloc.file, xloc.line);
 	}
 #endif
     }
-- 
1.8.5.3