Patchwork Fix reload inheritance of post-modified addresses.

login
register
mail settings
Submitter Bernd Schmidt
Date Jan. 23, 2011, 9:13 p.m.
Message ID <4D3C99E1.6020904@codesourcery.com>
Download mbox | patch
Permalink /patch/80071/
State New
Headers show

Comments

Bernd Schmidt - Jan. 23, 2011, 9:13 p.m.
On 01/12/2011 04:33 PM, Richard Sandiford wrote:
> Bernd Schmidt <bernds@codesourcery.com> writes:
>> On 01/12/2011 03:31 PM, Richard Sandiford wrote:
>>> Ah. :-)  What went wrong with PRE_MODIFY?  That big comment was trying to
>>> justify why PRE_MODIFY was actually OK...
>>
>> One problem was that inc_for_reload returned the wrong insn. It
>> generated a sequence
>>   r1 = r1 + 280
>>   r3 = r1
>>
>> with r1 being the reload reg for the inner pseudo, r3 the reload reg for
>> the PRE_MODIFY. It returns the add insn, causing us to set
>> spill_reg_store for R3 to an insn that sets R1. If that's fixed, the
>> other problem is that spill_reg_stored_to is set incorrectly. Presumably
>> it should be set to R3 here, but rather than add more interdependencies
>> I decided it would be best just to skip this code for PRE_MODIFY as well.
> 
> Ah, right.  In that case I'll defer to your patch.

Please excuse the delay, I was travelling. I committed the following
version (with the comment cut down a bit) after regression testing on
QEMU arm-linux, and bootstrapping & testing on i686-linux.

Will commit on the 4.5 branch in a few days as well.


Bernd

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 169143)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@ 
+2011-01-23  Bernd Schmidt  <bernds@codesourcery.com>
+	    Richard Sandiford  <rdsandiford@googlemail.com>
+
+	PR rtl-optimization/47166
+	* reload1.c (emit_reload_insns): Disable the spill_reg_store
+	mechanism for PRE_MODIFY and POST_MODIFY.
+	(inc_for_reload): For PRE_MODIFY, return the insn that sets the
+	reloadreg.
+
 2011-01-23  Andreas Schwab  <schwab@linux-m68k.org>
 
 	* compare-elim.c (maybe_select_cc_mode): Add ATTRIBUTE_UNUSED
Index: reload1.c
===================================================================
--- reload1.c	(revision 169143)
+++ reload1.c	(working copy)
@@ -1,7 +1,7 @@ 
 /* Reload pseudo regs into hard regs for insns that require hard regs.
    Copyright (C) 1987, 1988, 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
-   Free Software Foundation, Inc.
+   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
+   2011 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -8086,10 +8086,22 @@  emit_reload_insns (struct insn_chain *ch
 	  /* Maybe the spill reg contains a copy of reload_out.  */
 	  if (rld[r].out != 0
 	      && (REG_P (rld[r].out)
-#ifdef AUTO_INC_DEC
-		  || ! rld[r].out_reg
-#endif
-		  || REG_P (rld[r].out_reg)))
+		  || (rld[r].out_reg
+		      ? REG_P (rld[r].out_reg)
+		      /* The reload value is an auto-modification of
+			 some kind.  For PRE_INC, POST_INC, PRE_DEC
+			 and POST_DEC, we record an equivalence
+			 between the reload register and the operand
+			 on the optimistic assumption that we can make
+			 the equivalence hold.  reload_as_needed must
+			 then either make it hold or invalidate the
+			 equivalence.
+
+			 PRE_MODIFY and POST_MODIFY addresses are reloaded
+			 somewhat differently, and allowing them here leads
+			 to problems.  */
+		      : (GET_CODE (rld[r].out) != POST_MODIFY
+			 && GET_CODE (rld[r].out) != PRE_MODIFY))))
 	    {
 	      rtx reg;
 	      enum machine_mode mode;
@@ -9033,7 +9045,7 @@  inc_for_reload (rtx reloadreg, rtx in, r
 		 be used as an address.  */
 
 	      if (! post)
-		emit_insn (gen_move_insn (reloadreg, incloc));
+		add_insn = emit_insn (gen_move_insn (reloadreg, incloc));
 
 	      return add_insn;
 	    }