Re: [new regalloc]: Spill for SImode down in FP regs

[Michael Matz <matz at suse dot de>]

Hi Hartmut,


Many thanks for working on it.  I was already quite filled with finding
the alpha miscompilation.

On Thu, 13 Jun 2002, Hartmut Penner wrote:

> The compiler produces a insn (set (reg:SI 6 %r6) (reg:SI 17 %f2))
> which does not satisfy it's constraints, since s390 has no instruction
> loading a floating point register into a general purpose register.
> This insn is introduced by the ra for a spill. The function
> 'remember_web_was_spilled' ignores the
> preferred regclass and set the regclass to 'ALL_REGS'.

Ahh.  This one.  I already waited for some architectures which barf on
that.

> For s390 (and maybe other architecture), the regclass should in this
> specific case only be widened to GENERAL_REGS.

Hmm, yep.  Either we need to conditionalize this on the size (like you
did), or maybe only widen it to a certain extent (like e.g. if the
preferred_class contains any GENERAL_REGS widen it to that.  If it
contains an FP_REG widen it to that).  I think currently your fix is
enough.  Once we track the class of webs precisely (like in Denis' work)
this problem should go away.

Note also, that I fixed some things, which e.g. make alpha bootstrap
again.  It has to do with ra.c not handling CLASS_CANNOT_CHANGE_MODE at
all.  This means, that at least alpha, ppc and s390 were anyway broken due
to that (not necessarily non-bootstrappable, though).  The diff below
(which I checked in) fixes that.

I'll also apply your patch after testing it on x86.


Ciao,
Michael.
-- 
        * df.h (DF_REF_MODE_CHANGE): New flag.
        * df.c (df_def_record_1, df_uses_record): Set this flag for refs
        involving subregs with invalid mode changes, when
        CLASS_CANNOT_CHANGE_MODE is defined.
        * ra.c (struct web.mode_changed): New member.
        (reinit_one_web): Reset it.
        (compare_and_free_webs): Check it.
        (parts_to_webs_1): Set it.
        (init_one_web_common, remember_web_was_spilled, colorize_one_web):
        If mode_changed is set, cut off all regs in CLASS_CANNOT_CHANGE_MODE.
        (remember_web_was_spilled): Use the preferred and alternate class for
        stack pseudos.
        (insert_stores): Don't remember stores over trapping insns.
        (detect_web_parts_to_rebuild): Remove all uses of hardregs from
        uses_as_bitmap.

? ra-new.c
Index: df.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/df.c,v
retrieving revision 1.1.2.13
diff -u -p -r1.1.2.13 df.c
--- df.c	24 May 2002 10:14:33 -0000	1.1.2.13
+++ df.c	13 Jun 2002 08:19:05 -0000
@@ -964,6 +964,13 @@ df_def_record_1 (df, x, bb, insn)
       return;
     }

+#ifdef CLASS_CANNOT_CHANGE_MODE
+  if (GET_CODE (dst) == SUBREG
+      && CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (dst),
+				     GET_MODE (SUBREG_REG (dst))))
+    flags |= DF_REF_MODE_CHANGE;
+#endif
+
   /* May be, we should flag the use of strict_low_part somehow.  Might be
      handy for the reg allocator.  */
   while (GET_CODE (dst) == STRICT_LOW_PART
@@ -978,6 +985,12 @@ df_def_record_1 (df, x, bb, insn)
 	  loc = &XEXP (dst, 0);
 	  dst = *loc;
 	}
+#ifdef CLASS_CANNOT_CHANGE_MODE
+      if (GET_CODE (dst) == SUBREG
+	  && CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (dst),
+				         GET_MODE (SUBREG_REG (dst))))
+        flags |= DF_REF_MODE_CHANGE;
+#endif
       loc = &XEXP (dst, 0);
       dst = *loc;
       flags |= DF_REF_READ_WRITE;
@@ -1073,6 +1086,11 @@ df_uses_record (df, loc, ref_type, bb, i
 	  df_uses_record (df, loc, ref_type, bb, insn, flags);
 	  return;
 	}
+#ifdef CLASS_CANNOT_CHANGE_MODE
+      if (CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (x),
+				      GET_MODE (SUBREG_REG (x))))
+        flags |= DF_REF_MODE_CHANGE;
+#endif

       /* ... Fall through ...  */

@@ -1089,16 +1107,24 @@ df_uses_record (df, loc, ref_type, bb, i

 	switch (GET_CODE (dst))
 	  {
+	    enum df_ref_flags use_flags;
 	    case SUBREG:
 	      if (read_modify_subreg_p (dst))
 		{
+		  use_flags = DF_REF_READ_WRITE;
+#ifdef CLASS_CANNOT_CHANGE_MODE
+		  if (CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (x),
+						  GET_MODE (SUBREG_REG (x))))
+		    use_flags |= DF_REF_MODE_CHANGE;
+#endif
 		  df_uses_record (df, &SUBREG_REG (dst), DF_REF_REG_USE, bb,
-				  insn, DF_REF_READ_WRITE);
+				  insn, use_flags);
 		  break;
 		}
 	      /* ... FALLTHRU ...  */
 	    case REG:
 	    case PC:
