RTL frontend, insn recognition, and pointer equality

RTL frontend, insn recognition, and pointer equality

[David Malcolm]

I've updated the RTL frontend to the new "compact" dump format and have
it "mostly working", for some definition of that term [1].  I'm
focusing on the "__RTL" extension to cc1, rather than having a
true "rtl1" frontend.

I've run into an issue with insn recognition relating to pointer
equality of rtx.

I have a test case for "final", which contains this x86_64 code
(shown here in compact form):

      (cinsn (set (mem/v:BLK (scratch:DI) [0  A8])
                    (unspec:BLK [
                            (mem/v:BLK (scratch:DI) [0  A8])
                        ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
                 (nil))

This fails inside "final" with:
gcc.dg/rtl/x86_64/final.c:130:1: error: unrecognizable insn:
(insn 5 4 6 2 (set (mem/v:BLK (scratch:DI) [0  A8])
        (unspec:BLK [
                (mem/v:BLK (scratch:DI) [0  A8])
            ] UNSPEC_MEMORY_BLOCKAGE)) -1
     (nil))

(the error message is showing the non-compact form of the
loaded insn).

The pertinent part of i386.md is this:

;; Do not schedule instructions accessing memory across this point.

(define_expand "memory_blockage"
  [(set (match_dup 0)
	(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BLOCKAGE))]
  ""
{
  operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
  MEM_VOLATILE_P (operands[0]) = 1;
})

(define_insn "*memory_blockage"
  [(set (match_operand:BLK 0)
	(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BLOCKAGE))]
  ""
  ""
  [(set_attr "length" "0")])


If I run that directly in memory, the insn *is* recognized:

(gdb) call gen_memory_blockage ()
$19 = (insn 33 0 0 (set (mem/v:BLK (scratch:DI) [0  A8])
        (unspec:BLK [
                (mem/v:BLK (scratch:DI) [0  A8])
            ] UNSPEC_MEMORY_BLOCKAGE)) -1
     (nil))

(gdb) call recog_memoized ((rtx_insn *)$19)
$20 = 695
(gdb) call debug ($19)
(insn 33 0 0 (set (mem/v:BLK (scratch:DI) [0  A8])
        (unspec:BLK [
                (mem/v:BLK (scratch:DI) [0  A8])
            ] UNSPEC_MEMORY_BLOCKAGE)) 695 {*memory_blockage}
     (nil))

By contrast, if I load the rtx from a dump file, the insn is
*not* recognized.

Stepping through insn-recog.c revealed the problem is that the
  (match_dup 0)
fails when run on a reconstructed-from-dump copy:

44388	    case 17:
44389	      if (GET_MODE (x2) != BLKmode)
44390	        return -1;
44391	      x3 = XVECEXP (x2, 0, 0);
44392	      if (!rtx_equal_p (x3, operands[0]))
44393	        return -1;
44394	      return 695; /* *memory_blockage */

(gdb) call debug (x3)
(mem/v:BLK (scratch:DI) [0  A8])

(gdb) call debug (operands[0])
(mem/v:BLK (scratch:DI) [0  A8])

(gdb) p x3
$40 = (rtx) 0x7ffff19ee150
(gdb) p operands[0]
$41 = (rtx) 0x7ffff19ee120

(gdb) call rtx_equal_p ($40, $41)
$42 = 0

The issue is that on loading from a file, we have two distinct
  (scratch:DI)
rtx.

rtl.c:rtx_equal_p has:

    case DEBUG_EXPR:
    case VALUE:
    case SCRATCH:
    CASE_CONST_UNIQUE:
      return 0;

i.e. SCRATCH values always compare as non-equal, unless they
have pointer equality.

It works when built directly from insn-emit.c within gen_memory_blockage:

74308	  emit_insn (gen_rtx_SET (operand0,
74309		gen_rtx_UNSPEC (BLKmode,
74310		gen_rtvec (1,
74311			copy_rtx (operand0)),
74312		17)));

since rtl.c:copy_rtx has:

    case SCRATCH:
      /* SCRATCH must be shared because they represent distinct values.  */
      return orig;

So we have a case where a gen_* builds a pattern that contains a
graph, i.e. where a SCRATCH appears in two places:

(insn 5 4 6 2 (set (mem/v:BLK (scratch:DI) [0  A8])
                               ^^^^^^^^^^
        (unspec:BLK [
                (mem/v:BLK (scratch:DI) [0  A8])
                            ^^^^^^^^^^
            ] UNSPEC_MEMORY_BLOCKAGE)) -1
     (nil))

and for which the recognizer will only recognize the insn
if that graph-like property is preserved: that we have pointer-equality
for the two (scratch:DI).

I'm wondering what the best course of action for the RTL frontend is.

Some ideas:

(a) dump the insn name even in compact mode, and do a string-match to
look it up when reading dumps.  This would isolate us somewhat from .md
file changes, but if we lose the memoized value, it can't be
re-recognized.  This feels like papering over the problem.

(b) for all codes for which rtx_equal_p requires pointer equality, add
some kind of extra ID to the dump, allowing the loader to reconstruct
the graph.  This could be the pointer itself:

  (scratch:DI [0x7ffff19ee150])
  (scratch:DI [ptr:0x7ffff19ee150])

or somesuch, but it would perhaps be better to use some kind of more
human-friendly ID e.g.

  (scratch:DI [ptr-idx: 42])

or similar, rather than subject ourselves to raw hex values.  Probably
need an extra flag to print-rtl.c, to avoid making the dumps more
verbose for everyone who doesn't want this (if so, does "compact" mode
need renaming...)

Or just do this for SCRATCH, maybe?

(c) some kind of special-casing?  (ugh)

(d) something I haven't thought of


I'm leaning towards (b).

Thoughts?
Dave

[1] FWIW, a work-in-progress patch kit is at:
  https://dmalcolm.fedorapeople.org/gcc/2016-10-19/rtl-frontend-v143-relative-to-r241136/
though it's not yet ready for review.

Re: RTL frontend, insn recognition, and pointer equality

[Bernd Schmidt]

On 10/20/2016 01:49 AM, David Malcolm wrote:
>
> (b) for all codes for which rtx_equal_p requires pointer equality, add
> some kind of extra ID to the dump, allowing the loader to reconstruct
> the graph.  This could be the pointer itself:
>
>   (scratch:DI [0x7ffff19ee150])
>   (scratch:DI [ptr:0x7ffff19ee150])
>
> or somesuch, but it would perhaps be better to use some kind of more
> human-friendly ID e.g.
>
>   (scratch:DI [ptr-idx: 42])
>
> or similar, rather than subject ourselves to raw hex values.  Probably
> need an extra flag to print-rtl.c, to avoid making the dumps more
> verbose for everyone who doesn't want this (if so, does "compact" mode
> need renaming...)
>
> Or just do this for SCRATCH, maybe?

Recognizing by SCRATCH wouldn't catch everything, I believe. You should 
be able to check n_dups and dup_loc in recog_data to identify cases 
where you need to ensure something is restored with pointer equality.

I'd hope you already have pointer equality for all instances of each 
pseudo register.


Bernd

Re: RTL frontend, insn recognition, and pointer equality

[Bernd Schmidt]

On 10/20/2016 11:22 AM, Bernd Schmidt wrote:
> On 10/20/2016 01:49 AM, David Malcolm wrote:
>>
>> (b) for all codes for which rtx_equal_p requires pointer equality, add
>> some kind of extra ID to the dump, allowing the loader to reconstruct
>> the graph.  This could be the pointer itself:
>>
>>   (scratch:DI [0x7ffff19ee150])
>>   (scratch:DI [ptr:0x7ffff19ee150])
>>
>> or somesuch, but it would perhaps be better to use some kind of more
>> human-friendly ID e.g.
>>
>>   (scratch:DI [ptr-idx: 42])
>>
>> or similar, rather than subject ourselves to raw hex values.  Probably
>> need an extra flag to print-rtl.c, to avoid making the dumps more
>> verbose for everyone who doesn't want this (if so, does "compact" mode
>> need renaming...)
>>
>> Or just do this for SCRATCH, maybe?
>
> Recognizing by SCRATCH wouldn't catch everything, I believe. You should
> be able to check n_dups and dup_loc in recog_data to identify cases
> where you need to ensure something is restored with pointer equality.
>
> I'd hope you already have pointer equality for all instances of each
> pseudo register.

As for the output format, I think you're on the right track assigning 
IDs, but maybe the following would be easier to parse for machines and 
humans:

       (cinsn (set (mem/v:BLK {0}(scratch:DI) [0  A8])
                     (unspec:BLK [
                             (mem/v:BLK (match_dup 0) [0  A8])
                         ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
                  (nil))

where the {0} could be recognized as a prefix saying "store the address 
of the next rtx into slot 0, with a finalization pass replacing 
match_dups with the corresponding address.

Could also maybe do something like (0|scratch:DI), that ought to also be 
unambiguous.


Bernd

Re: RTL frontend, insn recognition, and pointer equality

[David Malcolm]

On Thu, 2016-10-20 at 13:02 +0200, Bernd Schmidt wrote:
> 
> On 10/20/2016 11:22 AM, Bernd Schmidt wrote:
> > On 10/20/2016 01:49 AM, David Malcolm wrote:
> > > 
> > > (b) for all codes for which rtx_equal_p requires pointer
> > > equality, add
> > > some kind of extra ID to the dump, allowing the loader to
> > > reconstruct
> > > the graph.  This could be the pointer itself:
> > > 
> > >   (scratch:DI [0x7ffff19ee150])
> > >   (scratch:DI [ptr:0x7ffff19ee150])
> > > 
> > > or somesuch, but it would perhaps be better to use some kind of
> > > more
> > > human-friendly ID e.g.
> > > 
> > >   (scratch:DI [ptr-idx: 42])
> > > 
> > > or similar, rather than subject ourselves to raw hex values. 
> > >  Probably
> > > need an extra flag to print-rtl.c, to avoid making the dumps more
> > > verbose for everyone who doesn't want this (if so, does "compact"
> > > mode
> > > need renaming...)
> > > 
> > > Or just do this for SCRATCH, maybe?
> > 
> > Recognizing by SCRATCH wouldn't catch everything, I believe. You
> > should
> > be able to check n_dups and dup_loc in recog_data to identify cases
> > where you need to ensure something is restored with pointer
> > equality.

Thanks; I'll look into this.

> > I'd hope you already have pointer equality for all instances of
> > each
> > pseudo register.

The loader already does this; I'll add an explicit selftest for it.
 
> As for the output format, I think you're on the right track assigning
> IDs, but maybe the following would be easier to parse for machines
> and 
> humans:
> 
>        (cinsn (set (mem/v:BLK {0}(scratch:DI) [0  A8])
>                      (unspec:BLK [
>                              (mem/v:BLK (match_dup 0) [0  A8])
>                          ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
>                   (nil))
> 
> where the {0} could be recognized as a prefix saying "store the
> address 
> of the next rtx into slot 0, with a finalization pass replacing 
> match_dups with the corresponding address.
> 
> Could also maybe do something like (0|scratch:DI), that ought to also
> be 
> unambiguous.

match_dup is within the part of rtl.def that's guarded by
#ifdef GENERATOR_FILE, so as-is we can't use match_dup on the host
(unless we expose it there, I suppose).  But we may be able to special
-case it in the loader, and do it directly (or give it its own name,
maybe "reuse_rtx"?)

As for syntax, I think I prefer to have the ID to appear within the
paren, since we have enough weird syntax to cope with within the
operands, so it might look like:

   ({0} scratch:DI)
or:
   (0|scratch:DI)

with the insn as a whole looking like:

       (cinsn (set (mem/v:BLK (0|scratch:DI) [0  A8])
                      (unspec:BLK [
                              (mem/v:BLK (reuse_rtx 0) [0  A8])
                          ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
                   (nil))

Re: RTL frontend, insn recognition, and pointer equality

[Bernd Schmidt]

On 10/20/2016 04:51 PM, David Malcolm wrote:
>    (0|scratch:DI)
>
> with the insn as a whole looking like:
>
>        (cinsn (set (mem/v:BLK (0|scratch:DI) [0  A8])
>                       (unspec:BLK [
>                               (mem/v:BLK (reuse_rtx 0) [0  A8])
>                           ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
>                    (nil))

LGTM. I'd try to expose match_dup though, it's the standard name for 
this sort of thing. Hopefully it won't have to be added to a lot of 
switch statements to shut up warnings.


Bernd

[PATCH] Start adding selftests for print_rtx

[David Malcolm]

On Thu, 2016-10-20 at 17:43 +0200, Bernd Schmidt wrote:
> On 10/20/2016 04:51 PM, David Malcolm wrote:
> >    (0|scratch:DI)
> > 
> > with the insn as a whole looking like:
> > 
> >        (cinsn (set (mem/v:BLK (0|scratch:DI) [0  A8])
> >                       (unspec:BLK [
> >                               (mem/v:BLK (reuse_rtx 0) [0  A8])
> >                           ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
> >                    (nil))
> 
> LGTM. I'd try to expose match_dup though, it's the standard name for
> this sort of thing. Hopefully it won't have to be added to a lot of
> switch statements to shut up warnings.
> 

I'm working on the above, both for dumping, and for reading it back
in.

Here's an enabling patch, which starts adding some selftests
for print-rtl.c.  I put them in rtl-tests.c to take advantage of
some existing rtx-creation tests, and to avoid putting them in
print-rtl.c itself (since that gets built for both build and host,
and changes to it trigger big rebuilds).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
	* print-rtl-function.c (flag_compact): Move extern decl to...
	* print-rtl.h (flag_compact): ...here.
	* rtl-tests.c (selftests::assert_rtl_dump_eq): New function.
	(ASSERT_RTL_DUMP_EQ): New macro.
	(selftest::test_dumping_regs): New function.
	(selftest::test_dumping_insns): New function.
	(selftest::test_uncond_jump): Add uses of ASSERT_RTL_DUMP_EQ on
	the insns.
	(selftest::rtl_tests_c_tests): Call the new test functions.
---
 gcc/print-rtl-function.c |  2 --
 gcc/print-rtl.h          |  2 ++
 gcc/rtl-tests.c          | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
index f46304b..7ce1b90 100644
--- a/gcc/print-rtl-function.c
+++ b/gcc/print-rtl-function.c
@@ -34,8 +34,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "memmodel.h"
 #include "emit-rtl.h"
 
-extern bool flag_compact;
-
 /* Print an "(edge-from)" or "(edge-to)" directive describing E
    to OUTFILE.  */
 
diff --git a/gcc/print-rtl.h b/gcc/print-rtl.h
index 7a1dcaf..8dfba8b 100644
--- a/gcc/print-rtl.h
+++ b/gcc/print-rtl.h
@@ -20,6 +20,8 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_PRINT_RTL_H
 #define GCC_PRINT_RTL_H
 
+extern bool flag_compact;
+
 #ifdef BUFSIZ
 extern void print_rtl (FILE *, const_rtx);
 #endif
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index 723efa5..6d472d1 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -57,6 +57,85 @@ verify_print_pattern (const char *expected, rtx pat)
   ASSERT_STREQ (expected, pp_formatted_text (&pp));
 }
 
+/* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
+   Use LOC as the effective location when reporting errors.  */
+
+static void
+assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x)
+{
+  named_temp_file tmp_out (".rtl");
+  FILE *outfile = fopen (tmp_out.get_filename (), "w");
+  flag_compact = true;
+  print_rtl (outfile, x);
+  flag_compact = false;
+  fclose (outfile);
+
+  char *dump = read_file (SELFTEST_LOCATION, tmp_out.get_filename ());
+  ASSERT_STREQ_AT (loc, expected_dump, dump);
+  free (dump);
+}
+
+/* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode.  */
+
+#define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
+  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
+
+/* Verify that regs are dumped as expected (in compact mode).  */
+
+static void
+test_dumping_regs ()
+{
+  /* Test dumping of hard regs.  This is inherently target-specific due
+     to the name.  */
+#ifdef I386_OPTS_H
+  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
+#endif
+
+  /* Test dumping of virtual regs.  The various virtual regs are inited as
+     Pmode, so this is target-specific.  The tests below assume DImode, so
+     only run the tests for targets where Pmode is DImode.  */
+  if (Pmode == DImode)
+    {
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-incoming-args)",
+			  virtual_incoming_args_rtx);
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-stack-vars)",
+			  virtual_stack_vars_rtx);
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-stack-dynamic)",
+			  virtual_stack_dynamic_rtx);
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-outgoing-args)",
+			  virtual_outgoing_args_rtx);
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-cfa)",
+			  virtual_cfa_rtx);
+      ASSERT_RTL_DUMP_EQ ("(reg:DI virtual-preferred-stack-boundary)",
+			  virtual_preferred_stack_boundary_rtx);
+    }
+
+  /* Test dumping of non-virtual pseudos.  */
+  ASSERT_RTL_DUMP_EQ ("(reg:SI %0)",
+    gen_raw_REG (SImode, LAST_VIRTUAL_REGISTER + 1));
+  ASSERT_RTL_DUMP_EQ ("(reg:SI %1)",
+    gen_raw_REG (SImode, LAST_VIRTUAL_REGISTER + 2));
+}
+
+/* Verify that insns are dumped as expected (in compact mode).  */
+
+static void
+test_dumping_insns ()
+{
+  /* Barriers.  */
+  rtx_barrier *barrier = as_a <rtx_barrier *> (rtx_alloc (BARRIER));
+  SET_NEXT_INSN (barrier) = NULL;
+  ASSERT_RTL_DUMP_EQ ("(cbarrier)\n", barrier);
+
+  /* Labels.  */
+  rtx_insn *label = gen_label_rtx ();
+  CODE_LABEL_NUMBER (label) = 42;
+  ASSERT_RTL_DUMP_EQ ("(clabel 0 42 \"\")\n", label);
+
+  LABEL_NAME (label)= "some_label";
+  ASSERT_RTL_DUMP_EQ ("(clabel 0 42 (\"some_label\"))\n", label);
+}
+
 /* Unit testing of "single_set".  */
 
 static void
@@ -92,6 +171,10 @@ test_uncond_jump ()
 
   verify_print_pattern ("pc=L0", jump_pat);
 
+  ASSERT_RTL_DUMP_EQ ("(set (pc)\n"
+		      "    (label_ref 0))",
+		      jump_pat);
+
   rtx_insn *jump_insn = emit_jump_insn (jump_pat);
   ASSERT_FALSE (any_condjump_p (jump_insn));
   ASSERT_TRUE (any_uncondjump_p (jump_insn));
@@ -99,6 +182,11 @@ test_uncond_jump ()
   ASSERT_TRUE (simplejump_p (jump_insn));
   ASSERT_TRUE (onlyjump_p (jump_insn));
   ASSERT_TRUE (control_flow_insn_p (jump_insn));
+
+  ASSERT_RTL_DUMP_EQ ("(cjump_insn (set (pc)\n"
+		      "        (label_ref 0))\n"
+		      "     (nil))\n",
+		      jump_insn);
 }
 
 /* Run all of the selftests within this file.  */
@@ -106,6 +194,8 @@ test_uncond_jump ()
 void
 rtl_tests_c_tests ()
 {
+  test_dumping_regs ();
+  test_dumping_insns ();
   test_single_set ();
   test_uncond_jump ();
 
-- 
1.8.5.3

Re: [PATCH] Start adding selftests for print_rtx

[Bernd Schmidt]

On 10/21/2016 02:36 AM, David Malcolm wrote:
> +  /* Test dumping of hard regs.  This is inherently target-specific due
> +     to the name.  */
> +#ifdef I386_OPTS_H
> +  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
> +#endif

Generally putting in target dependencies like this isn't something we 
like to do. The patch is OK without this part, and we can revisit this, 
but maybe there wants to be a target hook for running target-specific 
selftests.

> +  ASSERT_RTL_DUMP_EQ ("(cjump_insn (set (pc)\n"
> +		      "        (label_ref 0))\n"
> +		      "     (nil))\n",
> +		      jump_insn);
>  }

I do wonder about the (nil)s and whether we can eliminate them.


Bernd

[PATCH] Start adding target-specific selftests

[David Malcolm]

On Fri, 2016-10-21 at 12:04 +0200, Bernd Schmidt wrote:
> On 10/21/2016 02:36 AM, David Malcolm wrote:
> > +  /* Test dumping of hard regs.  This is inherently target
> > -specific due
> > +     to the name.  */
> > +#ifdef I386_OPTS_H
> > +  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
> > +#endif
> 
> Generally putting in target dependencies like this isn't something we
> like to do. The patch is OK without this part, and we can revisit
> this,
> but maybe there wants to be a target hook for running target-specific
> selftests.

Thanks.  I removed the above target-specific part, and committed it
as r241405 (having reverified bootstrap&regrtest on x86_64-pc-linux-gnu).

The following patch implements a target hook for running target-specific
selftests.

It implements the above test for dumping of hard regs, putting it
within i386.c.

It's rather trivial, but I have followups that add further
target-specific tests, so hopefully this foundation is OK.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?
 
> > +  ASSERT_RTL_DUMP_EQ ("(cjump_insn (set (pc)\n"
> > +		      "        (label_ref 0))\n"
> > +		      "     (nil))\n",
> > +		      jump_insn);
> >  }
> 
> I do wonder about the (nil)s and whether we can eliminate them.

