Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-29 Thread Jeff Law

On 09/28/2016 10:49 AM, Bernd Schmidt wrote:

On 09/28/2016 06:36 PM, Jeff Law wrote:

A "p" prefix for pseudos might still be a good idea, but there's still
the issue of a real "p0" register name causing confusion.

So how do you think we should deal with distinguishing between the
different registers that may appear in a dump file?


I think the main problem we were trying to solve is making sure we can
make future-proof dumps. So that would argue for allowing h0, v0, p0
syntax in the reader, but not printing them out that way by default.

Correct, I'm mostly concerned with future proofing.

I'm certainly OK with a flag to create dumps in a form that's easier to 
parse, but leaving things as-is by default.





Also, if we don't already, allow hard regs to be specified by name in
the reader, and maybe even require it for virtuals.

Works for me.

jeff


Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-29 Thread David Malcolm
On Wed, 2016-09-28 at 18:49 +0200, Bernd Schmidt wrote:
> On 09/28/2016 06:36 PM, Jeff Law wrote:
> > > A "p" prefix for pseudos might still be a good idea, but there's
> > > still
> > > the issue of a real "p0" register name causing confusion.
> > So how do you think we should deal with distinguishing between the
> > different registers that may appear in a dump file?
> 
> I think the main problem we were trying to solve is making sure we
> can 
> make future-proof dumps. So that would argue for allowing h0, v0, p0 
> syntax in the reader, but not printing them out that way by default.
> 
> Also, if we don't already, allow hard regs to be specified by name in
> the reader, and maybe even require it for virtuals.

Yes: the problems I'm trying to solve here are:
(a) making dumps more resilient against changing .md files, and
(b) where possible, allowing target-independent dump fragments where
everything is a pseudo

The issue I ran into was parsing the "r" format code, for a regno,
where print_rtx can print various things after the actual number.  My
hope with the prefix patch was to give the parser more of a hint as to
the category of reg (and to perhaps make dumps easier for humans to
read).

But it looks like the "r" format code is only ever used by REG, which
means there's always a closing parenthesis at the end of the material
emitted for the "r" format code.  So given that I *think* that the
parser already has all it needs, and that the patch is redundant.

So my plan for the reader is:
- read the number emitted by "r"
- see if there's a name after the number.  If there is, assume a hard
or virtual reg, and try to parse the name:
  - if the name is recognized, use the target's current number for that
name (future-proofing virtuals against .md changes)
  - if the name is not recognized, issue a fatal error (it's probably a
typo, or maybe a backport from a future version of gcc, or a target
incompatibility)
  - if there's no name, assume it's a pseudo.  Remap all pseudos in a
postprocessing phase to ensure that they *are* pseudos (even if the .md
has gained some hard regs in the meantime), leaving the numbering
untouched if possible (the common case).

...and to drop the regno-prefixing idea from this patch.

Hopefully this sounds sane.

(I'm also working on a function-dumping patch, which will cover CFG
information and various "crtl" information and other state that can't
be reconstructed and hence ought to be in the dump).

Dave


Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-28 Thread Bernd Schmidt

On 09/28/2016 06:36 PM, Jeff Law wrote:

A "p" prefix for pseudos might still be a good idea, but there's still
the issue of a real "p0" register name causing confusion.

So how do you think we should deal with distinguishing between the
different registers that may appear in a dump file?


I think the main problem we were trying to solve is making sure we can 
make future-proof dumps. So that would argue for allowing h0, v0, p0 
syntax in the reader, but not printing them out that way by default.


Also, if we don't already, allow hard regs to be specified by name in 
the reader, and maybe even require it for virtuals.



Bernd


Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-28 Thread Jeff Law

On 09/28/2016 10:30 AM, Bernd Schmidt wrote:

On 09/28/2016 06:23 PM, Jeff Law wrote:


  (reg/i:SI h0 ax)
  (reg/i:SF h21 xmm0)


(Replying to myself, in the hope of better demonstrating the idea)

The following patch implements this idea for RTL dumps, so that all
REGNO
values in dumps get a one character prefix: 'h' for hard registers, 'v'
for virtual registers, and 'p' for non-virtual pseudos (making it easier
for both humans and parsers to grok the meaning of a REGNO).

I think you nailed it.  h, v & p prefixing for each of the register
types, but leaving the actual register number as-is in the dump file.