+	    case PARALLEL:
 	      break;
 	    case MEM:
 	      df_uses_record (df, &XEXP (dst, 0),
@@ -1110,8 +1136,14 @@ df_uses_record (df, loc, ref_type, bb, i
 	      dst = XEXP (dst, 0);
 	      if (GET_CODE (dst) != SUBREG)
 		abort ();
+	      use_flags = DF_REF_READ_WRITE;
+#ifdef CLASS_CANNOT_CHANGE_MODE
+	      if (CLASS_CANNOT_CHANGE_MODE_P (GET_MODE (x),
+					      GET_MODE (SUBREG_REG (x))))
+		use_flags |= DF_REF_MODE_CHANGE;
+#endif
 	      df_uses_record (df, &SUBREG_REG (dst), DF_REF_REG_USE, bb,
-			     insn, DF_REF_READ_WRITE);
+			     insn, use_flags);
 	      break;
 	    case ZERO_EXTRACT:
 	    case SIGN_EXTRACT:
Index: df.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/df.h,v
retrieving revision 1.1.2.8
diff -u -p -r1.1.2.8 df.h
--- df.h	3 May 2002 19:51:06 -0000	1.1.2.8
+++ df.h	13 Jun 2002 08:19:05 -0000
@@ -50,7 +50,16 @@ struct df_link

 enum df_ref_flags
   {
-    DF_REF_READ_WRITE = 1
+    DF_REF_READ_WRITE = 1,
+
+    /* This flag is set on register references itself representing a or
+       being inside a subreg on machines which have CLASS_CANNOT_CHANGE_MODE
+       and where the mode change of that subreg expression is invalid for
+       this class.  Note, that this flag can also be set on df_refs
+       representing the REG itself (i.e. one might not see the subreg
+       anyore).  Also note, that this flag is set also for hardreg refs.
+       I.e. you must check yourself if it's a pseudo.  */
+    DF_REF_MODE_CHANGE = 2
   };

 /* Define a register reference structure.  */
Index: ra.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/ra.c,v
retrieving revision 1.1.2.55
diff -u -p -r1.1.2.55 ra.c
--- ra.c	7 May 2002 20:53:30 -0000	1.1.2.55
+++ ra.c	13 Jun 2002 08:19:06 -0000
@@ -175,6 +175,9 @@ struct web
                                   in a move) */
   unsigned int live_over_abnormal:1; /* 1 when this web (or parts thereof) are live
 			       over an abnormal edge.  */
+  unsigned int mode_changed:1; /* != 0 if this web is used in subregs where
+				  the mode change was illegal for hardregs
+				  in CLASS_CANNOT_CHANGE_MODE.  */
   unsigned int old_web:1; /* != 0, when this web stems from the last iteration
 			     of the allocator, and still contains all info.  */
   unsigned int in_load:1; /* For rewrite_program2() to remember webs, which
@@ -2424,6 +2427,11 @@ init_one_web_common (web, reg)
       AND_COMPL_HARD_REG_SET (web->usable_regs, never_use_colors);
       prune_hardregs_for_mode (&web->usable_regs,
 			       PSEUDO_REGNO_MODE (web->regno));
+#ifdef CLASS_CANNOT_CHANGE_MODE
+      if (web->mode_changed)
+        AND_COMPL_HARD_REG_SET (web->usable_regs, reg_class_contents[
+			          (int) CLASS_CANNOT_CHANGE_MODE]);
+#endif
       web->num_freedom = hard_regs_count (web->usable_regs);
       web->num_freedom -= web->add_hardregs;
       if (!web->num_freedom)
@@ -2458,6 +2466,7 @@ reinit_one_web (web, reg)
   web->is_coalesced = 0;
   web->artificial = 0;
   web->live_over_abnormal = 0;
+  web->mode_changed = 0;
   web->move_related = 0;
   web->in_load = 0;
   web->target_of_spilled_move = 0;
@@ -2911,6 +2920,7 @@ compare_and_free_webs (link)
       if (web1->regno != web2->regno
 	  || web1->crosses_call != web2->crosses_call
 	  || web1->live_over_abnormal != web2->live_over_abnormal
+	  || web1->mode_changed != web2->mode_changed
 	  || !rtx_equal_p (web1->orig_x, web2->orig_x)
 	  || web1->type != web2->type
 	  /* Only compare num_defs/num_uses with non-hardreg webs.
@@ -3073,6 +3083,9 @@ parts_to_webs_1 (df, copy_webs, all_refs
 	{
 	  web = def2web[i];
 	  web = find_web_for_subweb (web);
+	  if ((DF_REF_FLAGS (ref) & DF_REF_MODE_CHANGE) != 0
+	      && web->regno >= FIRST_PSEUDO_REGISTER)
+	    web->mode_changed = 1;
 	  if (i >= def_id
 	      && TEST_BIT (live_over_abnormal, ref_id))
 	    web->live_over_abnormal = 1;
@@ -3106,6 +3119,9 @@ parts_to_webs_1 (df, copy_webs, all_refs
 	}
       else
 	subweb = web;
+      if ((DF_REF_FLAGS (ref) & DF_REF_MODE_CHANGE) != 0
+	  && web->regno >= FIRST_PSEUDO_REGISTER)
+	web->mode_changed = 1;
       if (i < def_id)
 	{
 	  if (ra_pass > 1)
@@ -3462,9 +3478,22 @@ remember_web_was_spilled (web)
      alternatives needs applying.  Currently this is dealt with by reload, as
      many other things, but at some time we want to integrate that
      functionality into the allocator.  */