I hope to.

gcc/ChangeLog:
	* config/i386/i386.c: Include "selftest.h" and "selftest-rtl.h".
	(selftest::ix86_test_dumping_hard_regs): New function.
	(selftest::ix86_run_selftests): New function.
	(TARGET_RUN_TARGET_SELFTESTS): When CHECKING_P, wire this up to
	selftest::ix86_run_selftests.
	* doc/tm.texi.in (TARGET_RUN_TARGET_SELFTESTS): New.
	* doc/tm.texi: Regenerate
	* rtl-tests.c: Include "selftest-rtl.h".
	(selftest::assert_rtl_dump_eq): Make non-static.
	(ASSERT_RTL_DUMP_EQ): Move to selftest-rtl.h.
	(selftest::test_dumping_regs): Update comment.
	* selftest-rtl.h: New file.
	* selftest-run-tests.c: Include "target.h".
	(selftest::run_tests): If non-NULL, call
	targetm.run_target_selftests.
	* target.def (run_target_selftests): New hook.
---
 gcc/config/i386/i386.c   | 34 ++++++++++++++++++++++++++++++++++
 gcc/doc/tm.texi          |  4 ++++
 gcc/doc/tm.texi.in       |  2 ++
 gcc/rtl-tests.c          | 10 +++-------
 gcc/selftest-rtl.h       | 45 +++++++++++++++++++++++++++++++++++++++++++++
 gcc/selftest-run-tests.c |  5 +++++
 gcc/target.def           |  6 ++++++
 7 files changed, 99 insertions(+), 7 deletions(-)
 create mode 100644 gcc/selftest-rtl.h

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3e6f8fd..8f6ceb4 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -77,6 +77,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "case-cfn-macros.h"
 #include "regrename.h"
 #include "dojump.h"
+#include "selftest.h"
+#include "selftest-rtl.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -50365,6 +50367,33 @@ ix86_addr_space_zero_address_valid (addr_space_t as)
 #undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
 #define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID ix86_addr_space_zero_address_valid
 
+/* Target-specific selftests.  */
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Verify that hard regs are dumped as expected (in compact mode).  */
+
+static void
+ix86_test_dumping_hard_regs ()
+{
+  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
+  ASSERT_RTL_DUMP_EQ ("(reg:SI dx)", gen_raw_REG (SImode, 1));
+}
+
+/* Run all target-specific selftests.  */
+
+static void
+ix86_run_selftests (void)
+{
+  ix86_test_dumping_hard_regs ();
+}
+
+} // namespace selftest
+
+#endif /* CHECKING_P */
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_RETURN_IN_MEMORY
 #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
@@ -50840,6 +50869,11 @@ ix86_addr_space_zero_address_valid (addr_space_t as)
 #undef TARGET_CUSTOM_FUNCTION_DESCRIPTORS
 #define TARGET_CUSTOM_FUNCTION_DESCRIPTORS 1
 
+#if CHECKING_P
+#undef TARGET_RUN_TARGET_SELFTESTS
+#define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
+#endif /* #if CHECKING_P */
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 #include "gt-i386.h"
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 29dc73b..7efcf57 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11821,3 +11821,7 @@ All and all it does not take long to convert ports that the
 maintainer is familiar with.
 
 @end defmac
+
+@deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
+If selftests are enabled, run any selftests for this target.
+@end deftypefn
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index efcd741..fb94dd8 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8307,3 +8307,5 @@ All and all it does not take long to convert ports that the
 maintainer is familiar with.
 
 @end defmac
+
+@hook TARGET_RUN_TARGET_SELFTESTS
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index b723560..10c0ddc 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgbuild.h"
 #include "print-rtl.h"
 #include "selftest.h"
+#include "selftest-rtl.h"
 #include "function.h"
 #include "memmodel.h"
 #include "emit-rtl.h"
@@ -60,7 +61,7 @@ verify_print_pattern (const char *expected, rtx pat)
 /* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
    Use LOC as the effective location when reporting errors.  */
 
-static void
+void
 assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x)
 {
   named_temp_file tmp_out (".rtl");
@@ -75,18 +76,13 @@ assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x)
   free (dump);
 }
 
-/* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode.  */
-
-#define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
-  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
-
 /* Verify that regs are dumped as expected (in compact mode).  */
 
 static void
 test_dumping_regs ()
 {
   /* Dumps of hard regs contain a target-specific name, so we don't test
-     it here.  */
+     it here; this can be tested in target-specific selftests.  */
 
   /* Test dumping of virtual regs.  The various virtual regs are inited as
      Pmode, so this is target-specific.  The tests below assume DImode, so
diff --git a/gcc/selftest-rtl.h b/gcc/selftest-rtl.h
new file mode 100644
index 0000000..0f0e167
--- /dev/null
+++ b/gcc/selftest-rtl.h
@@ -0,0 +1,45 @@
+/* A self-testing framework, for use by -fself-test.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_SELFTEST_RTL_H
+#define GCC_SELFTEST_RTL_H
+
+/* The selftest code should entirely disappear in a production
+   configuration, hence we guard all of it with #if CHECKING_P.  */
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
+   Use LOC as the effective location when reporting errors.  */
+
+extern void
+assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x);
+
+/* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode.  */
+
+#define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
+  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
+
+} /* end of namespace selftest.  */
+
+#endif /* #if CHECKING_P */
+
+#endif /* GCC_SELFTEST_RTL_H */
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index d9d3ea1..68930ae 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "selftest.h"
 #include "tree.h"
+#include "target.h"
 #include "langhooks.h"
 
 /* This function needed to be split out from selftest.c as it references
@@ -77,6 +78,10 @@ selftest::run_tests ()
   /* This one relies on most of the above.  */
   function_tests_c_tests ();
 
+  /* Run any target-specific selftests.  */
+  if (targetm.run_target_selftests)
+    targetm.run_target_selftests ();
+
   /* Run any lang-specific selftests.  */
   lang_hooks.run_lang_selftests ();
 
diff --git a/gcc/target.def b/gcc/target.def
index 29d1f81..61c5397 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -6136,6 +6136,12 @@ HOOK_VECTOR_END (mode_switching)
 #include "target-insns.def"
 #undef DEF_TARGET_INSN
 
+DEFHOOK
+(run_target_selftests,
+ "If selftests are enabled, run any selftests for this target.",
+ void, (void),
+ NULL)
+
 /* Close the 'struct gcc_target' definition.  */
 HOOK_VECTOR_END (C90_EMPTY_HACK)
 
-- 
1.8.5.3

[Ping] Re: [PATCH] Start adding target-specific selftests

[David Malcolm]

(ping)

https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01785.html 

