Tweaks to print_rtx_function

On Wed, 2016-10-12 at 19:31 +0200, Bernd Schmidt wrote:
> On 10/12/2016 07:48 PM, David Malcolm wrote:

[...snip...]

> > I think the only remaining item from our discussion above is what
> > to do
> > about the numbering of pseudos in the dumps (currently it just
> > prints the regno
> > unmodified).
> >
> > Other than that, is the resultant dump format good enough that I
> > can start
> > rewriting the RTL frontend parser, or are there other changes you'd
> > want?
>
> Give me a day or two to think it over, and for others to chime in.
> But I
> think this is reasonably close to what it should look like. Maybe
> empty
> edge flags don't need to be printed, and possibly there could be a
> more
> compact format for a large number edges like what you have for the
> switch?

[...snip...]

This patch implements:
* the renumbering of non-virtual pseudos, using
  LAST_VIRTUAL_REGISTER + 1 as a base.
* omitting the edge "(flags)" directive if there aren't any

Bootstrap & regrtest in progress.

OK for trunk if they pass?

gcc/ChangeLog:
	* print-rtl-function.c (print_edge): Omit "(flags)" when none are
	set.
	* print-rtl.c (print_rtx_operand_code_r): In compact mode, print
	pseudos offset by (LAST_VIRTUAL_REGISTER + 1), so that the first
	non-virtual pseudo is 0.
---
 gcc/print-rtl-function.c | 13 +++++++++----
 gcc/print-rtl.c          |  8 ++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

On 10/12/2016 11:04 PM, David Malcolm wrote:
>
> This patch implements:
> * the renumbering of non-virtual pseudos, using
>   LAST_VIRTUAL_REGISTER + 1 as a base.
> * omitting the edge "(flags)" directive if there aren't any
>
> Bootstrap & regrtest in progress.
>
> OK for trunk if they pass?

I tend to think probably yes. Let's say ok if I don't object by tomorrow 
:) I'm still wondering whether we want to use some sort of prefix like 
$p or %p which is distinct from any hard register name for clarity.


Bernd

On Thu, 2016-10-13 at 12:24 +0200, Bernd Schmidt wrote:
> On 10/12/2016 11:04 PM, David Malcolm wrote:
> > 
> > This patch implements:
> > * the renumbering of non-virtual pseudos, using
> >   LAST_VIRTUAL_REGISTER + 1 as a base.
> > * omitting the edge "(flags)" directive if there aren't any
> > 
> > Bootstrap & regrtest in progress.
> > 
> > OK for trunk if they pass?

(they did)

> I tend to think probably yes. Let's say ok if I don't object by
> tomorrow 
> :) I'm still wondering whether we want to use some sort of prefix
> like 
> $p or %p which is distinct from any hard register name for clarity.

I'll attempt to sum up the discussion on this so far...

We're already printing names for hard regs and for virtual regs, so
it's non-virtual pseudos that need figuring out.


Consider regno == 90 for target x86_64, where LAST_VIRTUAL_REGISTER is
86 and hence this is a non-virtual pseudo.  The first non-virtual
pseudo has regno 87.

Currently this is printed in non-compact form as:

  (reg:SI 90)

Our goals for printing it in "compact" form (and hence parsed by the
RTL frontend) are (in no particular order):

* readability for humans
* lack of ambiguity for parsers
* "hackability" for humans editing the file
* future-proof against changes to the machine description
* make it reasonably target-independent

I thought it might be useful to brainstorm [1] some ideas on this, so here are various possible ways it could be printed for this use-case:

* Offset by LAST_VIRTUAL_REGISTER + 1 (as in the patch), and printed
just as a number, giving:

  (reg:SI 3) 

* Prefixed by a "sigil" character:

  (reg:SI $3) 
  (reg:SI %3) 
  (reg:SI #3) 
  (reg:SI +3)
  (reg:SI @3)
 
(reg:SI &3)
  (reg:SI ?3)
  (reg:SI P3)

* Prefixed so it looks like a register name:

  (reg:SI pseudo-3)
  (reg:SI pseudo_3)
  (reg:SI pseudo+3)


* Other syntax ideas:

  (reg:SI (3))

  (reg:SI {3})

  (reg:SI (87+3))
  (reg:SI (LAST_VIRTUAL_REGISTER + 4))

Looking at print_rtx_operand_code_r there are also things like ORIGINAL_REGNO, REG_EXPR and REG_OFFSET which get printed after the main regno, e.g.:

  (reg:SI 1 [ <retval> ])

My thought was to use whatever we come up with for REGNO for ORIGINAL_REGNO (after the "orig:").	

Hope this is constructive
Dave

[1] i.e. this is my list before applying any "is this idea a good one" filtering/self-censorship...

On 10/13/2016 04:08 PM, David Malcolm wrote:
> I thought it might be useful to brainstorm [1] some ideas on this,
> so  here are various possible ways it could be printed for this use-case:
>
> * Offset by LAST_VIRTUAL_REGISTER + 1 (as in the patch), and printed
> just as a number, giving:
>
>   (reg:SI 3)

Unambiguous in the compact format, nice low register numbers, but some 
potential for confusion with hard regs based on what people are used to.

> * Prefixed by a "sigil" character:

 >   (reg:SI %3)

Avoids the confusion issue and shouldn't overlap with hard register 
names. I think this is the one I prefer, followed by plain (reg:SI 3).

>   (reg:SI P3)

Can't use this, as there are machines with P3 registers.

> * Prefixed so it looks like a register name:
>
>   (reg:SI pseudo-3)
>   (reg:SI pseudo_3)
>   (reg:SI pseudo+3)

Not too different from just a "%" prefix and probably too verbose.

> Looking at print_rtx_operand_code_r there are also things like
> ORIGINAL_REGNO, REG_EXPR and REG_OFFSET which get printed after the
> main regno, e.g.: >

>   (reg:SI 1 [ <retval> ])

That's the REG_EXPR here presumably? The interesting part comes when 
parsing this.


Bernd