-  COPY_HARD_REG_SET (web->usable_regs, reg_class_contents[(int) ALL_REGS]);
+  if (web->regno >= max_normal_pseudo)
+    {
+      COPY_HARD_REG_SET (web->usable_regs,
+			reg_class_contents[reg_preferred_class (web->regno)]);
+      IOR_HARD_REG_SET (web->usable_regs,
+			reg_class_contents[reg_alternate_class (web->regno)]);
+    }
+  else
+    COPY_HARD_REG_SET (web->usable_regs, reg_class_contents[(int) ALL_REGS]);
   AND_COMPL_HARD_REG_SET (web->usable_regs, never_use_colors);
   prune_hardregs_for_mode (&web->usable_regs, PSEUDO_REGNO_MODE (web->regno));
+#ifdef CLASS_CANNOT_CHANGE_MODE
+  if (web->mode_changed)
+    AND_COMPL_HARD_REG_SET (web->usable_regs, reg_class_contents[
+			      (int) CLASS_CANNOT_CHANGE_MODE]);
+#endif
   web->num_freedom = hard_regs_count (web->usable_regs);
   if (!web->num_freedom)
     abort();
@@ -5496,6 +5525,11 @@ colorize_one_web (web, hard)
       else
 	COPY_HARD_REG_SET (colors,
 			   usable_regs[reg_preferred_class (web->regno)]);
+#ifdef CLASS_CANNOT_CHANGE_MODE
+      if (web->mode_changed)
+        AND_COMPL_HARD_REG_SET (colors, reg_class_contents[
+			          (int) CLASS_CANNOT_CHANGE_MODE]);
+#endif
       COPY_HARD_REG_SET (call_clobbered, colors);
       AND_HARD_REG_SET (call_clobbered, call_used_reg_set);

@@ -5522,6 +5556,11 @@ colorize_one_web (web, hard)
 	  else
 	    IOR_HARD_REG_SET (colors, usable_regs
 			      [reg_alternate_class (web->regno)]);
+#ifdef CLASS_CANNOT_CHANGE_MODE
+	  if (web->mode_changed)
+	    AND_COMPL_HARD_REG_SET (colors, reg_class_contents[
+				      (int) CLASS_CANNOT_CHANGE_MODE]);
+#endif
 	  COPY_HARD_REG_SET (call_clobbered, colors);
 	  AND_HARD_REG_SET (call_clobbered, call_used_reg_set);

@@ -7005,7 +7044,7 @@ insert_stores (new_deaths)
     {
       unsigned int uid = INSN_UID (insn);
       if (/*GET_CODE (insn) == CODE_LABEL || */GET_CODE (insn) == BARRIER
-	  || JUMP_P (insn))
+	  || JUMP_P (insn) || can_throw_internal (insn))
 	{
 	  /* Clear any info about already emitted stores.  */
 	  last_slot = NULL_RTX;
@@ -7972,6 +8011,24 @@ detect_web_parts_to_rebuild (void)
 	    mark_refs_for_checking (web2, uses_as_bitmap);
 	  });
       }
+
+  /* We also recheck unconditionally all uses of any hardregs.  This means
+     we _can_ delete all these uses from the live_at_end[] bitmaps.
+     And because we sometimes delete insn refering to hardregs (when
+     they became useless because they setup a rematerializable pseudo, which
+     then was rematerialized), some of those uses will go away with the next
+     df_analyse().  This means we even _must_ delete those uses from
+     the live_at_end[] bitmaps.  For simplicity we simply delete
+     all of them.  */
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+    if (!fixed_regs[i])
+      {
+	struct df_link *link;
+	for (link = df->regs[i].uses; link; link = link->next)
+	  if (link->ref)
+	    bitmap_set_bit (uses_as_bitmap, DF_REF_ID (link->ref));
+      }
+
   live_at_end -= 2;
   for (i = 0; i < (unsigned int) n_basic_blocks + 2; i++)
     bitmap_operation (live_at_end[i], live_at_end[i], uses_as_bitmap,