RFA: reload bug fix (take 4) (Was: Re: RFC: reloading sums)

Joern Rennecke joern.rennecke@superh.com
Thu Jun 26 15:26:00 GMT 2003


Jim Wilson wrote:
> The patch needs in->code replaced with GET_CODE (in).  There should be a
> comment mentioning that these PLUS RTL come from find_reloads_address,
> and that they get fixed inside gen_reload.  The debugging code needs to
> be removed.

I have already done these code changes in:
http://gcc.gnu.org/ml/gcc-patches/2003-06/msg02398.html
> 
> I think "notional" is supposed to be "optional".

No, it's technically a non-optional reload, but there is no actual
input reloading to do.  I've put a clarification in the comments.
	
> 	  "addres" should be
> "address".

Yes, changed.  I have attached the amended patch (only comment changes from
the previous version in the URL quoted above).
	
-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658
-------------- next part --------------
2003-06-26  J"orn Rennecke <joern.rennecke@superh.com>

gcc:
	* reload.c (can_reload_into): New function.
	(push_reload): Use it.
gcc/testsuite:
	* gcc.c-torture/execute/multi-ix.c: New test.

Index: reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.214
diff -p -r1.214 reload.c
*** reload.c	12 Jun 2003 19:01:08 -0000	1.214
--- reload.c	26 Jun 2003 13:35:24 -0000
*************** reload_inner_reg_of_subreg (x, mode, out
*** 840,845 ****
--- 840,895 ----
  	      != (int) HARD_REGNO_NREGS (REGNO (inner), GET_MODE (inner))));
  }
  