On Fri, 2016-10-21 at 13:45 -0400, David Malcolm wrote:
> On Fri, 2016-10-21 at 12:04 +0200, Bernd Schmidt wrote:
> > On 10/21/2016 02:36 AM, David Malcolm wrote:
> > > +  /* Test dumping of hard regs.  This is inherently target
> > > -specific due
> > > +     to the name.  */
> > > +#ifdef I386_OPTS_H
> > > +  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
> > > +#endif
> > 
> > Generally putting in target dependencies like this isn't something
> > we
> > like to do. The patch is OK without this part, and we can revisit
> > this,
> > but maybe there wants to be a target hook for running target
> > -specific
> > selftests.
> 
> Thanks.  I removed the above target-specific part, and committed it
> as r241405 (having reverified bootstrap&regrtest on x86_64-pc-linux
> -gnu).
> 
> The following patch implements a target hook for running target
> -specific
> selftests.
> 
> It implements the above test for dumping of hard regs, putting it
> within i386.c.
> 
> It's rather trivial, but I have followups that add further
> target-specific tests, so hopefully this foundation is OK.
> 
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
>  
> > > +  ASSERT_RTL_DUMP_EQ ("(cjump_insn (set (pc)\n"
> > > +		      "        (label_ref 0))\n"
> > > +		      "     (nil))\n",
> > > +		      jump_insn);
> > >  }
> > 
> > I do wonder about the (nil)s and whether we can eliminate them.
> 
> I hope to.
> 
> gcc/ChangeLog:
> 	* config/i386/i386.c: Include "selftest.h" and "selftest
> -rtl.h".
> 	(selftest::ix86_test_dumping_hard_regs): New function.
> 	(selftest::ix86_run_selftests): New function.
> 	(TARGET_RUN_TARGET_SELFTESTS): When CHECKING_P, wire this up to
> 	selftest::ix86_run_selftests.
> 	* doc/tm.texi.in (TARGET_RUN_TARGET_SELFTESTS): New.
> 	* doc/tm.texi: Regenerate
> 	* rtl-tests.c: Include "selftest-rtl.h".
> 	(selftest::assert_rtl_dump_eq): Make non-static.
> 	(ASSERT_RTL_DUMP_EQ): Move to selftest-rtl.h.
> 	(selftest::test_dumping_regs): Update comment.
> 	* selftest-rtl.h: New file.
> 	* selftest-run-tests.c: Include "target.h".
> 	(selftest::run_tests): If non-NULL, call
> 	targetm.run_target_selftests.
> 	* target.def (run_target_selftests): New hook.
> ---
>  gcc/config/i386/i386.c   | 34 ++++++++++++++++++++++++++++++++++
>  gcc/doc/tm.texi          |  4 ++++
>  gcc/doc/tm.texi.in       |  2 ++
>  gcc/rtl-tests.c          | 10 +++-------
>  gcc/selftest-rtl.h       | 45
> +++++++++++++++++++++++++++++++++++++++++++++
>  gcc/selftest-run-tests.c |  5 +++++
>  gcc/target.def           |  6 ++++++
>  7 files changed, 99 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/selftest-rtl.h
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 3e6f8fd..8f6ceb4 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -77,6 +77,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "case-cfn-macros.h"
>  #include "regrename.h"
>  #include "dojump.h"
> +#include "selftest.h"
> +#include "selftest-rtl.h"
>  
>  /* This file should be included last.  */
>  #include "target-def.h"
> @@ -50365,6 +50367,33 @@ ix86_addr_space_zero_address_valid
> (addr_space_t as)
>  #undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
>  #define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> ix86_addr_space_zero_address_valid
>  
> +/* Target-specific selftests.  */
> +
> +#if CHECKING_P
> +
> +namespace selftest {
> +
> +/* Verify that hard regs are dumped as expected (in compact mode). 
>  */
> +
> +static void
> +ix86_test_dumping_hard_regs ()
> +{
> +  ASSERT_RTL_DUMP_EQ ("(reg:SI ax)", gen_raw_REG (SImode, 0));
> +  ASSERT_RTL_DUMP_EQ ("(reg:SI dx)", gen_raw_REG (SImode, 1));
> +}
> +
> +/* Run all target-specific selftests.  */
> +
> +static void
> +ix86_run_selftests (void)
> +{
> +  ix86_test_dumping_hard_regs ();
> +}
> +
> +} // namespace selftest
> +
> +#endif /* CHECKING_P */
> +
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_RETURN_IN_MEMORY
>  #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
> @@ -50840,6 +50869,11 @@ ix86_addr_space_zero_address_valid
> (addr_space_t as)
>  #undef TARGET_CUSTOM_FUNCTION_DESCRIPTORS
>  #define TARGET_CUSTOM_FUNCTION_DESCRIPTORS 1
>  
> +#if CHECKING_P
> +#undef TARGET_RUN_TARGET_SELFTESTS
> +#define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
> +#endif /* #if CHECKING_P */
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>  \f
>  #include "gt-i386.h"
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 29dc73b..7efcf57 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -11821,3 +11821,7 @@ All and all it does not take long to convert
> ports that the
>  maintainer is familiar with.
>  
>  @end defmac
> +
> +@deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
> +If selftests are enabled, run any selftests for this target.
> +@end deftypefn
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index efcd741..fb94dd8 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -8307,3 +8307,5 @@ All and all it does not take long to convert
> ports that the
>  maintainer is familiar with.
>  
>  @end defmac
> +
> +@hook TARGET_RUN_TARGET_SELFTESTS
> diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
> index b723560..10c0ddc 100644
> --- a/gcc/rtl-tests.c
> +++ b/gcc/rtl-tests.c
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgbuild.h"
>  #include "print-rtl.h"
>  #include "selftest.h"
> +#include "selftest-rtl.h"
>  #include "function.h"
>  #include "memmodel.h"
>  #include "emit-rtl.h"
> @@ -60,7 +61,7 @@ verify_print_pattern (const char *expected, rtx
> pat)
>  /* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
>     Use LOC as the effective location when reporting errors.  */
>  
> -static void
> +void
>  assert_rtl_dump_eq (const location &loc, const char *expected_dump,
> rtx x)
>  {
>    named_temp_file tmp_out (".rtl");
> @@ -75,18 +76,13 @@ assert_rtl_dump_eq (const location &loc, const
> char *expected_dump, rtx x)
>    free (dump);
>  }
>  
> -/* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode. 
>  */
> -
> -#define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
> -  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
> -
>  /* Verify that regs are dumped as expected (in compact mode).  */
>  
>  static void
>  test_dumping_regs ()
>  {
>    /* Dumps of hard regs contain a target-specific name, so we don't
> test
> -     it here.  */
> +     it here; this can be tested in target-specific selftests.  */
>  
>    /* Test dumping of virtual regs.  The various virtual regs are
> inited as
>       Pmode, so this is target-specific.  The tests below assume
> DImode, so
> diff --git a/gcc/selftest-rtl.h b/gcc/selftest-rtl.h
> new file mode 100644
> index 0000000..0f0e167
> --- /dev/null
> +++ b/gcc/selftest-rtl.h
> @@ -0,0 +1,45 @@
> +/* A self-testing framework, for use by -fself-test.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT
> ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_SELFTEST_RTL_H
> +#define GCC_SELFTEST_RTL_H
> +
> +/* The selftest code should entirely disappear in a production
> +   configuration, hence we guard all of it with #if CHECKING_P.  */
> +
> +#if CHECKING_P
> +
> +namespace selftest {
> +
> +/* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
> +   Use LOC as the effective location when reporting errors.  */
> +
> +extern void
> +assert_rtl_dump_eq (const location &loc, const char *expected_dump,
> rtx x);
> +
> +/* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode. 
>  */
> +
> +#define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
> +  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
> +
> +} /* end of namespace selftest.  */
> +
> +#endif /* #if CHECKING_P */
> +
> +#endif /* GCC_SELFTEST_RTL_H */
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index d9d3ea1..68930ae 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "coretypes.h"
>  #include "selftest.h"
>  #include "tree.h"
> +#include "target.h"
>  #include "langhooks.h"
>  
>  /* This function needed to be split out from selftest.c as it
> references
> @@ -77,6 +78,10 @@ selftest::run_tests ()
>    /* This one relies on most of the above.  */
>    function_tests_c_tests ();
>  
> +  /* Run any target-specific selftests.  */
> +  if (targetm.run_target_selftests)
> +    targetm.run_target_selftests ();
> +
>    /* Run any lang-specific selftests.  */
>    lang_hooks.run_lang_selftests ();
>  
> diff --git a/gcc/target.def b/gcc/target.def
> index 29d1f81..61c5397 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -6136,6 +6136,12 @@ HOOK_VECTOR_END (mode_switching)
>  #include "target-insns.def"
>  #undef DEF_TARGET_INSN
>  
> +DEFHOOK
> +(run_target_selftests,
> + "If selftests are enabled, run any selftests for this target.",
> + void, (void),
> + NULL)
> +
>  /* Close the 'struct gcc_target' definition.  */
>  HOOK_VECTOR_END (C90_EMPTY_HACK)
>  

Re: [PATCH] Start adding target-specific selftests

[Bernd Schmidt]

On 10/21/2016 07:45 PM, David Malcolm wrote:
>
> gcc/ChangeLog:
> 	* config/i386/i386.c: Include "selftest.h" and "selftest-rtl.h".
> 	(selftest::ix86_test_dumping_hard_regs): New function.
> 	(selftest::ix86_run_selftests): New function.
> 	(TARGET_RUN_TARGET_SELFTESTS): When CHECKING_P, wire this up to
> 	selftest::ix86_run_selftests.
> 	* doc/tm.texi.in (TARGET_RUN_TARGET_SELFTESTS): New.
> 	* doc/tm.texi: Regenerate
> 	* rtl-tests.c: Include "selftest-rtl.h".
> 	(selftest::assert_rtl_dump_eq): Make non-static.
> 	(ASSERT_RTL_DUMP_EQ): Move to selftest-rtl.h.
> 	(selftest::test_dumping_regs): Update comment.
> 	* selftest-rtl.h: New file.
> 	* selftest-run-tests.c: Include "target.h".
> 	(selftest::run_tests): If non-NULL, call
> 	targetm.run_target_selftests.
> 	* target.def (run_target_selftests): New hook.

Ok.


Bernd

[PATCH] rtx_writer: avoid printing trailing nils

[David Malcolm]

On Fri, 2016-10-21 at 12:04 +0200, Bernd Schmidt wrote:
> On 10/21/2016 02:36 AM, David Malcolm wrote:
> > +  ASSERT_RTL_DUMP_EQ ("(cjump_insn (set (pc)\n"
> > +                           "        (label_ref 0))\n"
> > +                                          "     (nil))\n",
> > +                                                      jump_insn);
> >  }
> 
> I do wonder about the (nil)s and whether we can eliminate them.

The following patch does so, by adding a flag param to
rtx_writer::print_rtx allowing omitting the (nil) when in compact mode.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
	* print-rtl.c (rtx_writer::print_rtx_operand_code_0): Don't print
	trailing "(nil)" for NOTE_VAR_LOCATION and for ENTRY_VALUE_EXP.
	(rtx_writer::print_rtx_operand_code_e): Likewise for
	CALL_INSN_FUNCTION_USAGE and for REG_NOTES.
	(rtx_writer::print_rtx_operand_codes_E_and_V): Always print nils
	within a vec.
	(rtx_writer::print_rtx): Add param "print_nil" to support not
	printing some trailing "(nil)" values when in compact mode.
	Don't print them for PAT_VAR_LOCATION_LOC.
	(print_inline_rtx): Update for new param.
	(debug_rtx): Likewise.
	(rtx_writer::print_rtl): Likewise.
	(rtx_writer::print_rtl_single_with_indent): Likewise.
	* print-rtl.h (rtx_writer::print_rtx): Add "print_nil" param.
	* rtl-tests.c (selftests::test_uncond_jump): Remove trailing
	"(nil)" from expected output.
---
 gcc/print-rtl.c | 44 +++++++++++++++++++++++++++++---------------
 gcc/print-rtl.h |  2 +-
 gcc/rtl-tests.c |  2 +-
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 341ecdf..643eb57 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -158,7 +158,7 @@ rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
 	case NOTE_INSN_VAR_LOCATION:
 	case NOTE_INSN_CALL_ARG_LOCATION:
 	  fputc (' ', m_outfile);
-	  print_rtx (NOTE_VAR_LOCATION (in_rtx));
+	  print_rtx (NOTE_VAR_LOCATION (in_rtx), false);
 	  break;
 
 	case NOTE_INSN_CFI:
@@ -201,7 +201,7 @@ rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
       m_indent += 2;
       if (!m_sawclose)
 	fprintf (m_outfile, " ");
-      print_rtx (ENTRY_VALUE_EXP (in_rtx));
+      print_rtx (ENTRY_VALUE_EXP (in_rtx), false);
       m_indent -= 2;
     }
 #endif
@@ -224,11 +224,17 @@ rtx_writer::print_rtx_operand_code_e (const_rtx in_rtx, int idx)
   if (idx == 7 && CALL_P (in_rtx))
     {
       m_in_call_function_usage = true;
-      print_rtx (XEXP (in_rtx, idx));
+      print_rtx (XEXP (in_rtx, idx), false);
       m_in_call_function_usage = false;
     }
   else
-    print_rtx (XEXP (in_rtx, idx));
+    {
+      bool print_nil = true;
+      /* Avoid printing trailing "(nil)" for REG_NOTES.  */
+      if (INSN_CHAIN_CODE_P (GET_CODE (in_rtx)) && idx == 6)
+	print_nil = false;
+      print_rtx (XEXP (in_rtx, idx), print_nil);
+    }
   m_indent -= 2;
 }
 
@@ -252,7 +258,7 @@ rtx_writer::print_rtx_operand_codes_E_and_V (const_rtx in_rtx, int idx)
 	m_sawclose = 1;
 
       for (int j = 0; j < XVECLEN (in_rtx, idx); j++)
-	print_rtx (XVECEXP (in_rtx, idx, j));
+	print_rtx (XVECEXP (in_rtx, idx, j), true);
 
       m_indent -= 2;
     }
@@ -564,10 +570,15 @@ rtx_writer::print_rtx_operand (const_rtx in_rtx, int idx)
     }
 }
 
-/* Print IN_RTX onto m_outfile.  This is the recursive part of printing.  */
+/* Print IN_RTX onto m_outfile.  This is the recursive part of printing.
+   PRINT_NIL controls the printing of "(nil)" in compact mode.
+   In compact mode, if IN_RTX is NULL, then "(nil)" is printed if PRINT_NIL
+   is true, and nothing is printed if PRINT_NIL is false.
+   In non-compact mode, "(nil)" is always printed for NULL, and PRINT_NIL
+   is ignored.  */
 
 void
-rtx_writer::print_rtx (const_rtx in_rtx)
+rtx_writer::print_rtx (const_rtx in_rtx, bool print_nil)
 {
   int idx = 0;
 
@@ -582,8 +593,11 @@ rtx_writer::print_rtx (const_rtx in_rtx)
 
   if (in_rtx == 0)
     {
-      fputs ("(nil)", m_outfile);
-      m_sawclose = 1;
+      if (print_nil || !m_compact)
+	{
+	  fputs ("(nil)", m_outfile);
+	  m_sawclose = 1;
+	}
       return;
     }
   else if (GET_CODE (in_rtx) > NUM_RTX_CODE)
@@ -657,7 +671,7 @@ rtx_writer::print_rtx (const_rtx in_rtx)
 	  else
 	    print_mem_expr (m_outfile, PAT_VAR_LOCATION_DECL (in_rtx));
 	  fputc (' ', m_outfile);
-	  print_rtx (PAT_VAR_LOCATION_LOC (in_rtx));
+	  print_rtx (PAT_VAR_LOCATION_LOC (in_rtx), false);
 	  if (PAT_VAR_LOCATION_STATUS (in_rtx)
 	      == VAR_INIT_STATUS_UNINITIALIZED)
 	    fprintf (m_outfile, " [uninit]");
@@ -765,7 +779,7 @@ void
 print_inline_rtx (FILE *outf, const_rtx x, int ind)
 {
   rtx_writer w (outf, ind, false, false);
-  w.print_rtx (x);
+  w.print_rtx (x, true);
 }
 
 /* Call this function from the debugger to see what X looks like.  */
@@ -774,7 +788,7 @@ DEBUG_FUNCTION void
 debug_rtx (const_rtx x)
 {
   rtx_writer w (stderr, 0, false, false);
-  w.print_rtx (x);
+  w.print_rtx (x, true);
   fprintf (stderr, "\n");
 }
 
@@ -900,14 +914,14 @@ rtx_writer::print_rtl (const_rtx rtx_first)
 	     tmp_rtx = NEXT_INSN (tmp_rtx))
 	  {
 	    fputs (print_rtx_head, m_outfile);
-	    print_rtx (tmp_rtx);
+	    print_rtx (tmp_rtx, true);
 	    fprintf (m_outfile, "\n");
 	  }
 	break;
 
       default:
 	fputs (print_rtx_head, m_outfile);
-	print_rtx (rtx_first);
+	print_rtx (rtx_first, true);
       }
 }
 
@@ -948,7 +962,7 @@ rtx_writer::print_rtl_single_with_indent (const_rtx x, int ind)
   int old_indent = m_indent;
   m_indent = ind;
   m_sawclose = 0;
-  print_rtx (x);
+  print_rtx (x, true);
   putc ('\n', m_outfile);
   m_indent = old_indent;
   return 1;
diff --git a/gcc/print-rtl.h b/gcc/print-rtl.h
index 8496ffa..64043dd 100644
--- a/gcc/print-rtl.h
+++ b/gcc/print-rtl.h
@@ -27,7 +27,7 @@ class rtx_writer
  public:
   rtx_writer (FILE *outfile, int ind, bool simple, bool compact);
 
-  void print_rtx (const_rtx in_rtx);
+  void print_rtx (const_rtx in_rtx, bool print_nil);
   void print_rtl (const_rtx rtx_first);
   int print_rtl_single_with_indent (const_rtx x, int ind);
 
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index cf5239f..006ef92 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -177,7 +177,7 @@ test_uncond_jump ()
 
   ASSERT_RTL_DUMP_EQ ("(cjump_insn 1 (set (pc)\n"
 		      "        (label_ref 0))\n"
-		      "     (nil))\n",
+		      "     )\n",
 		      jump_insn);
 }
 
-- 
1.8.5.3

