This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Bug in reload_cse_move2add()


On Jan  4, 2001, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Jan  4, 2001, Joern Rennecke <amylaar@redhat.com> wrote:
>>> Good idea!  But what if the earliest register itself is incremented?
>>> (I haven't studied your patch yet)

>> In that case, the optimization would be disabled - because the luid is
>> incremented.  But I think that should hardly ever happen, so little
>> potential performance should be lost.

> How about not modifying the luid of the earliest register, but instead
> just modifying its offset?  The assumption is that the earliest
> register will always have an initial offset of zero, so we can tell by
> how much it has changed since the initial copy.

Here's a patch that implements this change.  It fixes the bug that
showed up in ARM bootstraps (Richard, could you please give it a spin
on your arm-netbsd, just to make sure?), and introduces the potential
for a number of interesting optimizations.  All register that differ
from an initial register by a constant are not tracked as a whole and,
even if the base register dies, their values are still correctly
tracked.  It also fixes the bug that motivated the original patch
posted on Jan 1st, following the general direction suggested by
Joern, but with a few developments by myself.  This bootstrapped on
i686-pc-linux-gnu (and the code generated for passing pointers into
the stack as arguments to functions looks quite nice!),
alphaev6-unkwown-linux-gnu (except for libstdc++-v3) and built all
target libraries for arm-elf.

Ok to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>, J"orn Rennecke <amylaar@redhat.com>

	* reload1.c (move2add_note_store): Treat all registers about which
	no information is known as potential bases, and treat all
	registers directly or indirectly derived from it as members of the
	same set of values.
	(reload_cse_move2add): Adjust accordingly.  Take offset of base
	register into account.

Index: gcc/reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.253
diff -u -p -r1.253 reload1.c
--- gcc/reload1.c 2001/01/13 20:18:20 1.253
+++ gcc/reload1.c 2001/01/15 19:19:02
@@ -8950,8 +8950,9 @@ reload_combine_note_use (xp, insn)
     }
 }
 
-/* See if we can reduce the cost of a constant by replacing a move with
-   an add.  */
+/* See if we can reduce the cost of a constant by replacing a move
+   with an add.  We track situations in which a register is set to a
+   constant or to a register plus a constant.  */
 /* We cannot do our optimization across labels.  Invalidating all the
    information about register contents we have would be costly, so we
    use last_label_luid (local variable of reload_cse_move2add) to note
@@ -8977,6 +8978,10 @@ static enum machine_mode reg_mode[FIRST_
    reload_cse_move2add and move2add_note_store.  */
 static int move2add_luid;
 
+/* move2add_last_label_uid is set whenever a label is found.  Labels
+   invalidate all previously collected reg_offset data.  */
+static int move2add_last_label_luid;
+
 /* Generate a CONST_INT and force it in the range of MODE.  */
 
 static rtx
@@ -9002,19 +9007,18 @@ reload_cse_move2add (first)
 {
   int i;
   rtx insn;
-  int last_label_luid;
 
   for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; i--)
     reg_set_luid[i] = 0;
 
