question about register pairs
diff mbox

Message ID 201310250415.r9P4FEZt020568@greed.delorie.com
State New
Headers show

Commit Message

DJ Delorie Oct. 25, 2013, 4:15 a.m. UTC
Yup, my registers are smaller than Pmode.

This is what I ended up with...

Some notes: I lie to gcc and tell it that $fp (reg 22) is two bytes
when it's really one.  None of the register classes have reg 23 (used
for the upper half of $fp) in them.  Reg 23 is also listed as being
two bytes, to keep the register pairing logic in reload in sync for
registers above $fp (otherwise it only checks odd registers afer $fp).

Does this all make sense?

Comments

Joern Rennecke Oct. 25, 2013, 12:35 p.m. UTC | #1
On 25 October 2013 05:15, DJ Delorie <dj@redhat.com> wrote:
>
> Yup, my registers are smaller than Pmode.
>
> This is what I ended up with...
>
> Some notes: I lie to gcc and tell it that $fp (reg 22) is two bytes
> when it's really one.

Well, it's not really a lie if you map hardware registers 22 and 23 to
a single register for the purposes of gcc internals.  Although it
does make some other things more awkward, e.g. when you
copy fp, and this gets split so you have an insn that copies the
highpart of fp to another register.

> Index: reload.c
> ===================================================================
> --- reload.c    (revision 203733)
> +++ reload.c    (working copy)
> @@ -723,13 +723,15 @@ find_valid_class_1 (enum machine_mode ou
>    unsigned int best_size = 0;
>    int cost;
>
>    for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
>      {
>        int bad = 0;
> -      for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
> +      for (regno = 0;
> +          regno < FIRST_PSEUDO_REGISTER && !bad;
> +          regno += HARD_REGNO_NREGS (regno, mode))
>         {
>           if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
>               && !HARD_REGNO_MODE_OK (regno, mode))
>             bad = 1;
>         }

Seeing the patched code in its entirety like this, I notice that we
would use HARD_REGNO_NREGS
for a regno that's not ok for the mode.
That can be avoided if we put a break into the if.  And then the !bad
term in the loop condition
becomes redundant.
Although the HARD_REGNO_NREGS definition in tm.texi says that HARD_REGNO_NREGS
must never return zero.  That wouldn't be technically violated with an
assert, but I suppose the
spirit of the text is that the macro should return always a
non-positive value, so I suppose this
use of HARD_REGNO_MODE_OK is not actually a problem.
DJ Delorie Oct. 25, 2013, 7:15 p.m. UTC | #2
> > Some notes: I lie to gcc and tell it that $fp (reg 22) is two bytes
> > when it's really one.
> 
> Well, it's not really a lie if you map hardware registers 22 and 23 to
> a single register for the purposes of gcc internals.

Yeah, I'm basically making those two registers into a permanent bigger
register.

> Although it does make some other things more awkward, e.g. when you
> copy fp, and this gets split so you have an insn that copies the
> highpart of fp to another register.

Not a problem, the chip can copy the whole $fp in one insn.  Registers
are only 8 bits on this chip.

> Seeing the patched code in its entirety like this, I notice that we
> would use HARD_REGNO_NREGS for a regno that's not ok for the mode.

The core problem is that gcc has no way of dealing with register pairs
as real registers - the second halves are still registers, still need
to be in the reg class, still need _NREGS() values, etc.  There's no
way to tell gcc that a "register" is a set of (for example) N
registers starting at 3, 5, 9, 12, or 14.

> That can be avoided if we put a break into the if.  And then the
> !bad term in the loop condition becomes redundant.

The problem I had to solve was to re-synchronize the counter with the
even-numbered register.  The naive patch counted even registers up to
$fp, then checked odd (invalid) registers after that.

The only other way I can think to "solve" this problem is to add a
VALID_STARTING_REG(reg,mode) macro, but that would require changes all
over the core code.
DJ Delorie Oct. 31, 2013, 10:11 p.m. UTC | #3
> Seeing the patched code in its entirety like this, I notice that we
> would use HARD_REGNO_NREGS for a regno that's not ok for the mode.
> That can be avoided if we put a break into the if.  And then the
> !bad term in the loop condition becomes redundant.  Although the
> HARD_REGNO_NREGS definition in tm.texi says that HARD_REGNO_NREGS
> must never return zero.  That wouldn't be technically violated with
> an assert, but I suppose the spirit of the text is that the macro
> should return always a non-positive value, so I suppose this use of
> HARD_REGNO_MODE_OK is not actually a problem.

Alternately, we could assume that if there's a gap in the register
class, that the next register that *is* in the register class is a
legitimate register pair?  I can't think of any reason why you'd do
otherwise, and certainly if it *isn't* the first of a pair, there's no
way to pass the test anyway.

Then we'd just increment by HARD_REGNO_NREGS *or* 1, depending on
whether the register is in the set or not, and break out of the test
for the first reg that's in the set but not valid for the mode, so no
need to increment after that case.

Patch
diff mbox

Index: reload.c
===================================================================
--- reload.c	(revision 203733)
+++ reload.c	(working copy)
@@ -723,13 +723,15 @@  find_valid_class_1 (enum machine_mode ou
   unsigned int best_size = 0;
   int cost;
 
   for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
     {
       int bad = 0;
-      for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
+      for (regno = 0;
+	   regno < FIRST_PSEUDO_REGISTER && !bad;
+	   regno += HARD_REGNO_NREGS (regno, mode))
 	{
 	  if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
 	      && !HARD_REGNO_MODE_OK (regno, mode))
 	    bad = 1;
 	}
       
Index: config/rl78/rl78.h
===================================================================
--- config/rl78/rl78.h	(revision 203733)
+++ config/rl78/rl78.h	(working copy)
@@ -242,12 +242,14 @@  enum reg_class
   "V_REGS",						\
   "GR_REGS",						\
   "PSWREG",						\
   "ALL_REGS"						\
 }
 
+/* Note that no class may include the second register in $fp, because
+   we treat $fp as a single HImode register.  */
 #define REG_CLASS_CONTENTS				\
 {							\
   { 0x00000000, 0x00000000 },	/* No registers,  */		\
   { 0x00000001, 0x00000000 }, \
   { 0x00000002, 0x00000000 }, \
   { 0x00000003, 0x00000000 }, \
Index: config/rl78/rl78.c
===================================================================
--- config/rl78/rl78.c	(revision 203733)
+++ config/rl78/rl78.c	(working copy)
@@ -321,19 +321,21 @@  rl78_option_override (void)
       for (i = 24; i < 32; i++)
 	fixed_regs[i] = 0;
     }
 }
 
 /* Most registers are 8 bits.  Some are 16 bits because, for example,
-   gcc doesn't like dealing with $FP as a register pair.  This table
-   maps register numbers to size in bytes.  */
+   gcc doesn't like dealing with $FP as a register pair (the second
+   half of $fp is also 2 to keep reload happy wrt register pairs, but
+   no register class includes it).  This table maps register numbers
+   to size in bytes.  */
 static const int register_sizes[] =
 {
   1, 1, 1, 1, 1, 1, 1, 1,
   1, 1, 1, 1, 1, 1, 1, 1,
-  1, 1, 1, 1, 1, 1, 2, 1,
+  1, 1, 1, 1, 1, 1, 2, 2,
   1, 1, 1, 1, 1, 1, 1, 1,
   2, 2, 1, 1, 1
 };
 
 /* Predicates used in the MD patterns.  This one is true when virtual
    insns may be matched, which typically means before (or during) the