Re: [PATCH] rtx_writer: avoid printing trailing nils

[Bernd Schmidt]

On 11/04/2016 06:32 PM, David Malcolm wrote:
>    ASSERT_RTL_DUMP_EQ ("(cjump_insn 1 (set (pc)\n"
>  		      "        (label_ref 0))\n"
> -		      "     (nil))\n",
> +		      "     )\n",
>  		      jump_insn);

This looks like the patch doesn't go the whole way - we'd want the space 
savings from avoiding the newline.

You could consider giving the print_nil thing a default argument.


Bernd

Re: [PATCH] rtx_writer: avoid printing trailing nils

[Bernd Schmidt]

Here's something simpler. Only very lightly tested, but isn't something 
like this all that's needed? Could decide whether to apply it to 
expr_list etc. as well.

Index: print-rtl.c
===================================================================
--- print-rtl.c	(revision 241233)
+++ print-rtl.c	(working copy)
@@ -697,7 +697,12 @@ print_rtx (const_rtx in_rtx)

    /* Get the format string and skip the first elements if we have handled
       them already.  */
-  for (; idx < GET_RTX_LENGTH (GET_CODE (in_rtx)); idx++)
+  int limit = GET_RTX_LENGTH (GET_CODE (in_rtx));
+  if (flag_compact
+      && INSN_CHAIN_CODE_P (GET_CODE (in_rtx))
+      && XEXP (in_rtx, limit - 1) == NULL_RTX)
+    limit--;
+  for (; idx < limit; idx++)
      print_rtx_operand (in_rtx, idx);

    switch (GET_CODE (in_rtx))


Bernd

[PATCH] rtx_writer: avoid printing trailing default values

[David Malcolm]

On Fri, 2016-11-04 at 18:51 +0100, Bernd Schmidt wrote:
> Here's something simpler. Only very lightly tested, but isn't
> something
> like this all that's needed? Could decide whether to apply it to
> expr_list etc. as well.
> 
> Index: print-rtl.c
> ===================================================================
> --- print-rtl.c	(revision 241233)
> +++ print-rtl.c	(working copy)
> @@ -697,7 +697,12 @@ print_rtx (const_rtx in_rtx)
> 
>     /* Get the format string and skip the first elements if we have
> handled
>        them already.  */
> -  for (; idx < GET_RTX_LENGTH (GET_CODE (in_rtx)); idx++)
> +  int limit = GET_RTX_LENGTH (GET_CODE (in_rtx));
> +  if (flag_compact
(as of r241586 this is now "m_compact")

> +      && INSN_CHAIN_CODE_P (GET_CODE (in_rtx))
> +      && XEXP (in_rtx, limit - 1) == NULL_RTX)
> +    limit--;
> +  for (; idx < limit; idx++)
>       print_rtx_operand (in_rtx, idx);
> 
>     switch (GET_CODE (in_rtx))

Thanks.

This seems to assume that the final code in the fmt string can be
accessed via XEXP (in_rtx, limit - 1), which if RTL checking is
enabled requires that the code be either 'e' or 'u'.

This isn't the case for JUMP_INSN, JUMP_TABLE_DATA, BARRIER
(all code '0'), CODE_LABEL (code 's') and NOTE (code 'i').

Also, we might want to omit the REG_NOTES from, say a JUMP_INSN, which
is the *penultimate* operand - for example, it doesn't omit the
trailing (nil) from the cjump_insn in test_uncond_jump.

That said, this is much simpler than my patch, so I used it as the
basis for the following: it uses the same approach as your patch, but
loops backwards from the end of the format string (rather than just once)
until it finds a non-default value, using a new function
"operand_has_default_value_p" to handle the logic for that.

With this, compact dumps omit the trailing (nil) for both regular insns
and for JUMP_INSNs, and omits the surplus newline seen in my earlier patch.

It also appears removes the trailing (nil) from expr_list.

Bootstrap&regrtest in progress.

OK for trunk if it passes?

gcc/ChangeLog:
	* print-rtl.c (operand_has_default_value_p): New function.
	(rtx_writer::print_rtx): In compact mode, omit trailing operands
	that have the default values.
	* rtl-tests.c (selftest::test_dumping_insns): Remove empty
	label string from expected dump.
	(seltest::test_uncond_jump): Remove trailing "(nil)" for REG_NOTES
	from expected dump.
---
 gcc/print-rtl.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 gcc/rtl-tests.c |  5 ++---
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 341ecdf..493aa68 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -564,6 +564,40 @@ rtx_writer::print_rtx_operand (const_rtx in_rtx, int idx)
     }
 }
 
+/* Subroutine of rtx_writer::print_rtx.
+   In compact mode, determine if operand IDX of IN_RTX is interesting
+   to dump, or (if in a trailing position) it can be omitted.  */
+
+static bool
+operand_has_default_value_p (const_rtx in_rtx, int idx)
+{
+  const char *format_ptr = GET_RTX_FORMAT (GET_CODE (in_rtx));
+
+  switch (format_ptr[idx])
+    {
+    case 'e':
+    case 'u':
+      return XEXP (in_rtx, idx) == NULL_RTX;
+
+    case 's':
+      return XSTR (in_rtx, idx) == NULL;
+
+    case '0':
+      switch (GET_CODE (in_rtx))
+	{
+	case JUMP_INSN:
+	  return JUMP_LABEL (in_rtx) == NULL_RTX;
+
+	default:
+	  return false;
+
+	}
+
+    default:
+      return false;
+    }
+}
+
 /* Print IN_RTX onto m_outfile.  This is the recursive part of printing.  */
 
 void
@@ -681,9 +715,18 @@ rtx_writer::print_rtx (const_rtx in_rtx)
 	fprintf (m_outfile, " %d", INSN_UID (in_rtx));
     }
 
+  /* Determine which is the final operand to print.
+     In compact mode, skip trailing operands that have the default values
+     e.g. trailing "(nil)" values.  */
+  int limit = GET_RTX_LENGTH (GET_CODE (in_rtx));
+  if (m_compact)
+    while (limit > idx && operand_has_default_value_p (in_rtx, limit - 1))
+      limit--;
+
   /* Get the format string and skip the first elements if we have handled
      them already.  */
-  for (; idx < GET_RTX_LENGTH (GET_CODE (in_rtx)); idx++)
+
+  for (; idx < limit; idx++)
     print_rtx_operand (in_rtx, idx);
 
   switch (GET_CODE (in_rtx))
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index cf5239f..228226b 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -122,7 +122,7 @@ test_dumping_insns ()
   /* Labels.  */
   rtx_insn *label = gen_label_rtx ();
   CODE_LABEL_NUMBER (label) = 42;
-  ASSERT_RTL_DUMP_EQ ("(clabel 0 42 \"\")\n", label);
+  ASSERT_RTL_DUMP_EQ ("(clabel 0 42)\n", label);
 
   LABEL_NAME (label)= "some_label";
   ASSERT_RTL_DUMP_EQ ("(clabel 0 42 (\"some_label\"))\n", label);
@@ -176,8 +176,7 @@ test_uncond_jump ()
   ASSERT_TRUE (control_flow_insn_p (jump_insn));
 
   ASSERT_RTL_DUMP_EQ ("(cjump_insn 1 (set (pc)\n"
-		      "        (label_ref 0))\n"
-		      "     (nil))\n",
+		      "        (label_ref 0)))\n",
 		      jump_insn);
 }
 
-- 
1.8.5.3

Re: [PATCH] rtx_writer: avoid printing trailing default values

[Bernd Schmidt]

On 11/04/2016 08:14 PM, David Malcolm wrote:
>
> With this, compact dumps omit the trailing (nil) for both regular insns
> and for JUMP_INSNs, and omits the surplus newline seen in my earlier patch.
>
> It also appears removes the trailing (nil) from expr_list.
>
> Bootstrap&regrtest in progress.
>
> OK for trunk if it passes?

This seems OK, with something left to be addressed perhaps in a followup:

> +	case JUMP_INSN:
> +	  return JUMP_LABEL (in_rtx) == NULL_RTX;

Weren't we going to omit JUMP_LABELs in compact mode? If so, we'll need 
a m_compact test here eventually as well.


Bernd

Re: [PATCH] rtx_writer: avoid printing trailing default values

[David Malcolm]

On Fri, 2016-11-04 at 19:53 +0100, Bernd Schmidt wrote:
> On 11/04/2016 08:14 PM, David Malcolm wrote:
> > 
> > With this, compact dumps omit the trailing (nil) for both regular
> > insns
> > and for JUMP_INSNs, and omits the surplus newline seen in my
> > earlier patch.
> > 
> > It also appears removes the trailing (nil) from expr_list.
> > 
> > Bootstrap&regrtest in progress.
> > 
> > OK for trunk if it passes?
> 
> This seems OK, with something left to be addressed perhaps in a
> followup:
> 
> > +	case JUMP_INSN:
> > +	  return JUMP_LABEL (in_rtx) == NULL_RTX;
> 
> Weren't we going to omit JUMP_LABELs in compact mode? If so, we'll
> need 
> a m_compact test here eventually as well.

We already omit JUMP_LABELs in compact mode, in
rtx_writer::print_rtx_operand_code_0, but we need this test to pass so
that we can potentially omit earlier (nil) operands in a JUMP_INSN e.g.
REG_NOTES.

Given that, perhaps this test should read:

	case JUMP_INSN:	

	  /* JUMP_LABELs are always omitted in compact mode, so treat
	     any value here as omittable, so that earlier operands can
	     potentially be omitted also.  */
	  return true;

or:

          return m_compact;

if we want to support calling operand_has_default_value_p in non
-compact mode.

OK with one of those changes?  (assuming successful testing)

Re: [PATCH] rtx_writer: avoid printing trailing default values

[Bernd Schmidt]

On 11/04/2016 08:25 PM, David Malcolm wrote:

>           return m_compact;

Ok with this one plus a comment.


Bernd

Re: [PATCH] rtx_writer: avoid printing trailing default values

[David Malcolm]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Fri, 2016-11-04 at 20:40 +0100, Bernd Schmidt wrote:
> On 11/04/2016 08:25 PM, David Malcolm wrote:
> 
> >           return m_compact;
> 
> Ok with this one plus a comment.
> 

Thanks.

Using m_compact required turning the static function into a (private)
member function.  For reference, here's what I committed (r241908),
having verified bootstrap&regrtest.   

[-- Attachment #2: r241908.patch --]
[-- Type: text/x-patch, Size: 3724 bytes --]

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 241907)
+++ gcc/ChangeLog	(revision 241908)
@@ -1,3 +1,16 @@
+2016-11-07  David Malcolm  <dmalcolm@redhat.com>
+
+	* print-rtl.c (rtx_writer::operand_has_default_value_p): New
+	method.
+	(rtx_writer::print_rtx): In compact mode, omit trailing operands
+	that have the default values.
+	* print-rtl.h (rtx_writer::operand_has_default_value_p): New
+	method.
+	* rtl-tests.c (selftest::test_dumping_insns): Remove empty
+	label string from expected dump.
+	(seltest::test_uncond_jump): Remove trailing "(nil)" for REG_NOTES
+	from expected dump.
+
 2016-11-07  Jakub Jelinek  <jakub@redhat.com>
 
 	PR target/77834
Index: gcc/print-rtl.c
===================================================================
--- gcc/print-rtl.c	(revision 241907)
+++ gcc/print-rtl.c	(revision 241908)
@@ -564,6 +564,43 @@
     }
 }
 
+/* Subroutine of rtx_writer::print_rtx.
+   In compact mode, determine if operand IDX of IN_RTX is interesting
+   to dump, or (if in a trailing position) it can be omitted.  */
+
+bool
+rtx_writer::operand_has_default_value_p (const_rtx in_rtx, int idx)
+{
+  const char *format_ptr = GET_RTX_FORMAT (GET_CODE (in_rtx));
+
+  switch (format_ptr[idx])
+    {
+    case 'e':
+    case 'u':
+      return XEXP (in_rtx, idx) == NULL_RTX;
+
+    case 's':
+      return XSTR (in_rtx, idx) == NULL;
+
+    case '0':
+      switch (GET_CODE (in_rtx))
+	{
+	case JUMP_INSN:
+	  /* JUMP_LABELs are always omitted in compact mode, so treat
+	     any value here as omittable, so that earlier operands can
+	     potentially be omitted also.  */
+	  return m_compact;
+
+	default:
+	  return false;
+
+	}
+
+    default:
+      return false;
+    }
+}
+
 /* Print IN_RTX onto m_outfile.  This is the recursive part of printing.  */
 
 void
@@ -681,9 +718,18 @@
 	fprintf (m_outfile, " %d", INSN_UID (in_rtx));
     }
 
+  /* Determine which is the final operand to print.
+     In compact mode, skip trailing operands that have the default values
+     e.g. trailing "(nil)" values.  */
+  int limit = GET_RTX_LENGTH (GET_CODE (in_rtx));
+  if (m_compact)
+    while (limit > idx && operand_has_default_value_p (in_rtx, limit - 1))
+      limit--;
+
   /* Get the format string and skip the first elements if we have handled
      them already.  */
-  for (; idx < GET_RTX_LENGTH (GET_CODE (in_rtx)); idx++)
+
+  for (; idx < limit; idx++)
     print_rtx_operand (in_rtx, idx);
 
   switch (GET_CODE (in_rtx))
Index: gcc/print-rtl.h
===================================================================
--- gcc/print-rtl.h	(revision 241907)
+++ gcc/print-rtl.h	(revision 241908)
@@ -39,6 +39,7 @@
   void print_rtx_operand_code_r (const_rtx in_rtx);
   void print_rtx_operand_code_u (const_rtx in_rtx, int idx);
   void print_rtx_operand (const_rtx in_rtx, int idx);
+  bool operand_has_default_value_p (const_rtx in_rtx, int idx);
 
  private:
   FILE *m_outfile;
Index: gcc/rtl-tests.c
===================================================================
--- gcc/rtl-tests.c	(revision 241907)
+++ gcc/rtl-tests.c	(revision 241908)
@@ -122,7 +122,7 @@
   /* Labels.  */
   rtx_insn *label = gen_label_rtx ();
   CODE_LABEL_NUMBER (label) = 42;
-  ASSERT_RTL_DUMP_EQ ("(clabel 0 42 \"\")\n", label);
+  ASSERT_RTL_DUMP_EQ ("(clabel 0 42)\n", label);
 
   LABEL_NAME (label)= "some_label";
   ASSERT_RTL_DUMP_EQ ("(clabel 0 42 (\"some_label\"))\n", label);
@@ -176,8 +176,7 @@
   ASSERT_TRUE (control_flow_insn_p (jump_insn));
 
   ASSERT_RTL_DUMP_EQ ("(cjump_insn 1 (set (pc)\n"
-		      "        (label_ref 0))\n"
-		      "     (nil))\n",
+		      "        (label_ref 0)))\n",
 		      jump_insn);
 }
 

[PATCH] print_rtx: implement support for reuse IDs

[David Malcolm]

On Thu, 2016-10-20 at 11:22 +0200, Bernd Schmidt wrote:
> > Recognizing by SCRATCH wouldn't catch everything, I believe. You
> > should
> > be able to check n_dups and dup_loc in recog_data to identify cases
> > where you need to ensure something is restored with pointer
> > equality.

Thanks.  I attemped to use those fields of recog_data, but it doesn't
seem to be exactly what's needed here.