-  last_label_luid = 0;
+  move2add_last_label_luid = 0;
   move2add_luid = 1;
   for (insn = first; insn; insn = NEXT_INSN (insn), move2add_luid++)
     {
       rtx pat, note;
 
       if (GET_CODE (insn) == CODE_LABEL)
-	last_label_luid = move2add_luid;
+	move2add_last_label_luid = move2add_luid;
       if (! INSN_P (insn))
 	continue;
       pat = PATTERN (insn);
@@ -9031,7 +9035,7 @@ reload_cse_move2add (first)
 	     register in the mode of REG.  */
 	  /* ??? We don't know how zero / sign extension is handled, hence
 	     we can't go from a narrower to a wider mode.  */
-	  if (reg_set_luid[regno] > last_label_luid
+	  if (reg_set_luid[regno] > move2add_last_label_luid
 	      && ((GET_MODE_SIZE (GET_MODE (reg))
 		   == GET_MODE_SIZE (reg_mode[regno]))
 		  || ((GET_MODE_SIZE (GET_MODE (reg))
@@ -9083,25 +9087,32 @@ reload_cse_move2add (first)
 				  ...
 				  (set (REGX) (plus (REGX) (CONST_INT B-A)))  */
 	      else if (GET_CODE (src) == REG
-		       && reg_base_reg[regno] == (int) REGNO (src)
-		       && reg_set_luid[regno] > reg_set_luid[REGNO (src)])
+		       && reg_set_luid[regno] == reg_set_luid[REGNO (src)]
+		       && reg_base_reg[regno] == reg_base_reg[REGNO (src)])
 		{
 		  rtx next = next_nonnote_insn (insn);
 		  rtx set = NULL_RTX;
 		  if (next)
 		    set = single_set (next);
-		  if (next
-		      && set
+		  if (set
 		      && SET_DEST (set) == reg
 		      && GET_CODE (SET_SRC (set)) == PLUS
 		      && XEXP (SET_SRC (set), 0) == reg
 		      && GET_CODE (XEXP (SET_SRC (set), 1)) == CONST_INT)
 		    {
 		      rtx src3 = XEXP (SET_SRC (set), 1);
-		      rtx new_src
-			= gen_mode_int (GET_MODE (reg),
-					INTVAL (src3)
-					- INTVAL (reg_offset[regno]));
+		      HOST_WIDE_INT added_offset = INTVAL (src3);
+		      HOST_WIDE_INT base_offset
+			= (reg_base_reg[REGNO (src)] >= 0
+			   && reg_offset[REGNO (src)]
+			   && GET_CODE (reg_offset[REGNO (src)]) == CONST_INT
+			   ? INTVAL (reg_offset[REGNO (src)])
+			   : 0);
+		      HOST_WIDE_INT regno_offset = INTVAL (reg_offset[regno]);
+		      rtx new_src = gen_mode_int (GET_MODE (reg),
+						  added_offset
+						  + base_offset
+						  - regno_offset);
 		      int success = 0;
 
 		      if (new_src == const0_rtx)
@@ -9124,9 +9135,10 @@ reload_cse_move2add (first)
 			  NOTE_SOURCE_FILE (insn) = 0;
 			}
 		      insn = next;
-		      reg_set_luid[regno] = move2add_luid;
 		      reg_mode[regno] = GET_MODE (reg);
-		      reg_offset[regno] = src3;
+		      reg_offset[regno] = gen_mode_int (GET_MODE (reg),
+							added_offset
+							+ base_offset);
 		      continue;
 		    }
 		}
@@ -9138,13 +9150,14 @@ reload_cse_move2add (first)
 	  if (REG_NOTE_KIND (note) == REG_INC
 	      && GET_CODE (XEXP (note, 0)) == REG)
 	    {
-	      /* Indicate that this register has been recently written to,
-		 but the exact contents are not available.  */
+	      /* Reset the information about this register, setting it
+		 up as a potential base.  */
 	      int regno = REGNO (XEXP (note, 0));
 	      if (regno < FIRST_PSEUDO_REGISTER)
 		{
+		  reg_base_reg[regno] = regno;
 		  reg_set_luid[regno] = move2add_luid;
-		  reg_offset[regno] = note;
+		  reg_offset[regno] = const0_rtx;
 		}
 	    }
 	}
@@ -9157,8 +9170,11 @@ reload_cse_move2add (first)
 	    {
 	      if (call_used_regs[i])
 		{
+		  /* Reset the information about this register,
+		     setting it up as a potential base.  */
+		  reg_base_reg[i] = i;
 		  reg_set_luid[i] = move2add_luid;
-		  reg_offset[i] = insn;	/* Invalidate contents.  */
+		  reg_offset[i] = const0_rtx;	/* Invalidate contents.  */
 		}
 	    }
 	}
@@ -9193,8 +9209,9 @@ move2add_note_store (dst, set, data)
 	  || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC)
 	{
 	  regno = REGNO (XEXP (dst, 0));
+	  reg_base_reg[regno] = regno;
 	  reg_set_luid[regno] = move2add_luid;
-	  reg_offset[regno] = dst;
+	  reg_offset[regno] = const0_rtx;
 	}
       return;
     }
@@ -9209,42 +9226,71 @@ move2add_note_store (dst, set, data)
       && GET_CODE (SET_DEST (set)) != STRICT_LOW_PART)
     {
       rtx src = SET_SRC (set);
+      rtx base_reg, offset;
+      int base_regno;
 
       reg_mode[regno] = mode;
       switch (GET_CODE (src))
 	{
 	case PLUS:
 	  {
-	    rtx src0 = XEXP (src, 0);
-
-	    if (GET_CODE (src0) == REG)
+	    if (GET_CODE (XEXP (src, 0)) == REG
+		&& GET_CODE (XEXP (src, 1)) == CONST_INT)
 	      {
-		if (REGNO (src0) != regno
-		    || reg_offset[regno] != const0_rtx)
-		  reg_base_reg[regno] = REGNO (src0);
-
+		base_reg = XEXP (src, 0);
+		offset = XEXP (src, 1);
+	      }
+	    else
+	      {
+		reg_base_reg[regno] = regno;
+		reg_offset[regno] = const0_rtx;
 		reg_set_luid[regno] = move2add_luid;
-		reg_offset[regno] = XEXP (src, 1);
-		break;
+		return;
 	      }
 
-	    reg_set_luid[regno] = move2add_luid;
-	    reg_offset[regno] = set;	/* Invalidate contents.  */
 	    break;
 	  }
 
 	case REG:
-	  reg_base_reg[regno] = REGNO (SET_SRC (set));
-	  reg_offset[regno] = const0_rtx;
-	  reg_set_luid[regno] = move2add_luid;
+	  base_reg = src;
+	  offset = const0_rtx;
 	  break;
 
 	default:
+	  /* Invalidate the register, except in case SET_SRC (set) is
+	     a CONST_INT, in which case the register will start being
+	     tracked as a constant.  */
 	  reg_base_reg[regno] = -1;
 	  reg_offset[regno] = SET_SRC (set);
 	  reg_set_luid[regno] = move2add_luid;
-	  break;
+	  return;
+	}
+
+      base_regno = REGNO (base_reg);
+      /* If information about the base register is not valid, set it
+	 up as a new base register, pretending its value is know
+	 starting in the current insn.  */
+      if (reg_set_luid[base_regno] <= move2add_last_label_luid)
+	{
+	  reg_base_reg[base_regno] = base_regno;
+	  reg_offset[base_regno] = const0_rtx;
+	  reg_set_luid[base_regno] = move2add_luid;
 	}
+
+      /* Copy base information from our base register.  */
+      reg_base_reg[regno] = reg_base_reg[base_regno];
+      reg_set_luid[regno] = reg_set_luid[base_regno];
+
+      /* If our base register is based on some other register, compute
+	 the sum of the offsets.  Otherwise, use the offset of the
+	 current insn only.  */
+      if (reg_base_reg[base_regno] >= 0
+	  && reg_offset[base_regno]
+	  && GET_CODE (reg_offset[base_regno]) == CONST_INT)
+	reg_offset[regno]
+	  = plus_constant (offset, INTVAL (reg_offset[base_regno]));
+      else
+	reg_offset[regno] = offset;
     }
   else
     {
@@ -9252,10 +9298,11 @@ move2add_note_store (dst, set, data)
 
       for (i = regno; i < endregno; i++)
 	{
-	  /* Indicate that this register has been recently written to,
-	     but the exact contents are not available.  */
+	  /* Reset information about this register, setting it up as a
+	     potential base.  */
+	  reg_base_reg[i] = i;
 	  reg_set_luid[i] = move2add_luid;
-	  reg_offset[i] = dst;
+	  reg_offset[i] = const0_rtx;
 	}
     }
 }

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]