Herman ten Brugge | 11 May 2003 14:23
Picon
Favicon

Re: new register allocator and HARD_REGNO_CALL_PART_CLOBBERED

Daniel Berlin wrote :
> 
> 
> On Thursday, May 1, 2003, at 05:31  PM, Herman ten Brugge wrote:
> >
> > HARD_REGNO_CALL_PART_CLOBBERED should be called at the place where we
> > create a hard register with a specific mode.
> 
> No, it shouldn't.
> It should be called at the point where we need to know if a hard 
> register is clobbered in part.
> Hence the name.
> new-ra what pseudos/hard registers conflict with what pseudos/hard 
> registers  before we can decided what registers to assign to where.
> Please stop assuming that what the current allocators do is the right 
> way to do it.
> It's not for anything but a linear scan allocator.
> >  Before that we can do
> > nothing because HARD_REGNO_CALL_PART_CLOBBERED needs the correct mode.
> >
> > For example the c4x target will only save the integer part of register 
> > r4.
> > So if we need an integer register we can use r4 during a call. If we 
> > need
> > a floating point register we can not use r4 during a call.
> 
> Uggh.
> That's horrific.
> You aren't going to be able to get this to happen in a reasonable way.
> Ideally, the regclass pass should be handing us a register class that's 
> actually valid here. It's not.  Reload should also be fixing the 
> new-ra's mistake (which isn't really it's fault).  It doesn't.
> Either would be better than hacking the new-ra to handle these 
> oddities, and the first would result in faster and better allocation.
> 
> If you really want me to give you a patch to fix this in new-ra, i 
> will, but i *really* don't think it's the right place.
> >
> > The c4x abi specifies that r4,r5 and r8 are saved in integer mode
> > and r6,r7 are saved in floating point mode.
> >
> > So where should make the change at the place where we decide in what
> > mode a register is used. If df.c does that then we must find the 
> > correct
> > place to do that. If not then we have to find the other place where 
> > this
> > is decided.
> >
> > The last patch did not work because the mode was not set correctly for
> > the r4..r7 and r8 register. The mode was always QImode 
> > (reg_raw_mode[i])
> > for some reason. I showed that at least r4 was used as floating point
> > register so this should have been QFmode.
> >
> > After having a quick peek at the new-ra code it looks to me that a 
> > routine
> > like get_free_reg should check for HARD_REGNO_CALL_PART_CLOBBERED?
> No.
> Ideally, regclass shouldn't give us regclasses that are not correct.
> This is the biggest source of ugliness in the new-register-allocator, 
> is that we can't depend on what regs it says things can use being right.
> 
> If you *really* feel the need to ugly up the new-register-allocator 
> some more, you want to do it at build time.
> I won't submit this patch, however, and i hope it wouldn't be accepted.

That patch did not work because it was never executed in my testcase.
I have a patch that works for me and should probably work on all
targets that have HARD_REGNO_CALL_PART_CLOBBERED defined. I have tested
this on the c4x. I still think the HARD_REGNO_CALL_PART_CLOBBERED should
be used in the new register allocation pass. But I know this is very very
difficult.

	Herman

2003-05-11  Herman A.J. ten Brugge <hermantenbrugge <at> home.nl>

	* df.c (df_insn_refs_record) Mark hard_regno_call_part_clobbered
	registers as used.
	(df_init) Init the hard_regno_call_part_clobbered data.
	

--- df.c.org	2003-05-07 21:41:18.000000000 +0200
+++ df.c	2003-05-11 14:16:38.000000000 +0200
 <at>  <at>  -201,6 +201,9  <at>  <at>  static alloc_pool df_ref_pool;
 static alloc_pool df_link_pool;
 static struct df *ddf;

+static int df_hard_regno_call_part_clobbered;
+static int hard_regno_call_part_clobbered[FIRST_PSEUDO_REGISTER];
+
 static void df_reg_table_realloc PARAMS((struct df *, int));
 static void df_insn_table_realloc PARAMS((struct df *, unsigned int));
 static void df_bitmaps_alloc PARAMS((struct df *, int));
 <at>  <at>  -1275,6 +1278,12  <at>  <at>  df_insn_refs_record (df, bb, insn)
 		    df_uses_record (df, &SET_DEST (x),
 				    DF_REF_REG_USE, bb, insn, 0);
 		  }
+	      for (i = 0; i < df_hard_regno_call_part_clobbered; i++)
+		{
+	          x = df_reg_use_gen (hard_regno_call_part_clobbered[i]);
+	          df_uses_record (df, &XEXP (x, 0),
+				  DF_REF_REG_USE, bb, insn, 0);
+		}
 	    }
 	}

 <at>  <at>  -2182,6 +2191,7  <at>  <at>  df_analyse_1 (df, blocks, flags, update)
 struct df *
 df_init ()
 {
+  int i, m;
   struct df *df;

   df = xcalloc (1, sizeof (struct df));
 <at>  <at>  -2189,6 +2199,18  <at>  <at>  df_init ()
   /* Squirrel away a global for debugging.  */
   ddf = df;

+  /* HACK. HARD_REGNO_CALL_PART_CLOBBERED should be checked in the
+	   new register allocation pass.  */
+  df_hard_regno_call_part_clobbered = 0;
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+    {
+      for (m = 0; m < NUM_MACHINE_MODES; m++)
+	if (HARD_REGNO_MODE_OK (i, m)
+	    && HARD_REGNO_CALL_PART_CLOBBERED (i, m))
+	  break;
+      if (m != NUM_MACHINE_MODES)
+	hard_regno_call_part_clobbered[df_hard_regno_call_part_clobbered++] = i;
+    }
   return df;
 }


Gmane