+ /* Return nonzero if IN can be reloaded into REGNO with mode MODE without
+    requiring an extra reload register.  The caller has already found that
+    IN contains some reference to REGNO, so check that we can produce the
+    new value in a single step.  E.g. if we have
+    (set (reg r13) (plus (reg r13) (const int 1))), and there is an
+    instruction that adds one to a register, this should succeed.
+    However, if we have something like
+    (set (reg r13) (plus (reg r13) (const int 999))), and the constant 999
+    needs to be loaded into a register first, we need a separate reload
+    register.
+    Such PLUS reloads are generated by find_reload_address_part.
+    The out-of-range PLUS expressions are usually introduced in the instruction
+    patterns by register elimination and substituting pseudos without a home
+    by their function-invariant equivalences.  */
+ static int
+ can_reload_into (rtx in, int regno, enum machine_mode mode)
+ {
+   rtx dst, test_insn;
+   int r = 0;
+   struct recog_data save_recog_data;
+ 
+   /* For matching constraints, we often get notional input reloads where
+      we want to use the original register as the reload register.  I.e.
+      technically this is a non-optional input-output reload, but IN is
+      already a valid register, and has been chosen as the reload register.
+      Speed this up, since it trivially works.  */
+   if (GET_CODE (in) == REG)
+     return 1;
+ 
+   /* To test MEMs properly, we'd have to take into account all the reloads
+      that are already scheduled, which can become quite complicated.
+      And since we've already handled address reloads for this MEM, it
+      should always succeed anyway.  */
+   if (GET_CODE (in) == MEM)
+     return 1;
+ 
+   /* If we can make a simple SET insn that does the job, everything should
+      be fine.  */
+   dst =  gen_rtx_REG (mode, regno);
+   test_insn = make_insn_raw (gen_rtx_SET (VOIDmode, dst, in));
+   save_recog_data = recog_data;
+   if (recog_memoized (test_insn) >= 0)
+     {
+       extract_insn (test_insn);
+       r = constrain_operands (1);
+     }
+   recog_data = save_recog_data;
+   return r;
+ }
+ 
  /* Record one reload that needs to be performed.
     IN is an rtx saying where the data are to be found before this instruction.
     OUT says where they must be stored after the instruction.
*************** push_reload (in, out, inloc, outloc, cla
*** 1532,1538 ****
  					  regno + offs))
  		break;
  
! 	    if (offs == nregs)
  	      {
  		rld[i].reg_rtx = gen_rtx_REG (rel_mode, regno);
  		break;
--- 1582,1592 ----
  					  regno + offs))
  		break;
  
! 	    if (offs == nregs
! 		&& (! (refers_to_regno_for_reload_p
! 		       (regno, (regno + HARD_REGNO_NREGS (regno, inmode)),
! 				in, (rtx *)0))
! 		    || can_reload_into (in, regno, inmode)))
  	      {
  		rld[i].reg_rtx = gen_rtx_REG (rel_mode, regno);
  		break;
*** /dev/null	Thu Aug 30 21:30:55 2001
--- testsuite/gcc.c-torture/execute/multi-ix.c	Fri Jun 20 14:22:44 2003
***************
*** 0 ****
--- 1,181 ----
+ /* Test for a reload bug:
+    if you have a memory reference using the indexed addressing
+    mode, and the base address is a pseudo containing an address in the frame
+    and this pseudo fails to get a hard register, we end up with a double PLUS,
+    so the frame address gets reloaded.  Now, when the index got a hard register,
+    and it dies in this insn, push_reload will consider that hard register as
+    a reload register, and disregrad overlaps with rld[n_reloads].in .  That is
+    fine as long as the add can be done with a single insn, but when the
+    constant is so large that it has to be reloaded into a register first,
+    that clobbers the index.  */
+ 
+ #include <stdarg.h>
+ 
+ #ifdef STACK_SIZE
+ #define CHUNK ((STACK_SIZE-100)/40/sizeof(int))
+ #else
+ #define CHUNK 500
+ #endif
+ 
+ void s(int, ...);
+ void z(int, ...);
+ void c(int, ...);
+ 
+ typedef int l[CHUNK];
+ 
+ void
+ f (int n)
+ {
+   int i;
+   l a0, a1, a2, a3, a4, a5, a6, a7, a8, a9;
+   l a10, a11, a12, a13, a14, a15, a16, a17, a18, a19;
+   l a20, a21, a22, a23, a24, a25, a26, a27, a28, a29;
+   l a30, a31, a32, a33, a34, a35, a36, a37, a38, a39;
+   int i0, i1, i2, i3, i4, i5, i6, i7, i8, i9;
+   int i10, i11, i12, i13, i14, i15, i16, i17, i18, i19;
+   int i20, i21, i22, i23, i24, i25, i26, i27, i28, i29;
+   int i30, i31, i32, i33, i34, i35, i36, i37, i38, i39;
+ 
+   for (i = 0; i < n; i++)
+     {
+       s (40, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9,
+ 	 a10, a11, a12, a13, a14, a15, a16, a17, a18, a19,
+          a20, a21, a22, a23, a24, a25, a26, a27, a28, a29,
+          a30, a31, a32, a33, a34, a35, a36, a37, a38, a39);
+       i0 = a0[0];
+       i1 = a1[0];
+       i2 = a2[0];
+       i3 = a3[0];
+       i4 = a4[0];
+       i5 = a5[0];
+       i6 = a6[0];
+       i7 = a7[0];
+       i8 = a8[0];
+       i9 = a9[0];
+       i10 = a10[0];
+       i11 = a11[0];
+       i12 = a12[0];
+       i13 = a13[0];
+       i14 = a14[0];
+       i15 = a15[0];
+       i16 = a16[0];
+       i17 = a17[0];
+       i18 = a18[0];
+       i19 = a19[0];
+       i20 = a20[0];
+       i21 = a21[0];
+       i22 = a22[0];
+       i23 = a23[0];
+       i24 = a24[0];
+       i25 = a25[0];
+       i26 = a26[0];
+       i27 = a27[0];
+       i28 = a28[0];
+       i29 = a29[0];
+       i30 = a30[0];
+       i31 = a31[0];
+       i32 = a32[0];
+       i33 = a33[0];
+       i34 = a34[0];
+       i35 = a35[0];
+       i36 = a36[0];
+       i37 = a37[0];
+       i38 = a38[0];
+       i39 = a39[0];
+       z (40, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9,
+ 	 a10, a11, a12, a13, a14, a15, a16, a17, a18, a19,
+          a20, a21, a22, a23, a24, a25, a26, a27, a28, a29,
+          a30, a31, a32, a33, a34, a35, a36, a37, a38, a39);
+       a0[i0] = i0;
+       a1[i1] = i1;
+       a2[i2] = i2;
+       a3[i3] = i3;
+       a4[i4] = i4;
+       a5[i5] = i5;
+       a6[i6] = i6;
+       a7[i7] = i7;
+       a8[i8] = i8;
+       a9[i9] = i9;
+       a10[i10] = i10;
+       a11[i11] = i11;
+       a12[i12] = i12;
+       a13[i13] = i13;
+       a14[i14] = i14;
+       a15[i15] = i15;
+       a16[i16] = i16;
+       a17[i17] = i17;
+       a18[i18] = i18;
+       a19[i19] = i19;
+       a20[i20] = i20;
+       a21[i21] = i21;
+       a22[i22] = i22;
+       a23[i23] = i23;
+       a24[i24] = i24;
+       a25[i25] = i25;
+       a26[i26] = i26;
+       a27[i27] = i27;
+       a28[i28] = i28;
+       a29[i29] = i29;
+       a30[i30] = i30;
+       a31[i31] = i31;
+       a32[i32] = i32;
+       a33[i33] = i33;
+       a34[i34] = i34;
+       a35[i35] = i35;
+       a36[i36] = i36;
+       a37[i37] = i37;
+       a38[i38] = i38;
+       a39[i39] = i39;
+       c (40, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9,
+ 	 a10, a11, a12, a13, a14, a15, a16, a17, a18, a19,
+          a20, a21, a22, a23, a24, a25, a26, a27, a28, a29,
+          a30, a31, a32, a33, a34, a35, a36, a37, a38, a39);
+     }
+ }
+ 
+ int
+ main ()
+ {
+   f (1);
+   exit (0);
+ }
+ 
+ void s(int n, ...)
+ {
+   va_list list;
+ 
+   va_start (list, n);
+   while (--n)
+     {
+       int *a = va_arg (list, int *);
+       a[0] = n;
+     }
+   va_end (list);
+ }
+ 
+ void z(int n, ...)
+ {
+   va_list list;
+ 
+   va_start (list, n);
+   while (--n)
+     {
+       int *a = va_arg (list, int *);
+       bzero (a, sizeof (l));
+     }
+   va_end (list);
+ }
+ 
+ void c(int n, ...)
+ {
+   va_list list;
+ 
+   va_start (list, n);
+   while (--n)
+     {
+       int *a = va_arg (list, int *);
+       if (a[n] != n)
+ 	abort ();
+     }
+   va_end (list);
+ }


More information about the Gcc-patches mailing list