diff mbox

[tentative,reload] Make push_reload work with more types of subregs?

Message ID 87oa53gu46.fsf@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj Aug. 8, 2016, 3:26 p.m. UTC 
Bernd Schmidt writes:

> On 07/28/2016 09:33 AM, Senthil Kumar Selvaraj wrote:
>>
>>   Is there a reason why only REG and SYMBOL_REFs get valid
>>   subreg_in_class? I tried extending it handle constants and PLUS
>>   expressions, and it fixes PR 71873. It also fixes a another
>>   bug that was a work around for the reload failure (PR 64452) - that
>>   had a plus expression instead of the const.
>>
>>   Reg testing on avr and x86_64 did not show any new failures. Is this
>>   the right way to fix this?
>
> I think it looks quite plausible. Note that testing x86_64 on trunk will 
> not do anything - it is no longer using reload. Could you go back to an 
> older branch (4.7 I think is the last one using reload) and retest 
> x86_64 with that, for better test coverage?

I picked out the commit where you'd added SYMBOL_REF handling (rev
#190252), and patched that with the below code. Bootstrapped and
regtested on x86_64-pc-linux - the results were identical with and 
without the patch. Is this good enough for trunk?

Regards
Senthil

gcc/ChangeLog:

2016-08-08  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

  PR reload/71873
	* reload.c (push_reload): Compute subreg_in_class for
  subregs of constants and plus expressions.

gcc/testsuite/ChangeLog:

2016-08-08  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* gcc.target/avr/pr71873.c: New test.

Comments

Bernd Schmidt Aug. 9, 2016, 12:14 p.m. UTC | #1 
On 08/08/2016 05:26 PM, Senthil Kumar Selvaraj wrote:

> I picked out the commit where you'd added SYMBOL_REF handling (rev
> #190252), and patched that with the below code. Bootstrapped and
> regtested on x86_64-pc-linux - the results were identical with and
> without the patch. Is this good enough for trunk?

> -      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
> +      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF
> +               || CONSTANT_P (SUBREG_REG (in))
> +               || GET_CODE (SUBREG_REG (in)) == PLUS)
>  	subreg_in_class = find_valid_class_1 (inmode,
>  					      GET_MODE (SUBREG_REG (in)),
>  					      rclass);

Actually SYMBOL_REF should also be CONSTANT_P. For integers it looks 
like it'll pass VOIDmode to find_valid_class_1 and just return NO_REGS. 
which I suppose is OK.

Would you mind removing the SYMBOL_REF test and retesting? Ok with that 
change.


Bernd
diff mbox

Patch

Index: gcc/reload.c
===================================================================
--- gcc/reload.c	(revision 239239)
+++ gcc/reload.c	(working copy)
@@ -1141,7 +1141,9 @@ 
 						   SUBREG_BYTE (in),
 						   GET_MODE (in)),
 			      REGNO (SUBREG_REG (in)));
-      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
+      else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF
+               || CONSTANT_P (SUBREG_REG (in))
+               || GET_CODE (SUBREG_REG (in)) == PLUS)
 	subreg_in_class = find_valid_class_1 (inmode,
 					      GET_MODE (SUBREG_REG (in)),
 					      rclass);
Index: gcc/testsuite/gcc.target/avr/pr71873.c
===================================================================
--- gcc/testsuite/gcc.target/avr/pr71873.c	(nonexistent)
+++ gcc/testsuite/gcc.target/avr/pr71873.c	(working copy)
@@ -0,0 +1,30 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Os -fcaller-saves" } */
+
+#include <stdint.h>
+
+typedef struct {
+  uint8_t x;
+  uint32_t y;
+} A;
+
+A a;
+
+extern int bar(int);
+extern int foo (char *s, ...);
+
+extern uint8_t param;
+extern uint8_t h,m,s,ld,lm;
+extern uint16_t d;
+
+void gps_parse_string(int z)
+{
+  while (bar(z))
+  {
+    switch (param)
+    {
+      case 0: foo("a", &h, &m, &s, &d); break;
+      case 1: foo("d", &ld, &lm, &a.y); break;
+    }
+  }
+}