| Message ID | 1479829122.7673.83.camel@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 ^_^ ^_^
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 ^_^ ^_^
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
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-
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