new register allocator and HARD_REGNO_CALL_PART_CLOBBERED

[Daniel Berlin]

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.
Index: ra-build.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ra-build.c,v
retrieving revision 1.5.2.11
diff -u -3 -p -r1.5.2.11 ra-build.c
--- ra-build.c  9 Apr 2003 19:27:35 -0000       1.5.2.11
+++ ra-build.c  2 May 2003 02:20:54 -0000
@@ -87,6 +87,8 @@ static void remember_move PARAMS ((rtx))
  static void handle_asm_insn PARAMS ((struct df *, rtx));
  static void prune_hardregs_for_mode PARAMS ((HARD_REG_SET *,
                                              enum machine_mode));
+static void prune_call_clobbered_hardregs PARAMS ((HARD_REG_SET *,
+                                   enum machine_mode));
  static void init_one_web_common PARAMS ((struct web *, rtx));
  static void init_one_web PARAMS ((struct web *, rtx));
  static void reinit_one_web PARAMS ((struct web *, rtx));
@@ -2328,7 +2330,18 @@ conflicts_between_webs (df)
      }
  #endif
  }
-
+static void
+prune_call_clobbered_hardregs (set, mode)
+     HARD_REG_SET *set;
+     enum machine_mode mode;
+{
+  size_t i;
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+    {
+      if (HARD_REGNO_CALL_PART_CLOBBERED(i, mode))
+       CLEAR_HARD_REG_BIT (*set, i);
+    }
+}
  /* Remember that a web was spilled, and change some characteristics
     accordingly.  */

@@ -2374,6 +2387,8 @@ remember_web_was_spilled (web)
    if (web->mode_changed)
      AND_COMPL_HARD_REG_SET (web->usable_regs, 
invalid_mode_change_regs);
  #endif
+  if (web->crosses_call)
+    prune_call_clobbered_hardregs (&web->usable_regs, 
PSEUDO_REGNO_MODE (web->regno));
    web->num_freedom = hard_regs_count (web->usable_regs);
    if (!web->num_freedom)
      abort();

>  The
> routine needs more info because it needs to know if a register is
> used during a call. This may be stupid suggestion though.
Which is what i'm trying to avoid.
By that time, we already have a list of usable regs for that pseudo/web.
The fact that the list is wrong because of other compiler stupidity is 
not a good reason to kludge around it.