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]
Other format: [Raw text]

[PATCH/RFC] Avoid invalid subregs during reload, call for testers


   Hi.

   This patch tries to address two cases where reload would call
subreg_regno_offset() with invalid parameters. It also adds a gcc_assert()
to catch new uses of subreg_regno_offset() with invalid parameters.

   The first case where reload would call subreg_regno_offset() with invalid
parameters is when reloading a subreg of a hard reg with offset != 0. The
details are in the thread at
<URL:http://gcc.gnu.org/ml/gcc/2006-11/msg00821.html>. The fix is to leave
the byte offset at 0 because subreg_reg() already finds the hard reg we want.

   The second case is when trying to inherit a reload of a subreg of a
pseudo reg. This occurs when compiling _muldc3.o on i386 (with the proposed
patch to rtlanal.c to assert validity of subreg_regno_offset() parameters):

(gdb) frame 1           
#1  0x083f3411 in subreg_regno_offset (xregno=15, xmode=DFmode, offset=4, ymode=SImode) at ../../../src/gcc/gcc/rtlanal.c:2934
2934      gcc_assert (subreg_offset_representable_p (xregno, xmode, offset, ymode));
(gdb) print reg_names[xregno]
$2 = 0x86441ec "st(7)"

   There's no way to extract that SImode part of the x87 floating point
register %st(7). Let's see how we got here:

(gdb) frame 2
#2  0x083ea84e in choose_reload_regs (chain=0x8767bc8) at ../../../src/gcc/gcc/reload1.c:5657
5657                      i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);

(gdb) call debug_rtx (chain->insn)
(insn:HI 98 97 99 13 ../../../src/gcc/gcc/libgcc2.c:1844 (set (reg:CCGOC 17 flags)
        (compare:CCGOC (subreg:SI (reg/v:DF 82 [ b ]) 4)
            (const_int 0 [0x0]))) 0 {*cmpsi_ccno_1} (nil)
    (expr_list:REG_DEAD (reg/v:DF 82 [ b ])
        (nil)))

(gdb) call debug_reload()
Reload 0: reload_in (SI) = (subreg:SI (reg/v:DF 82 [ b ]) 4)
        GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 0), optional
        reload_in_reg: (subreg:SI (reg/v:DF 82 [ b ]) 4)

(gdb) print regno
$1 = 82
(gdb) call debug_rtx (reg_last_reload_reg[82])
(reg:DF 15 st(7))
(gdb) call debug_rtx (last_reg)          
(reg:DF 15 st(7))

   So the pseudo reg 82 had already been reloaded into hard reg 15, but we
have no chance of accessing subregs of it there. The fix is to check
subreg_offset_representable() before trying to inherit the reload.

   This passes bootstrap and testing on i686-pc-linux-gnu with all default
languages with no new failures. I would appreciate help in testing on other
targets.

   (If any of this is approved, please also commit it for me. I don't have
SVN write access.)

ChangeLog:

2006-12-04  Rask Ingemann Lambertsen  <rask@sygehus.dk>

	* reload1.c (choose_reload_regs): Don't set byte offset when
	resolving subregs of hard regs.  When trying to inherit a subreg of
	a reload, first check that the subreg is representable.

	* rtlanal.c (subreg_regno_offset): Assert that the subreg is
	representable.

Index: reload1.c
===================================================================
--- reload1.c	(revision 119451)
+++ reload1.c	(working copy)
@@ -5622,10 +5622,11 @@ choose_reload_regs (struct insn_chain *c
 	      else if (GET_CODE (rld[r].in_reg) == SUBREG
 		       && REG_P (SUBREG_REG (rld[r].in_reg)))
 		{
-		  byte = SUBREG_BYTE (rld[r].in_reg);
 		  regno = REGNO (SUBREG_REG (rld[r].in_reg));
 		  if (regno < FIRST_PSEUDO_REGISTER)
 		    regno = subreg_regno (rld[r].in_reg);
+		  else
+		    byte = SUBREG_BYTE (rld[r].in_reg);
 		  mode = GET_MODE (rld[r].in_reg);
 		}
 #ifdef AUTO_INC_DEC
@@ -5651,9 +5652,15 @@ choose_reload_regs (struct insn_chain *c
 		  enum reg_class class = rld[r].class, last_class;
 		  rtx last_reg = reg_last_reload_reg[regno];
 		  enum machine_mode need_mode;
+		  bool invalid_subreg = false;
 
 		  i = REGNO (last_reg);
-		  i += subreg_regno_offset (i, GET_MODE (last_reg), byte, mode);
+		  if (subreg_offset_representable_p (i, GET_MODE (last_reg),
+						     byte, mode))
+		    i += subreg_regno_offset (i, GET_MODE (last_reg), byte,
+					      mode);
+		  else
+		    invalid_subreg = true;
 		  last_class = REGNO_REG_CLASS (i);
 
 		  if (byte == 0)
@@ -5664,8 +5671,9 @@ choose_reload_regs (struct insn_chain *c
 						+ byte * BITS_PER_UNIT,
 						GET_MODE_CLASS (mode));
 
-		  if ((GET_MODE_SIZE (GET_MODE (last_reg))
-		       >= GET_MODE_SIZE (need_mode))
+		  if (!invalid_subreg
+		      && (GET_MODE_SIZE (GET_MODE (last_reg))
+			  >= GET_MODE_SIZE (need_mode))
 #ifdef CANNOT_CHANGE_MODE_CLASS
 		      /* Verify that the register in "i" can be obtained
 			 from LAST_REG.  */
Index: rtlanal.c
===================================================================
--- rtlanal.c	(revision 119451)
+++ rtlanal.c	(working copy)
@@ -2930,6 +2930,7 @@ subreg_regno_offset (unsigned int xregno
   int y_offset;
 
   gcc_assert (xregno < FIRST_PSEUDO_REGISTER);
+  gcc_assert (subreg_offset_representable_p (xregno, xmode, offset, ymode));
 
   /* Adjust nregs_xmode to allow for 'holes'.  */
   if (HARD_REGNO_NREGS_HAS_PADDING (xregno, xmode))

-- 
Rask Ingemann Lambertsen


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