Recall that we have:

      (cinsn (set (mem/v:BLK (scratch:DI) [0  A8])
                    (unspec:BLK [
                            (mem/v:BLK (scratch:DI) [0  A8])
                        ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
                 (nil))

If I do a recog and then insn_extract on the insn in question, then
this code in insn-extract.c fires:

    case 695:  /* *memory_blockage */
      ro[0] = *(ro_loc[0] = &XEXP (pat, 0));
      recog_data.dup_loc[0] = &XVECEXP (XEXP (pat, 1), 0, 0);
      recog_data.dup_num[0] = 0;
      break;

and we have:

      (gdb) call debug (*recog_data.dup_loc[0])
      (mem/v:BLK (scratch:DI) [0  A8])

      (gdb) call debug (ro[0])
      (mem/v:BLK (scratch:DI) [0  A8])

      (gdb) p ro[0]
      $17 = (rtx) 0x7ffff19bca98
      (gdb) p recog_data.dup_loc[0]
      $18 = (rtx *) 0x7ffff190eeb8

i.e. it's recorded that the two "(mem/v:BLK (scratch:DI) [0  A8])" match.

However, this doesn't seem to help in terms of actually writing out
the data: it's identified a match_dup, but it's of the two MEM
instances; it's the SCRATCH instance within them that's shared.

Somehow we'd need to traverse the identified match_dup cases and figure
out which descendents within them have identical pointers.

So I came up with a different approach, which doesn't directly use
recog_data.  Instead, there's a new "rtx_reuse_manager" class,
which supports directly identifying the identical SCRATCH instances
during a dump.  This approach seems to me to be simpler, and it's more
flexible, as it can cope with other ways in which pointer-equality
could occur, outside of a match_dup.

On Thu, 2016-10-20 at 17:43 +0200, Bernd Schmidt wrote:
> On 10/20/2016 04:51 PM, David Malcolm wrote:
> >    (0|scratch:DI)
> > 
> > with the insn as a whole looking like:
> > 
> >        (cinsn (set (mem/v:BLK (0|scratch:DI) [0  A8])
> >                       (unspec:BLK [
> >                               (mem/v:BLK (reuse_rtx 0) [0  A8])
> >                           ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
> >                    (nil))
> 
> LGTM. I'd try to expose match_dup though, it's the standard name for
> this sort of thing. Hopefully it won't have to be added to a lot of
> switch statements to shut up warnings.

The following patch implements the dumping side of the above proposed
format, via the new "rtx_reuse_manager" class.

I didn't expose match_dup to the host, instead introducing "reuse_rtx"
as a generic place for rtx pointer reuse.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu (on top
of "[PATCH] Start adding target-specific selftests").

OK for trunk?

I've (separately) implemented support for loading this format, using
test dumps emitted by this patch and it resolves the issue I had with
the __RTL cc1 selftest.  The testcases for that loading support is
currently integrated with the function_reader code, so I'll save it for
a followup.

gcc/ChangeLog:
	* config/i386/i386.c: Include print-rtl-reuse.h.
	(selftest::ix86_test_dumping_memory_blockage): New function.
	(selftest::ix86_run_selftests): Call it.
	* print-rtl-function.c: Include "print-rtl-reuse.h".
	(print_rtx_function): Create an rtx_reuse_manager and use it.
	* print-rtl-reuse.h: New file.
	* print-rtl.c: Include "print-rtl-reuse.h" and "rtl-iter.h".
	(rtx_reuse_manager::singleton): New global.
	(rtx_reuse_manager::rtx_reuse_manager): New ctor.
	(rtx_reuse_manager::~rtx_reuse_manager): New dtor.
	(uses_rtx_reuse_p): New function.
	(rtx_reuse_manager::preprocess): New function.
	(rtx_reuse_manager::has_reuse_id): New function.
	(rtx_reuse_manager::seen_def_p): New function.
	(rtx_reuse_manager::set_seen_def): New function.
	(print_rtx): If "in_rtx" has a reuse ID, print
	it as a prefix the first time in_rtx is seen, and print
	reuse_rtx subsequently.
	* rtl-tests.c: Include "print-rtl-reuse.h".
	(selftest::test_dumping_rtx_reuse): New function.
	(selftest::rtl_tests_c_tests): Call it.
---
 gcc/config/i386/i386.c   |  23 ++++++++
 gcc/print-rtl-function.c |   6 ++
 gcc/print-rtl-reuse.h    | 100 ++++++++++++++++++++++++++++++++
 gcc/print-rtl.c          | 144 +++++++++++++++++++++++++++++++++++++++++++++--
 gcc/rtl-tests.c          |  49 ++++++++++++++++
 5 files changed, 318 insertions(+), 4 deletions(-)
 create mode 100644 gcc/print-rtl-reuse.h

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8f6ceb4..b979cae 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -79,6 +79,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "dojump.h"
 #include "selftest.h"
 #include "selftest-rtl.h"
+#include "print-rtl-reuse.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -50382,12 +50383,34 @@ ix86_test_dumping_hard_regs ()
   ASSERT_RTL_DUMP_EQ ("(reg:SI dx)", gen_raw_REG (SImode, 1));
 }
 
+/* Test dumping an insn with repeated references to the same SCRATCH,
+   to verify the rtx_reuse code.  */
+
+static void
+ix86_test_dumping_memory_blockage ()
+{
+  rtx pat = gen_memory_blockage ();
+  rtx_reuse_manager r;
+  r.preprocess (pat);
+
+  /* Verify that the repeated references to the SCRATCH show use
+     reuse IDS.  The first should be prefixed with a reuse ID,
+     and the second should be dumped as a "reuse_rtx" of that ID.  */
+  ASSERT_RTL_DUMP_EQ
+    ("(cinsn (set (mem/v:BLK (0|scratch:DI) [0  A8])\n"
+     "        (unspec:BLK [\n"
+     "                (mem/v:BLK (reuse_rtx 0) [0  A8])\n"
+     "            ] UNSPEC_MEMORY_BLOCKAGE))\n"
+     "     (nil))\n", pat);
+}
+
 /* Run all target-specific selftests.  */
 
 static void
 ix86_run_selftests (void)
 {
   ix86_test_dumping_hard_regs ();
+  ix86_test_dumping_memory_blockage ();
 }
 
 } // namespace selftest
diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
index 7ce1b90..d5ab24e 100644
--- a/gcc/print-rtl-function.c
+++ b/gcc/print-rtl-function.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "function.h"
 #include "basic-block.h"
 #include "print-rtl.h"
+#include "print-rtl-reuse.h"
 #include "langhooks.h"
 #include "memmodel.h"
 #include "emit-rtl.h"
@@ -191,6 +192,11 @@ print_rtx_function (FILE *outfile, function *fn, bool compact)
 {
   flag_compact = compact;
 
+  /* Support "reuse_rtx" in the dump.  */
+  rtx_reuse_manager r;
+  for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
+    r.preprocess (insn);
+
   tree fdecl = fn->decl;
 
   const char *dname = lang_hooks.decl_printable_name (fdecl, 2);
diff --git a/gcc/print-rtl-reuse.h b/gcc/print-rtl-reuse.h
new file mode 100644
index 0000000..50eca9d
--- /dev/null
+++ b/gcc/print-rtl-reuse.h
@@ -0,0 +1,100 @@
+/* Track pointer reuse when printing RTL.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_PRINT_RTL_REUSE_H
+#define GCC_PRINT_RTL_REUSE_H
+
+#ifndef GENERATOR_FILE
+
+#include "bitmap.h"
+
+/* For some rtx codes (such as SCRATCH), instances are defined to only be
+   equal for pointer equality: two distinct SCRATCH instances are non-equal.
+   copy_rtx preserves this equality by reusing the SCRATCH instance.
+
+   For example, in this x86 instruction:
+
+      (cinsn (set (mem/v:BLK (scratch:DI) [0  A8])
+                    (unspec:BLK [
+                            (mem/v:BLK (scratch:DI) [0  A8])
+                        ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
+                 (nil))
+
+   the two instances of "(scratch:DI)" are actually the same underlying
+   rtx pointer (and thus "equal"), and the insn will only be recognized
+   (as "*memory_blockage") if this pointer-equality is preserved.
+
+   To be able to preserve this pointer-equality when round-tripping
+   through dumping/loading the rtl, we need some syntax.  The first
+   time a reused rtx is encountered in the dump, we prefix it with
+   a reuse ID:
+
+      (0|scratch:DI)
+
+   Subsequent references to the rtx in the dump can be expressed using
+   "reuse_rtx" e.g.:
+
+      (reuse_rtx 0)
+
+   This class is responsible for tracking a set of reuse IDs during a dump.
+
+   It is a singleton, registering and unregistering itself for use by
+   print_rtx during its lifetime.
+
+   Dumping with reuse-support is done in two passes:
+
+   (a) a first pass in which "preprocess" is called on each top-level rtx
+       to be seen in the dump.  This traverses the rtx and its descendents,
+       identifying rtx that will be seen more than once in the actual dump,
+       and assigning them reuse IDs.
+
+   (b) the actual dump, via print_rtx etc.  print_rtx detect the presence
+       of a live rtx_reuse_manager and uses it if there is one.  Any rtx
+       that were assigned reuse IDs will be printed with it the first time
+       that they are seen, and then printed as "(reuse_rtx ID)" subsequently.
+
+   The first phase is needed since otherwise there would be no way to tell
+   if an rtx will be reused when first encountering it.  */
+
+class rtx_reuse_manager
+{
+ public:
+  rtx_reuse_manager ();
+  ~rtx_reuse_manager ();
+  static rtx_reuse_manager *get () { return singleton; }
+
+  /* The first pass.  */
+  void preprocess (const_rtx x);
+
+  /* The second pass (within print_rtx).  */
+  bool has_reuse_id (const_rtx x, int *out);
+  bool seen_def_p (int reuse_id);
+  void set_seen_def (int reuse_id);
+
+ private:
+  static rtx_reuse_manager *singleton;
+  hash_map<const_rtx, int> m_rtx_occurrence_count;
+  hash_map<const_rtx, int> m_rtx_reuse_ids;
+  bitmap_head m_defs_seen;
+  int m_next_id;
+};
+
+#endif /* #ifndef GENERATOR_FILE */
+
+#endif  // GCC_PRINT_RTL_REUSE_H
diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 46f3c4d..186e6ed 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -51,6 +51,8 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 
 #include "print-rtl.h"
+#include "print-rtl-reuse.h"
+#include "rtl-iter.h"
 
 static FILE *outfile;
 
@@ -93,6 +95,114 @@ int flag_dump_unnumbered_links = 0;
 int flag_simple = 0;
 
 #ifndef GENERATOR_FILE
+
+/* Singleton pointer to the currently-live rtx_reuse_manager instance.  */
+
+rtx_reuse_manager *rtx_reuse_manager::singleton = NULL;
+
+/* rtx_reuse_manager's ctor.  Register this instance with the singleton,
+   and initialize.  */
+
+rtx_reuse_manager::rtx_reuse_manager ()
+: m_next_id (0)
+{
+  gcc_assert (singleton == NULL);
+  singleton = this;
+
+  bitmap_initialize (&m_defs_seen, NULL);
+}
+
+/* rtx_reuse_manager's dtor.  Unregister this instance with the
+   singleton.  */
+
+rtx_reuse_manager::~rtx_reuse_manager ()
+{
+  gcc_assert (singleton == this);
+  singleton = NULL;
+}
+
+/* Determine if X is of a kind suitable for dumping via reuse_rtx.  */
+
+static bool
+uses_rtx_reuse_p (const_rtx x)
+{
+  if (x == NULL)
+    return false;
+
+  switch (GET_CODE (x))
+    {
+    case DEBUG_EXPR:
+    case VALUE:
+    case SCRATCH:
+      return true;
+
+    /* We don't use reuse_rtx for consts.  */
+    CASE_CONST_UNIQUE:
+    default:
+      return false;
+    }
+}
+
+/* Traverse X and its descendents, determining if we see any rtx more than
+   once.  Any rtx suitable for "reuse_rtx" that is seen more than once is
+   assigned an ID.  */
+
+void
+rtx_reuse_manager::preprocess (const_rtx x)
+{
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    if (uses_rtx_reuse_p (*iter))
+      {
+	if (int *count = m_rtx_occurrence_count.get (*iter))
+	  {
+	    if (*count == 1)
+	      {
+		m_rtx_reuse_ids.put (*iter, m_next_id++);
+	      }
+	    (*count)++;
+	  }
+	else
+	  m_rtx_occurrence_count.put (*iter, 1);
+      }
+}
+
+/* Return true iff X has been assigned a reuse ID.  If it has,
+   and OUT is non-NULL, then write the reuse ID to *OUT.  */
+
+bool
+rtx_reuse_manager::has_reuse_id (const_rtx x, int *out)
+{
+  int *id = m_rtx_reuse_ids.get (x);
+  if (id)
+    {
+      if (out)
+	*out = *id;
+      return true;
+    }
+  else
+    return false;
+}
+
+/* Determine if set_seen_def has been called for the given reuse ID.  */
+
+bool
+rtx_reuse_manager::seen_def_p (int reuse_id)
+{
+  return bitmap_bit_p (&m_defs_seen, reuse_id);
+}
+
+/* Record that the definition of the given reuse ID has been seen.  */
+
+void
+rtx_reuse_manager::set_seen_def (int reuse_id)
+{
+  bitmap_set_bit (&m_defs_seen, reuse_id);
+}
+
+#endif /* #ifndef GENERATOR_FILE */
+
+#ifndef GENERATOR_FILE
 void
 print_mem_expr (FILE *outfile, const_tree expr)
 {
@@ -606,8 +716,34 @@ print_rtx (const_rtx in_rtx)
        return;
     }
 
+  fputc ('(', outfile);
+
   /* Print name of expression code.  */
 
+  /* Handle reuse.  */
+#ifndef GENERATOR_FILE
+  if (rtx_reuse_manager::get ())
+    {
+      int reuse_id;
+      if (rtx_reuse_manager::get ()->has_reuse_id (in_rtx, &reuse_id))
+	{
+	  /* Have we already seen the defn of this rtx?  */
+	  if (rtx_reuse_manager::get ()->seen_def_p (reuse_id))
+	    {
+	      fprintf (outfile, "reuse_rtx %i)", reuse_id);
+	      sawclose = 1;
+	      return;
+	    }
+	  else
+	    {
+	      /* First time we've seen this reused-rtx.  */
+	      fprintf (outfile, "%i|", reuse_id);
+	      rtx_reuse_manager::get ()->set_seen_def (reuse_id);
+	    }
+	}
+    }
+#endif /* #ifndef GENERATOR_FILE */
+
   /* In compact mode, prefix the code of insns with "c",
      giving "cinsn", "cnote" etc.  */
   if (flag_compact && is_a <const rtx_insn *, const struct rtx_def> (in_rtx))
@@ -616,14 +752,14 @@ print_rtx (const_rtx in_rtx)
 	 just "clabel".  */
       rtx_code code = GET_CODE (in_rtx);
       if (code == CODE_LABEL)
-	fprintf (outfile, "(clabel");
+	fprintf (outfile, "clabel");
       else
-	fprintf (outfile, "(c%s", GET_RTX_NAME (code));
+	fprintf (outfile, "c%s", GET_RTX_NAME (code));
     }
   else if (flag_simple && CONST_INT_P (in_rtx))
-    fputc ('(', outfile);
+    ; /* no code.  */
   else
-    fprintf (outfile, "(%s", GET_RTX_NAME (GET_CODE (in_rtx)));
+    fprintf (outfile, "%s", GET_RTX_NAME (GET_CODE (in_rtx)));
 
   if (! flag_simple)
     {
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index 10c0ddc..b1182c6 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "pretty-print.h"
 #include "cfgbuild.h"
 #include "print-rtl.h"
+#include "print-rtl-reuse.h"
 #include "selftest.h"
 #include "selftest-rtl.h"
 #include "function.h"
@@ -129,6 +130,53 @@ test_dumping_insns ()
   ASSERT_RTL_DUMP_EQ ("(clabel 0 42 (\"some_label\"))\n", label);
 }
 
+/* Manually exercise the rtx_reuse_manager code.  */
+
+static void
+test_dumping_rtx_reuse ()
+{
+  rtx_reuse_manager r;
+
+  rtx x = rtx_alloc (SCRATCH);
+  rtx y = rtx_alloc (SCRATCH);
+  rtx z = rtx_alloc (SCRATCH);
+
+  /* x and y will be seen more than once.  */
+  r.preprocess (x);
+  r.preprocess (x);
+  r.preprocess (y);
+  r.preprocess (y);
+
+  /* z will be only seen once.  */
+  r.preprocess (z);
+
+  /* Verify that x and y have been assigned reuse IDs.  */
+  int reuse_id_for_x;
+  ASSERT_TRUE (r.has_reuse_id (x, &reuse_id_for_x));
+  ASSERT_EQ (0, reuse_id_for_x);
+
+  int reuse_id_for_y;
+  ASSERT_TRUE (r.has_reuse_id (y, &reuse_id_for_y));
+  ASSERT_EQ (1, reuse_id_for_y);
+
+  /* z is only seen once and thus shouldn't get a reuse ID.  */
+  ASSERT_FALSE (r.has_reuse_id (z, NULL));
+
+  /* The first dumps of x and y should be prefixed by reuse ID;
+     all subsequent dumps of them should show up as "reuse_rtx".  */
+  ASSERT_RTL_DUMP_EQ ("(0|scratch)", x);
+  ASSERT_RTL_DUMP_EQ ("(reuse_rtx 0)", x);
+  ASSERT_RTL_DUMP_EQ ("(reuse_rtx 0)", x);
+
+  ASSERT_RTL_DUMP_EQ ("(1|scratch)", y);
+  ASSERT_RTL_DUMP_EQ ("(reuse_rtx 1)", y);
+  ASSERT_RTL_DUMP_EQ ("(reuse_rtx 1)", y);
+
+  /* z only appears once and thus shouldn't be prefixed with a
+     reuse ID.  */
+  ASSERT_RTL_DUMP_EQ ("(scratch)", z);
+}
+
 /* Unit testing of "single_set".  */
 
 static void
@@ -189,6 +237,7 @@ rtl_tests_c_tests ()
 {
   test_dumping_regs ();
   test_dumping_insns ();
+  test_dumping_rtx_reuse ();
   test_single_set ();
   test_uncond_jump ();
 
-- 
1.8.5.3

Re: [PATCH] print_rtx: implement support for reuse IDs

[Bernd Schmidt]

On 10/21/2016 10:27 PM, David Malcolm wrote:
> Thanks.  I attemped to use those fields of recog_data, but it doesn't
> seem to be exactly what's needed here.

Yeah, I may have been confused. I'm not sure that just looking at 
SCRATCHes is the right thing either, but I think you're on the right 
track, and we can use something like your patch for now and extend it 
later if necessary.

> + public:
> +  rtx_reuse_manager ();
> +  ~rtx_reuse_manager ();
> +  static rtx_reuse_manager *get () { return singleton; }

OTOH, this setup looks a bit odd to me. Are you trying to avoid 
converting the print_rtx stuff to its own class, or avoid passing the 
reuse manager as an argument to a lot of functions?

Some of this setup might not even be necessary. We have a "used" flag on 
rtx objects which is used to unshare RTL, and I think could also be used 
for a similar purpose when dumping. So, before printing, call 
reset_insn_used_flags on everything, then have another pass to set bits 
on everything that could conceivably be shared, and when you find 
something that already has the bit set, enter it into a table. Finally, 
print everything out, using the table. I think this would be somewhat 
simpler than adding another header file and class definition.

> +void
> +rtx_reuse_manager::preprocess (const_rtx x)
> +{
> +  subrtx_iterator::array_type array;
> +  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
> +    if (uses_rtx_reuse_p (*iter))
> +      {
> +	if (int *count = m_rtx_occurrence_count.get (*iter))
> +	  {
> +	    if (*count == 1)
> +	      {
> +		m_rtx_reuse_ids.put (*iter, m_next_id++);
> +	      }
> +	    (*count)++;
> +	  }
> +	else
> +	  m_rtx_occurrence_count.put (*iter, 1);
> +      }

Formatting rules suggest no braces around single statements, I think a 
more readable version of this would be:

   if (uses_rtx_reuse_p (*iter))
     {
       int *count = m_rtx_occurrence_count.get (*iter)
       if (count)
         {
           if ((*count)++ == 1)
             m_rtx_reuse_ids.put (*iter, m_next_id++);
         }
       else
	m_rtx_occurrence_count.put (*iter, 1);
     }


Bernd

[PATCH] Introduce class rtx_writer

[David Malcolm]

On Tue, 2016-10-25 at 14:47 +0200, Bernd Schmidt wrote:
> On 10/21/2016 10:27 PM, David Malcolm wrote:
> > Thanks.  I attemped to use those fields of recog_data, but it
> > doesn't
> > seem to be exactly what's needed here.
> 
> Yeah, I may have been confused. I'm not sure that just looking at
> SCRATCHes is the right thing either, but I think you're on the right
> track, and we can use something like your patch for now and extend it
> later if necessary.
> 
> > + public:
> > +  rtx_reuse_manager ();
> > +  ~rtx_reuse_manager ();
> > +  static rtx_reuse_manager *get () { return singleton; }
> 
> OTOH, this setup looks a bit odd to me. Are you trying to avoid
> converting the print_rtx stuff to its own class, or avoid passing the
> reuse manager as an argument to a lot of functions?

[...snip...]

I attempted to convert the print_rtx stuff to its own class, but ran
into a bug which stumped me for a while, hence I was trying to avoid
the need for it.

I've now fixed that bug.

The following patch moves various global state in print-rtl.c into
a new "rtx_writer" class, giving us a place to stash additional state
relating to dumping (and the possibility of putting extra
setup/cleanup in ctor/dtor).

I didn't bother renaming the variables (e.g. converting "indent" to
"m_indent"), to minimize churn, but I could do that also if you
prefer.

Various functions become methods, but everything labelled as
DEBUG_FUNCTION remains a function after the patch.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

If so, then I can try to rewrite the proposed rtx-reuse code using
rtx_writer.

gcc/ChangeLog:
	* print-rtl-function.c (flag_compact): Delete global.
	(print_rtx_function): Rewrite in terms of class rtx_writer.
	* print-rtl.c (outfile): Delete global.
	(sawclose): Likewise.
	(indent): Likewise.
	(in_call_function_usage): Likewise.
	(flag_compact): Likewise.
	(flag_simple): Likewise.
	(rtx_writer::rtx_writer): New ctor.
	(print_rtx_operand_code_0): Convert to...
	(rtx_writer::print_rtx_operand_code_0): ...this.
	(print_rtx_operand_code_e): Convert to...
	(rtx_writer::print_rtx_operand_code_e): ...this.
	(print_rtx_operand_codes_E_and_V): Convert to...
	(rtx_writer::print_rtx_operand_codes_E_and_V): ...this.
	(print_rtx_operand_code_i): Convert to...
	(rtx_writer::print_rtx_operand_code_i): ...this.
	(print_rtx_operand_code_r): Convert to...
	(rtx_writer::print_rtx_operand_code_r): ...this.
	(print_rtx_operand_code_u): Convert to...
	(rtx_writer::print_rtx_operand_code_u): ...this.
	(print_rtx_operand): Convert to...
	(rtx_writer::print_rtx_operand): ...this.
	(print_rtx): Convert to...
	(rtx_writer::print_rtx): ...this.
	(print_inline_rtx): Rewrite in terms of class rtx_writer.
	(debug_rtx): Likewise.
	(print_rtl): Convert to...
	(rtx_writer::print_rtl): ...this.
	(print_rtl): Reimplement in terms of class rtx_writer.
	(print_rtl_single): Rewrite in terms of class rtx_writer.
	(print_rtl_single_with_indent): Convert to..
	(rtx_writer::print_rtl_single_with_indent): ...this.
	(print_simple_rtl): Rewrite in terms of class rtx_writer.
	* print-rtl.h (flag_compact): Delete decl.
	(class rtx_writer): New class.
---
 gcc/print-rtl-function.c |   8 ++--
 gcc/print-rtl.c          | 121 +++++++++++++++++++++--------------------------
 gcc/print-rtl.h          |  39 ++++++++++++++-
 gcc/rtl-tests.c          |   5 +-
 4 files changed, 98 insertions(+), 75 deletions(-)

diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
index 7ce1b90..f37e1b7 100644
--- a/gcc/print-rtl-function.c
+++ b/gcc/print-rtl-function.c
@@ -189,7 +189,7 @@ can_have_basic_block_p (const rtx_insn *insn)
 DEBUG_FUNCTION void
 print_rtx_function (FILE *outfile, function *fn, bool compact)
 {
-  flag_compact = compact;
+  rtx_writer w (outfile, 0, false, compact);
 
   tree fdecl = fn->decl;
 
@@ -213,7 +213,7 @@ print_rtx_function (FILE *outfile, function *fn, bool compact)
 	  curr_bb = insn_bb;
 	  begin_any_block (outfile, curr_bb);
 	}
-      print_rtl_single_with_indent (outfile, insn, curr_bb ? 6 : 4);
+      w.print_rtl_single_with_indent (insn, curr_bb ? 6 : 4);
     }
   end_any_block (outfile, curr_bb);
   fprintf (outfile, "  ) ;; insn-chain\n");
@@ -221,11 +221,9 @@ print_rtx_function (FILE *outfile, function *fn, bool compact)
   /* Additional RTL state.  */
   fprintf (outfile, "  (crtl\n");
   fprintf (outfile, "    (return_rtx \n");
-  print_rtl_single_with_indent (outfile, crtl->return_rtx, 6);
+  w.print_rtl_single_with_indent (crtl->return_rtx, 6);
   fprintf (outfile, "    ) ;; return_rtx\n");
   fprintf (outfile, "  ) ;; crtl\n");
 
   fprintf (outfile, ") ;; function \"%s\"\n", dname);
-
-  flag_compact = false;
 }
diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 46f3c4d..2625409 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -52,23 +52,6 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "print-rtl.h"
 
-static FILE *outfile;
-
-static int sawclose = 0;
-
-static int indent;
-
-static bool in_call_function_usage;
-
-/* If true, use compact dump format:
-   - INSN_UIDs are omitted, except for jumps and CODE_LABELs,
-   - INSN_CODEs are omitted,
-   - register numbers are omitted for hard and virtual regs
-   - insn names are prefixed with "c" (e.g. "cinsn", "cnote", etc).  */
-bool flag_compact;
-
-static void print_rtx (const_rtx);
-
 /* String printed at beginning of each RTL when it is dumped.
    This string is set to ASM_COMMENT_START when the RTL is dumped in
    the assembly output file.  */
@@ -89,8 +72,13 @@ int flag_dump_unnumbered = 0;
 int flag_dump_unnumbered_links = 0;
 #endif
 
-/* Nonzero means use simplified format without flags, modes, etc.  */
-int flag_simple = 0;
+/* Constructor for rtx_writer.  */
+
+rtx_writer::rtx_writer (FILE *outf, int ind, bool simple, bool compact)
+: outfile (outf), sawclose (0), indent (ind), in_call_function_usage (false),
+  flag_simple (simple), flag_compact (compact)
+{
+}
 
 #ifndef GENERATOR_FILE
 void
@@ -107,9 +95,9 @@ print_mem_expr (FILE *outfile, const_tree expr)
    of a NOTE, where it indicates that the field has several different
    valid contents.  */
 
-static void
-print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
-			  int idx ATTRIBUTE_UNUSED)
+void
+rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
+				      int idx ATTRIBUTE_UNUSED)
 {
 #ifndef GENERATOR_FILE
   if (idx == 1 && GET_CODE (in_rtx) == SYMBOL_REF)
@@ -223,8 +211,8 @@ print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
    Also called by print_rtx_operand_code_u for handling code 'u'
    for LABEL_REFs when they don't reference a CODE_LABEL.  */
 
-static void
-print_rtx_operand_code_e (const_rtx in_rtx, int idx)
+void
+rtx_writer::print_rtx_operand_code_e (const_rtx in_rtx, int idx)
 {
   indent += 2;
   if (idx == 6 && INSN_P (in_rtx))
@@ -246,8 +234,8 @@ print_rtx_operand_code_e (const_rtx in_rtx, int idx)
 
 /* Subroutine of print_rtx_operand for handling codes 'E' and 'V'.  */
 
-static void
-print_rtx_operand_codes_E_and_V (const_rtx in_rtx, int idx)
+void
+rtx_writer::print_rtx_operand_codes_E_and_V (const_rtx in_rtx, int idx)
 {
   indent += 2;
   if (sawclose)
@@ -278,8 +266,8 @@ print_rtx_operand_codes_E_and_V (const_rtx in_rtx, int idx)
 
 /* Subroutine of print_rtx_operand for handling code 'i'.  */
 
-static void
-print_rtx_operand_code_i (const_rtx in_rtx, int idx)
+void
+rtx_writer::print_rtx_operand_code_i (const_rtx in_rtx, int idx)
 {
   if (idx == 4 && INSN_P (in_rtx))
     {
@@ -366,8 +354,8 @@ print_rtx_operand_code_i (const_rtx in_rtx, int idx)
 
 /* Subroutine of print_rtx_operand for handling code 'r'.  */
 
-static void
-print_rtx_operand_code_r (const_rtx in_rtx)
+void
+rtx_writer::print_rtx_operand_code_r (const_rtx in_rtx)
 {
   int is_insn = INSN_P (in_rtx);
   unsigned int regno = REGNO (in_rtx);
@@ -432,8 +420,8 @@ print_rtx_operand_code_r (const_rtx in_rtx)
 
 /* Subroutine of print_rtx_operand for handling code 'u'.  */
 
-static void
-print_rtx_operand_code_u (const_rtx in_rtx, int idx)
+void
+rtx_writer::print_rtx_operand_code_u (const_rtx in_rtx, int idx)
 {
   /* Don't print insn UIDs in compact mode, apart from in LABEL_REFs.  */
   if (flag_compact && GET_CODE (in_rtx) != LABEL_REF)
@@ -479,8 +467,8 @@ print_rtx_operand_code_u (const_rtx in_rtx, int idx)
 
 /* Subroutine of print_rtx.   Print operand IDX of IN_RTX.  */
 
-static void
-print_rtx_operand (const_rtx in_rtx, int idx)
+void
+rtx_writer::print_rtx_operand (const_rtx in_rtx, int idx)
 {
   const char *format_ptr = GET_RTX_FORMAT (GET_CODE (in_rtx));
 
@@ -578,8 +566,8 @@ print_rtx_operand (const_rtx in_rtx, int idx)
 
 /* Print IN_RTX onto OUTFILE.  This is the recursive part of printing.  */
 
-static void
-print_rtx (const_rtx in_rtx)
+void
+rtx_writer::print_rtx (const_rtx in_rtx)
 {
   int idx = 0;
 
@@ -778,15 +766,8 @@ print_rtx (const_rtx in_rtx)
 void
 print_inline_rtx (FILE *outf, const_rtx x, int ind)
 {
-  int oldsaw = sawclose;
-  int oldindent = indent;
-
-  sawclose = 0;
-  indent = ind;
-  outfile = outf;
-  print_rtx (x);
-  sawclose = oldsaw;
-  indent = oldindent;
+  rtx_writer w (outf, ind, false, false);
+  w.print_rtx (x);
 }
 
 /* Call this function from the debugger to see what X looks like.  */
@@ -794,9 +775,8 @@ print_inline_rtx (FILE *outf, const_rtx x, int ind)
 DEBUG_FUNCTION void
 debug_rtx (const_rtx x)
 {
-  outfile = stderr;
-  sawclose = 0;
-  print_rtx (x);
+  rtx_writer w (stderr, 0, false, false);
+  w.print_rtx (x);
   fprintf (stderr, "\n");
 }
 
@@ -892,23 +872,20 @@ debug_rtx_find (const rtx_insn *x, int uid)
 }
 
 /* External entry point for printing a chain of insns
-   starting with RTX_FIRST onto file OUTF.
+   starting with RTX_FIRST.
    A blank line separates insns.
 
    If RTX_FIRST is not an insn, then it alone is printed, with no newline.  */
 
 void
-print_rtl (FILE *outf, const_rtx rtx_first)
+rtx_writer::print_rtl (const_rtx rtx_first)
 {
   const rtx_insn *tmp_rtx;
 
-  outfile = outf;
-  sawclose = 0;
-
   if (rtx_first == 0)
     {
-      fputs (print_rtx_head, outf);
-      fputs ("(nil)\n", outf);
+      fputs (print_rtx_head, outfile);
+      fputs ("(nil)\n", outfile);
     }
   else
     switch (GET_CODE (rtx_first))
@@ -936,32 +913,45 @@ print_rtl (FILE *outf, const_rtx rtx_first)
       }
 }
 
+/* External entry point for printing a chain of insns
+   starting with RTX_FIRST onto file OUTF.
+   A blank line separates insns.
+
+   If RTX_FIRST is not an insn, then it alone is printed, with no newline.  */
+
+void
+print_rtl (FILE *outf, const_rtx rtx_first)
+{
+  rtx_writer w (outf, 0, false, false);
+  w.print_rtl (rtx_first);
+}
+
 /* Like print_rtx, except specify a file.  */
 /* Return nonzero if we actually printed anything.  */
 
 int
 print_rtl_single (FILE *outf, const_rtx x)
 {
-  return print_rtl_single_with_indent (outf, x, 0);
+  rtx_writer w (outf, 0, false, false);
+  return w.print_rtl_single_with_indent (x, 0);
 }
 
-/* Like print_rtl_single, except specify a file and indentation.  */
+/* Like print_rtl_single, except specify an indentation.  */
 
 int
-print_rtl_single_with_indent (FILE *outf, const_rtx x, int ind)
+rtx_writer::print_rtl_single_with_indent (const_rtx x, int ind)
 {
-  int old_indent = indent;
   char *s_indent = (char *) alloca ((size_t) ind + 1);
   memset ((void *) s_indent, ' ', (size_t) ind);
   s_indent[ind] = '\0';
+  fputs (s_indent, outfile);
+  fputs (print_rtx_head, outfile);
 
+  int old_indent = indent;
   indent = ind;
-  outfile = outf;
   sawclose = 0;
-  fputs (s_indent, outfile);
-  fputs (print_rtx_head, outfile);
   print_rtx (x);
-  putc ('\n', outf);
+  putc ('\n', outfile);
   indent = old_indent;
   return 1;
 }
@@ -973,9 +963,8 @@ print_rtl_single_with_indent (FILE *outf, const_rtx x, int ind)
 void
 print_simple_rtl (FILE *outf, const_rtx x)
 {
-  flag_simple = 1;
-  print_rtl (outf, x);
-  flag_simple = 0;
+  rtx_writer w (outf, 0, true, false);
+  w.print_rtl (x);
 }
 
 /* Print the elements of VEC to FILE.  */
diff --git a/gcc/print-rtl.h b/gcc/print-rtl.h
index 8dfba8b..a7a63c7 100644
--- a/gcc/print-rtl.h
+++ b/gcc/print-rtl.h
@@ -20,7 +20,44 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_PRINT_RTL_H
 #define GCC_PRINT_RTL_H
 
-extern bool flag_compact;
+/* A class for writing rtx to a FILE *.  */
+
+class rtx_writer
+{
+ public:
+  rtx_writer (FILE *outfile, int ind, bool simple, bool compact);
+
+  void print_rtx (const_rtx in_rtx);
+  void print_rtl (const_rtx rtx_first);
+  int print_rtl_single_with_indent (const_rtx x, int ind);
+
+ private:
+  void print_rtx_operand_code_0 (const_rtx in_rtx, int idx);
+  void print_rtx_operand_code_e (const_rtx in_rtx, int idx);
+  void print_rtx_operand_codes_E_and_V (const_rtx in_rtx, int idx);
+  void print_rtx_operand_code_i (const_rtx in_rtx, int idx);
+  void print_rtx_operand_code_r (const_rtx in_rtx);
+  void print_rtx_operand_code_u (const_rtx in_rtx, int idx);
+  void print_rtx_operand (const_rtx in_rtx, int idx);
+
+ private:
+  FILE *outfile;
+  int sawclose;
+  int indent;
+  bool in_call_function_usage;
+
+  /* True means use simplified format without flags, modes, etc.  */
+  bool flag_simple;
+
+  /* If true, use compact dump format:
+     - INSN_UIDs are omitted, except for jumps and CODE_LABELs,
+     - INSN_CODEs are omitted,
+     - register numbers are omitted for hard and virtual regs, and
+       non-virtual pseudos are offset relative to the first such reg, and
+       printed with a '%' sigil e.g. "%0" for (LAST_VIRTUAL_REGISTER + 1),
+     - insn names are prefixed with "c" (e.g. "cinsn", "cnote", etc).  */
+  bool flag_compact;
+};
 
 #ifdef BUFSIZ
 extern void print_rtl (FILE *, const_rtx);
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index b723560..36f423b 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -65,9 +65,8 @@ assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x)
 {
   named_temp_file tmp_out (".rtl");
   FILE *outfile = fopen (tmp_out.get_filename (), "w");
-  flag_compact = true;
-  print_rtl (outfile, x);
-  flag_compact = false;
+  rtx_writer w (outfile, 0, false, true);
+  w.print_rtl (x);
   fclose (outfile);
 
   char *dump = read_file (SELFTEST_LOCATION, tmp_out.get_filename ());
-- 
1.8.5.3

Re: [PATCH] Introduce class rtx_writer

[Bernd Schmidt]

On 10/26/2016 04:10 PM, David Malcolm wrote:
> The following patch moves various global state in print-rtl.c into
> a new "rtx_writer" class, giving us a place to stash additional state
> relating to dumping (and the possibility of putting extra
> setup/cleanup in ctor/dtor).
>
> I didn't bother renaming the variables (e.g. converting "indent" to
> "m_indent"), to minimize churn, but I could do that also if you
> prefer.

I do like avoiding churn, but at the same time - our guidelines suggest 
that we name members with m_. So the patch is OK if you do the conversion.


Bernd

[PATCH] print_rtx: implement support for reuse IDs (v2)

[David Malcolm]

On Tue, 2016-10-25 at 14:47 +0200, Bernd Schmidt wrote:
> On 10/21/2016 10:27 PM, David Malcolm wrote:
> > Thanks.  I attemped to use those fields of recog_data, but it
> > doesn't
> > seem to be exactly what's needed here.
> 
> Yeah, I may have been confused. I'm not sure that just looking at
> SCRATCHes is the right thing either, but I think you're on the right
> track, and we can use something like your patch for now and extend it
> later if necessary.
> 
> > + public:
> > +  rtx_reuse_manager ();
> > +  ~rtx_reuse_manager ();
> > +  static rtx_reuse_manager *get () { return singleton; }
> 
> OTOH, this setup looks a bit odd to me. Are you trying to avoid
> converting the print_rtx stuff to its own class, or avoid passing the
> reuse manager as an argument to a lot of functions?
>
> Some of this setup might not even be necessary. We have a "used" flag
> on
> rtx objects which is used to unshare RTL, and I think could also be
> used
> for a similar purpose when dumping. So, before printing, call
> reset_insn_used_flags on everything, then have another pass to set
> bits
> on everything that could conceivably be shared, and when you find
> something that already has the bit set, enter it into a table.
> Finally,
> print everything out, using the table. I think this would be somewhat
> simpler than adding another header file and class definition.

Now that we have a class rtx_writer, it's much clearer to drop the
singleton.

In this version I've eliminated the rtx_reuse_manager singleton,
instead allowing callers to pass a rtx_reuse_manager * to
rtx_writer's ctor.  This can be NULL, allowing most dumps to opt
out of the reuse-tracking, minimizing the risk of changing an
existing testcase; only print_rtl_function makes use of it (and
the selftests).

I eliminated print-rtl-reuse.h, moving class rtx_reuse_manager into
print-rtl.h and print-rtl.c

I kept the class rtx_reuse_manager, as it seems appropriate to
put responsibility for this aspect of dumping into its own class.
I attempted to move it into rtx_writer itself, but doing so made
the code less clear.
 
> > +void
> > +rtx_reuse_manager::preprocess (const_rtx x)
> > +{
> > +  subrtx_iterator::array_type array;
> > +  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
> > +    if (uses_rtx_reuse_p (*iter))
> > +      {
> > +	if (int *count = m_rtx_occurrence_count.get (*iter))
> > +	  {
> > +	    if (*count == 1)
> > +	      {
> > +		m_rtx_reuse_ids.put (*iter, m_next_id++);
> > +	      }
> > +	    (*count)++;
> > +	  }
> > +	else
> > +	  m_rtx_occurrence_count.put (*iter, 1);
> > +      }
> 
> Formatting rules suggest no braces around single statements, I think
> a
> more readable version of this would be:
> 
>    if (uses_rtx_reuse_p (*iter))
>      {
>        int *count = m_rtx_occurrence_count.get (*iter)
>        if (count)
>          {
>            if ((*count)++ == 1)
>              m_rtx_reuse_ids.put (*iter, m_next_id++);
>          }
>        else
> 	m_rtx_occurrence_count.put (*iter, 1);
>      }
> 
> 
> Bernd

Fixed in the way you you noted.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
	* config/i386/i386.c: Include print-rtl.h.
	(selftest::ix86_test_dumping_memory_blockage): New function.
	(selftest::ix86_run_selftests): Call it.
	* print-rtl-function.c (print_rtx_function): Create an
	rtx_reuse_manager and use it.
	* print-rtl.c: Include "rtl-iter.h".
	(rtx_writer::rtx_writer): Add reuse_manager param.
	(rtx_reuse_manager::rtx_reuse_manager): New ctor.
	(uses_rtx_reuse_p): New function.
	(rtx_reuse_manager::preprocess): New function.
	(rtx_reuse_manager::has_reuse_id): New function.
	(rtx_reuse_manager::seen_def_p): New function.
	(rtx_reuse_manager::set_seen_def): New function.
	(rtx_writer::print_rtx): If "in_rtx" has a reuse ID, print it as a
	prefix the first time in_rtx is seen, and print reuse_rtx
	subsequently.
	(print_inline_rtx): Supply NULL for new reuse_manager param.
	(debug_rtx): Likewise.
	(print_rtl): Likewise.
	(print_rtl_single): Likewise.
	(rtx_writer::print_rtl_single_with_indent): Likewise.
	* print-rtl.h: Include bitmap.h when building for host.
	(rtx_writer::rtx_writer): Add reuse_manager param.
	(rtx_writer::m_rtx_reuse_manager): New field.
	(class rtx_reuse_manager): New class.
	* rtl-tests.c (selftest::assert_rtl_dump_eq): Add reuse_manager
	param and use it when constructing rtx_writer.
	(selftest::test_dumping_rtx_reuse): New function.
	(selftest::rtl_tests_c_tests): Call it.
	* selftest-rtl.h (class rtx_reuse_manager): New forward decl.
	(selftest::assert_rtl_dump_eq): Add reuse_manager param.
	(ASSERT_RTL_DUMP_EQ): Supply NULL for reuse_manager param.
	(ASSERT_RTL_DUMP_EQ_WITH_REUSE): New macro.
---
 gcc/config/i386/i386.c   |  24 ++++++++
 gcc/print-rtl-function.c |   7 ++-
 gcc/print-rtl.c          | 139 +++++++++++++++++++++++++++++++++++++++++++----
 gcc/print-rtl.h          |  81 ++++++++++++++++++++++++++-
 gcc/rtl-tests.c          |  53 +++++++++++++++++-
 gcc/selftest-rtl.h       |  13 ++++-
 6 files changed, 300 insertions(+), 17 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index bffba80..7c4f0a3 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -82,6 +82,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssanames.h"
 #include "selftest.h"
 #include "selftest-rtl.h"
+#include "print-rtl.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -50627,12 +50628,35 @@ ix86_test_dumping_hard_regs ()
   ASSERT_RTL_DUMP_EQ ("(reg:SI dx)", gen_raw_REG (SImode, 1));
 }
 
+/* Test dumping an insn with repeated references to the same SCRATCH,
+   to verify the rtx_reuse code.  */
+
+static void
+ix86_test_dumping_memory_blockage ()
+{
+  set_new_first_and_last_insn (NULL, NULL);
+
+  rtx pat = gen_memory_blockage ();
+  rtx_reuse_manager r;
+  r.preprocess (pat);
+
+  /* Verify that the repeated references to the SCRATCH show
+     reuse IDs.  The first should be prefixed with a reuse ID,
+     and the second should be dumped as a "reuse_rtx" of that ID.  */
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE
+    ("(cinsn 1 (set (mem/v:BLK (0|scratch:DI) [0  A8])\n"
+     "        (unspec:BLK [\n"
+     "                (mem/v:BLK (reuse_rtx 0) [0  A8])\n"
+     "            ] UNSPEC_MEMORY_BLOCKAGE)))\n", pat, &r);
+}
+
 /* Run all target-specific selftests.  */
 
 static void
 ix86_run_selftests (void)
 {
   ix86_test_dumping_hard_regs ();
+  ix86_test_dumping_memory_blockage ();
 }
 
 } // namespace selftest
diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
index f37e1b7..b62f1b3 100644
--- a/gcc/print-rtl-function.c
+++ b/gcc/print-rtl-function.c
@@ -189,7 +189,12 @@ can_have_basic_block_p (const rtx_insn *insn)
 DEBUG_FUNCTION void
 print_rtx_function (FILE *outfile, function *fn, bool compact)
 {
-  rtx_writer w (outfile, 0, false, compact);
+  rtx_reuse_manager r;
+  rtx_writer w (outfile, 0, false, compact, &r);
+
+  /* Support "reuse_rtx" in the dump.  */
+  for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
+    r.preprocess (insn);
 
   tree fdecl = fn->decl;
 
diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 3f15a21..30ff8fa 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 
 #include "print-rtl.h"
+#include "rtl-iter.h"
 
 /* String printed at beginning of each RTL when it is dumped.
    This string is set to ASM_COMMENT_START when the RTL is dumped in
@@ -74,13 +75,103 @@ int flag_dump_unnumbered_links = 0;
 
 /* Constructor for rtx_writer.  */
 
-rtx_writer::rtx_writer (FILE *outf, int ind, bool simple, bool compact)
+rtx_writer::rtx_writer (FILE *outf, int ind, bool simple, bool compact,
+			rtx_reuse_manager *reuse_manager)
 : m_outfile (outf), m_sawclose (0), m_indent (ind),
-  m_in_call_function_usage (false), m_simple (simple), m_compact (compact)
+  m_in_call_function_usage (false), m_simple (simple), m_compact (compact),
+  m_rtx_reuse_manager (reuse_manager)
 {
 }
 
 #ifndef GENERATOR_FILE
+
+/* rtx_reuse_manager's ctor.  */
+
+rtx_reuse_manager::rtx_reuse_manager ()
+: m_next_id (0)
+{
+  bitmap_initialize (&m_defs_seen, NULL);
+}
+
+/* Determine if X is of a kind suitable for dumping via reuse_rtx.  */
+
+static bool
+uses_rtx_reuse_p (const_rtx x)
+{
+  if (x == NULL)
+    return false;
+
+  switch (GET_CODE (x))
+    {
+    case DEBUG_EXPR:
+    case VALUE:
+    case SCRATCH:
+      return true;
+
+    /* We don't use reuse_rtx for consts.  */
+    CASE_CONST_UNIQUE:
+    default:
+      return false;
+    }
+}
+
+/* Traverse X and its descendents, determining if we see any rtx more than
+   once.  Any rtx suitable for "reuse_rtx" that is seen more than once is
+   assigned an ID.  */
+
+void
+rtx_reuse_manager::preprocess (const_rtx x)
+{
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    if (uses_rtx_reuse_p (*iter))
+      {
+	if (int *count = m_rtx_occurrence_count.get (*iter))
+	  {
+	    if (*(count++) == 1)
+	      m_rtx_reuse_ids.put (*iter, m_next_id++);
+	  }
+	else
+	  m_rtx_occurrence_count.put (*iter, 1);
+      }
+}
+
+/* Return true iff X has been assigned a reuse ID.  If it has,
+   and OUT is non-NULL, then write the reuse ID to *OUT.  */
+
+bool
+rtx_reuse_manager::has_reuse_id (const_rtx x, int *out)
+{
+  int *id = m_rtx_reuse_ids.get (x);
+  if (id)
+    {
+      if (out)
+	*out = *id;
+      return true;
+    }
+  else
+    return false;
+}
+
+/* Determine if set_seen_def has been called for the given reuse ID.  */
+
+bool
+rtx_reuse_manager::seen_def_p (int reuse_id)
+{
+  return bitmap_bit_p (&m_defs_seen, reuse_id);
+}
+
+/* Record that the definition of the given reuse ID has been seen.  */
+
+void
+rtx_reuse_manager::set_seen_def (int reuse_id)
+{
+  bitmap_set_bit (&m_defs_seen, reuse_id);
+}
+
+#endif /* #ifndef GENERATOR_FILE */
+
+#ifndef GENERATOR_FILE
 void
 print_mem_expr (FILE *outfile, const_tree expr)
 {
@@ -631,8 +722,34 @@ rtx_writer::print_rtx (const_rtx in_rtx)
        return;
     }
 
+  fputc ('(', m_outfile);
+
   /* Print name of expression code.  */
 
+  /* Handle reuse.  */
+#ifndef GENERATOR_FILE
+  if (m_rtx_reuse_manager)
+    {
+      int reuse_id;
+      if (m_rtx_reuse_manager->has_reuse_id (in_rtx, &reuse_id))
+	{
+	  /* Have we already seen the defn of this rtx?  */
+	  if (m_rtx_reuse_manager->seen_def_p (reuse_id))
+	    {
+	      fprintf (m_outfile, "reuse_rtx %i)", reuse_id);
+	      m_sawclose = 1;
+	      return;
+	    }
+	  else
+	    {
+	      /* First time we've seen this reused-rtx.  */
+	      fprintf (m_outfile, "%i|", reuse_id);
+	      m_rtx_reuse_manager->set_seen_def (reuse_id);
+	    }
+	}
+    }
+#endif /* #ifndef GENERATOR_FILE */
+
   /* In compact mode, prefix the code of insns with "c",
      giving "cinsn", "cnote" etc.  */
   if (m_compact && is_a <const rtx_insn *, const struct rtx_def> (in_rtx))
@@ -641,14 +758,14 @@ rtx_writer::print_rtx (const_rtx in_rtx)
 	 just "clabel".  */
       rtx_code code = GET_CODE (in_rtx);
       if (code == CODE_LABEL)
-	fprintf (m_outfile, "(clabel");
+	fprintf (m_outfile, "clabel");
       else
-	fprintf (m_outfile, "(c%s", GET_RTX_NAME (code));
+	fprintf (m_outfile, "c%s", GET_RTX_NAME (code));
     }
   else if (m_simple && CONST_INT_P (in_rtx))
-    fputc ('(', m_outfile);
+    ; /* no code.  */
   else
-    fprintf (m_outfile, "(%s", GET_RTX_NAME (GET_CODE (in_rtx)));
+    fprintf (m_outfile, "%s", GET_RTX_NAME (GET_CODE (in_rtx)));
 
   if (! m_simple)
     {
@@ -810,7 +927,7 @@ rtx_writer::print_rtx (const_rtx in_rtx)
 void
 print_inline_rtx (FILE *outf, const_rtx x, int ind)
 {
-  rtx_writer w (outf, ind, false, false);
+  rtx_writer w (outf, ind, false, false, NULL);
   w.print_rtx (x);
 }
 
@@ -819,7 +936,7 @@ print_inline_rtx (FILE *outf, const_rtx x, int ind)
 DEBUG_FUNCTION void
 debug_rtx (const_rtx x)
 {
-  rtx_writer w (stderr, 0, false, false);
+  rtx_writer w (stderr, 0, false, false, NULL);
   w.print_rtx (x);
   fprintf (stderr, "\n");
 }
@@ -966,7 +1083,7 @@ rtx_writer::print_rtl (const_rtx rtx_first)
 void
 print_rtl (FILE *outf, const_rtx rtx_first)
 {
-  rtx_writer w (outf, 0, false, false);
+  rtx_writer w (outf, 0, false, false, NULL);
   w.print_rtl (rtx_first);
 }
 
@@ -976,7 +1093,7 @@ print_rtl (FILE *outf, const_rtx rtx_first)
 int
 print_rtl_single (FILE *outf, const_rtx x)
 {
-  rtx_writer w (outf, 0, false, false);
+  rtx_writer w (outf, 0, false, false, NULL);
   return w.print_rtl_single_with_indent (x, 0);
 }
 
@@ -1007,7 +1124,7 @@ rtx_writer::print_rtl_single_with_indent (const_rtx x, int ind)
 void
 print_simple_rtl (FILE *outf, const_rtx x)
 {
-  rtx_writer w (outf, 0, true, false);
+  rtx_writer w (outf, 0, true, false, NULL);
   w.print_rtl (x);
 }
 
diff --git a/gcc/print-rtl.h b/gcc/print-rtl.h
index 68db057..3e5a975 100644
--- a/gcc/print-rtl.h
+++ b/gcc/print-rtl.h
@@ -20,12 +20,19 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_PRINT_RTL_H
 #define GCC_PRINT_RTL_H
 
+#ifndef GENERATOR_FILE
+#include "bitmap.h"
+#endif /* #ifndef GENERATOR_FILE */
+
+class rtx_reuse_manager;
+
 /* A class for writing rtx to a FILE *.  */
 
 class rtx_writer
 {
  public:
-  rtx_writer (FILE *outfile, int ind, bool simple, bool compact);
+  rtx_writer (FILE *outfile, int ind, bool simple, bool compact,
+	      rtx_reuse_manager *reuse_manager);
 
   void print_rtx (const_rtx in_rtx);
   void print_rtl (const_rtx rtx_first);
@@ -58,6 +65,9 @@ class rtx_writer
        printed with a '%' sigil e.g. "%0" for (LAST_VIRTUAL_REGISTER + 1),
      - insn names are prefixed with "c" (e.g. "cinsn", "cnote", etc).  */
   bool m_compact;
+
+  /* An optional instance of rtx_reuse_manager.  */
+  rtx_reuse_manager *m_rtx_reuse_manager;
 };
 
 #ifdef BUFSIZ
@@ -78,4 +88,73 @@ extern const char *str_pattern_slim (const_rtx);
 
 extern void print_rtx_function (FILE *file, function *fn, bool compact);
 
+#ifndef GENERATOR_FILE
+
+/* For some rtx codes (such as SCRATCH), instances are defined to only be
+   equal for pointer equality: two distinct SCRATCH instances are non-equal.
+   copy_rtx preserves this equality by reusing the SCRATCH instance.
+
+   For example, in this x86 instruction:
+
+      (cinsn (set (mem/v:BLK (scratch:DI) [0  A8])
+                    (unspec:BLK [
+                            (mem/v:BLK (scratch:DI) [0  A8])
+                        ] UNSPEC_MEMORY_BLOCKAGE)) "test.c":2
+                 (nil))
+
+   the two instances of "(scratch:DI)" are actually the same underlying
+   rtx pointer (and thus "equal"), and the insn will only be recognized
+   (as "*memory_blockage") if this pointer-equality is preserved.
+
+   To be able to preserve this pointer-equality when round-tripping
+   through dumping/loading the rtl, we need some syntax.  The first
+   time a reused rtx is encountered in the dump, we prefix it with
+   a reuse ID:
+
+      (0|scratch:DI)
+
+   Subsequent references to the rtx in the dump can be expressed using
+   "reuse_rtx" e.g.:
+
+      (reuse_rtx 0)
+
+   This class is responsible for tracking a set of reuse IDs during a dump.
+
+   Dumping with reuse-support is done in two passes:
+
+   (a) a first pass in which "preprocess" is called on each top-level rtx
+       to be seen in the dump.  This traverses the rtx and its descendents,
+       identifying rtx that will be seen more than once in the actual dump,
+       and assigning them reuse IDs.
+
+   (b) the actual dump, via print_rtx etc.  print_rtx detect the presence
+       of a live rtx_reuse_manager and uses it if there is one.  Any rtx
+       that were assigned reuse IDs will be printed with it the first time
+       that they are seen, and then printed as "(reuse_rtx ID)" subsequently.
+
+   The first phase is needed since otherwise there would be no way to tell
+   if an rtx will be reused when first encountering it.  */
+
+class rtx_reuse_manager
+{
+ public:
+  rtx_reuse_manager ();
+
+  /* The first pass.  */
+  void preprocess (const_rtx x);
+
+  /* The second pass (within print_rtx).  */
+  bool has_reuse_id (const_rtx x, int *out);
+  bool seen_def_p (int reuse_id);
+  void set_seen_def (int reuse_id);
+
+ private:
+  hash_map<const_rtx, int> m_rtx_occurrence_count;
+  hash_map<const_rtx, int> m_rtx_reuse_ids;
+  bitmap_head m_defs_seen;
+  int m_next_id;
+};
+
+#endif /* #ifndef GENERATOR_FILE */
+
 #endif  // GCC_PRINT_RTL_H
diff --git a/gcc/rtl-tests.c b/gcc/rtl-tests.c
index 228226b..8edddfb 100644
--- a/gcc/rtl-tests.c
+++ b/gcc/rtl-tests.c
@@ -62,11 +62,12 @@ verify_print_pattern (const char *expected, rtx pat)
    Use LOC as the effective location when reporting errors.  */
 
 void
-assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x)
+assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x,
+		    rtx_reuse_manager *reuse_manager)
 {
   named_temp_file tmp_out (".rtl");
   FILE *outfile = fopen (tmp_out.get_filename (), "w");
-  rtx_writer w (outfile, 0, false, true);
+  rtx_writer w (outfile, 0, false, true, reuse_manager);
   w.print_rtl (x);
   fclose (outfile);
 
@@ -128,6 +129,53 @@ test_dumping_insns ()
   ASSERT_RTL_DUMP_EQ ("(clabel 0 42 (\"some_label\"))\n", label);
 }
 
+/* Manually exercise the rtx_reuse_manager code.  */
+
+static void
+test_dumping_rtx_reuse ()
+{
+  rtx_reuse_manager r;
+
+  rtx x = rtx_alloc (SCRATCH);
+  rtx y = rtx_alloc (SCRATCH);
+  rtx z = rtx_alloc (SCRATCH);
+
+  /* x and y will be seen more than once.  */
+  r.preprocess (x);
+  r.preprocess (x);
+  r.preprocess (y);
+  r.preprocess (y);
+
+  /* z will be only seen once.  */
+  r.preprocess (z);
+
+  /* Verify that x and y have been assigned reuse IDs.  */
+  int reuse_id_for_x;
+  ASSERT_TRUE (r.has_reuse_id (x, &reuse_id_for_x));
+  ASSERT_EQ (0, reuse_id_for_x);
+
+  int reuse_id_for_y;
+  ASSERT_TRUE (r.has_reuse_id (y, &reuse_id_for_y));
+  ASSERT_EQ (1, reuse_id_for_y);
+
+  /* z is only seen once and thus shouldn't get a reuse ID.  */
+  ASSERT_FALSE (r.has_reuse_id (z, NULL));
+
+  /* The first dumps of x and y should be prefixed by reuse ID;
+     all subsequent dumps of them should show up as "reuse_rtx".  */
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(0|scratch)", x, &r);
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(reuse_rtx 0)", x, &r);
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(reuse_rtx 0)", x, &r);
+
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(1|scratch)", y, &r);
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(reuse_rtx 1)", y, &r);
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(reuse_rtx 1)", y, &r);
+
+  /* z only appears once and thus shouldn't be prefixed with a
+     reuse ID.  */
+  ASSERT_RTL_DUMP_EQ_WITH_REUSE ("(scratch)", z, &r);
+}
+
 /* Unit testing of "single_set".  */
 
 static void
@@ -187,6 +235,7 @@ rtl_tests_c_tests ()
 {
   test_dumping_regs ();
   test_dumping_insns ();
+  test_dumping_rtx_reuse ();
   test_single_set ();
   test_uncond_jump ();
 
diff --git a/gcc/selftest-rtl.h b/gcc/selftest-rtl.h
index 0f0e167..f505018 100644
--- a/gcc/selftest-rtl.h
+++ b/gcc/selftest-rtl.h
@@ -25,18 +25,27 @@ along with GCC; see the file COPYING3.  If not see
 
 #if CHECKING_P
 
+class rtx_reuse_manager;
+
 namespace selftest {
 
 /* Verify that X is dumped as EXPECTED_DUMP, using compact mode.
    Use LOC as the effective location when reporting errors.  */
 
 extern void
-assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x);
+assert_rtl_dump_eq (const location &loc, const char *expected_dump, rtx x,
+		    rtx_reuse_manager *reuse_manager);
 
 /* Verify that RTX is dumped as EXPECTED_DUMP, using compact mode.  */
 
 #define ASSERT_RTL_DUMP_EQ(EXPECTED_DUMP, RTX) \
-  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX))
+  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX), NULL)
+
+/* As above, but using REUSE_MANAGER when dumping.  */
+
+#define ASSERT_RTL_DUMP_EQ_WITH_REUSE(EXPECTED_DUMP, RTX, REUSE_MANAGER) \
+  assert_rtl_dump_eq (SELFTEST_LOCATION, (EXPECTED_DUMP), (RTX), \
+		      (REUSE_MANAGER))
 
 } /* end of namespace selftest.  */
 
-- 
1.8.5.3