I'm actually no longer quite so sure this buys us much: a port might
have an actual register named "h0", leading to confusion. Virtual and
hard registers also already have their real name printed after the number.

A "p" prefix for pseudos might still be a good idea, but there's still
the issue of a real "p0" register name causing confusion.
So how do you think we should deal with distinguishing between the 
different registers that may appear in a dump file?


The case I'm worried about is the register meanings in a testsuite dump 
file changing over time if/when new hard registers are added to the port 
or we introduce new virtual registers.


FOr hard regs and virtuals we can probably map backwards using their 
names.  So given a register in a dump, if we can't reverse map it back 
to a hard reg or a virtual, then we assume its a pseudo?


jeff


Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-28 Thread Jeff Law

On 09/21/2016 01:01 PM, David Malcolm wrote:


Presumably we could use "v" rather than "p" as the prefix for the
first
5 pseudos (up to LAST_VIRTUAL_REGISTER), doing any adjustment at load
time, rather than at dump time.  So the above example would look
like:

   (reg/f:DI v82 virtual-stack-vars)

i.e. the 82 for x86_64's virtual-stack-vars would be prefixed with a
'v', and the loader would adjust it to be the current target's value
for VIRTUAL_STACK_VARS_REGNUM.

Do you like the idea of prefixing regnums of hardregs with 'h'? (so
that all regnos get a one-char prefix) e.g.
  (reg/i:SI h0 ax)
  (reg/i:SF h21 xmm0)


(Replying to myself, in the hope of better demonstrating the idea)

The following patch implements this idea for RTL dumps, so that all REGNO
values in dumps get a one character prefix: 'h' for hard registers, 'v'
for virtual registers, and 'p' for non-virtual pseudos (making it easier
for both humans and parsers to grok the meaning of a REGNO).
I think you nailed it.  h, v & p prefixing for each of the register 
types, but leaving the actual register number as-is in the dump file.




Successfully bootstrapped on x86_64-pc-linux-gnu.
There are various regression test failures involving scan-rtl-dump
due to regexes not matching e.g.
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set \\(reg/i:TI 
0 ax\\)"
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set \\(reg:TI [0-9]* 
\\[  \\]\\)"
If the approach is OK, I can do an updated patch that also fixes up the
relevant tests (adding the appropriate prefixes); this would touch
multiple targets.
Yea.  This obviously highlights some of the long term issues with making 
changes into the dump format.  As I said in our meeting yesterday, I do 
understand the desire to nail down the format :-)  But I also want to 
use the opportunity we have to make the dumps easier for your work to 
read & interpret.


jeff



Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-28 Thread Bernd Schmidt

On 09/28/2016 06:23 PM, Jeff Law wrote:


  (reg/i:SI h0 ax)
  (reg/i:SF h21 xmm0)


(Replying to myself, in the hope of better demonstrating the idea)

The following patch implements this idea for RTL dumps, so that all REGNO
values in dumps get a one character prefix: 'h' for hard registers, 'v'
for virtual registers, and 'p' for non-virtual pseudos (making it easier
for both humans and parsers to grok the meaning of a REGNO).

I think you nailed it.  h, v & p prefixing for each of the register
types, but leaving the actual register number as-is in the dump file.

I'm actually no longer quite so sure this buys us much: a port might 
have an actual register named "h0", leading to confusion. Virtual and 
hard registers also already have their real name printed after the number.


A "p" prefix for pseudos might still be a good idea, but there's still 
the issue of a real "p0" register name causing confusion.



Bernd



[PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

2016-09-21 Thread David Malcolm
On Tue, 2016-09-20 at 15:12 -0400, David Malcolm wrote:
> On Tue, 2016-09-20 at 09:20 -0600, Jeff Law wrote:
> > On 09/20/2016 08:34 AM, Bernd Schmidt wrote:
> > > On 09/20/2016 04:32 PM, David Malcolm wrote:
> > > >
> > > > To summarize so far: you want every pseudo to have its regno
> > > > dumped
> > > > with a 'p' prefix, and renumber them on dump (and then on
> > > > load).
> > > > OK.
> > >
> > > Renumbering is not helpful because it interferes with the view
> > > you
> > > have
> > > in the debugger. So, IMO just a prefix, and maybe
> > Yea, I guess it does since we want the numbers in the dump to be
> > the
> > same that we used to access the arrays.  So prefixing in the dump
> > with
> > adjustment of the number in the reader.
>
> To check I understand: am I right in thinking you want:
> (A) the numbers in the dump to be unmodified when dumping, so that we
> can easily look up values in arrays without confusion, and
> (B) regnums in the dump gain a 'p' prefix for values >=
>  FIRST_PSEUDO_REGISTER, so that humans and parsers can easily see
> when
> the regs are pseudos, and that
> (C) the parser will detect if a 'p'-prefixed regno actually has the
> same number as a hard reg (which can happen e.g. when a .md file
> changes, or when sharing .rtl dumps between targets), and remap the
> values on load accordingly
>
> ?
>
> (in which case we do need the regno_remapper class, or something like
> it)
>
> > >
> > > >   (reg/f:DI v1 virtual-stack-vars)
> > >
> > > this.
> > Doesn't the same apply to the number of virtual stack regs?  Those
> > are
> > in the same array as pseudos.  So ISTM we prefix in the dump, but
> > do
> > adjustment of the number in the reader?
>
> Presumably we could use "v" rather than "p" as the prefix for the
> first
> 5 pseudos (up to LAST_VIRTUAL_REGISTER), doing any adjustment at load
> time, rather than at dump time.  So the above example would look
> like:
>
>(reg/f:DI v82 virtual-stack-vars)
>
> i.e. the 82 for x86_64's virtual-stack-vars would be prefixed with a
> 'v', and the loader would adjust it to be the current target's value
> for VIRTUAL_STACK_VARS_REGNUM.
>
> Do you like the idea of prefixing regnums of hardregs with 'h'? (so
> that all regnos get a one-char prefix) e.g.
>   (reg/i:SI h0 ax)
>   (reg/i:SF h21 xmm0)

(Replying to myself, in the hope of better demonstrating the idea)

The following patch implements this idea for RTL dumps, so that all REGNO
values in dumps get a one character prefix: 'h' for hard registers, 'v'
for virtual registers, and 'p' for non-virtual pseudos (making it easier
for both humans and parsers to grok the meaning of a REGNO).

For example, on compiling this:

  int test (int i)
  {
return i * i;
  }

for target==x86_64, the dump for expand looks like this:

(note 1 0 4 NOTE_INSN_DELETED)
(note 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 4 3 2 (set (mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 A32])
(reg:SI h5 di [ i ])) ../../test.c:2 -1
 (nil))
(note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
(insn 6 3 7 2 (set (reg:SI p89)
(mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 A32])) 
../../test.c:3 -1
 (nil))
(insn 7 6 10 2 (parallel [
(set (reg:SI p87 [ _2 ])
(mult:SI (reg:SI p89)
(mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 
A32])))
(clobber (reg:CC h17 flags))
]) ../../test.c:3 -1
 (nil))
(insn 10 7 14 2 (set (reg:SI p88 [  ])
(reg:SI p87 [ _2 ])) ../../test.c:3 -1
 (nil))
(insn 14 10 15 2 (set (reg/i:SI h0 ax)
(reg:SI p88 [  ])) ../../test.c:4 -1
 (nil))
(insn 15 14 0 2 (use (reg/i:SI h0 ax)) ../../test.c:4 -1
 (nil))

Note the 'v' prefix here:
  (reg/f:DI v82 virtual-stack-vars)
the 'h' prefix here:
  (reg/i:SI h0 ax)
and the 'p' prefix here:
  (reg:SI p88 [  ])

Successfully bootstrapped on x86_64-pc-linux-gnu.
There are various regression test failures involving scan-rtl-dump
due to regexes not matching e.g.
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set 
\\(reg/i:TI 0 ax\\)"
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set 
\\(reg:TI [0-9]* \\[  \\]\\)"
If the approach is OK, I can do an updated patch that also fixes up the
relevant tests (adding the appropriate prefixes); this would touch
multiple targets.

gcc/ChangeLog:
* print-rtl.c (print_rtx): When dumping regnos, prefix the
number with 'h', 'v' or 'p', depending on whether it is a
hard register, a virtual register, or a non-virtual pseudo.
* doc/rtl.texi (Registers and Memory): Document the above change,
adding the prefix to examples that specifically refer to hard or
pseudo regs in the text.
---
 gcc/doc/rtl.texi | 26 +++