Re: [Patch] Fix PR 60040

2016-04-29 Thread Bernd Schmidt

On 04/28/2016 10:07 AM, Senthil Kumar Selvaraj wrote:


Here's the patch with the extra bits removed.


To get it some additional test coverage, I've tested it on 
gcc-4_7-branch (with another backport so that it applies) with an x86_64 
bootstrap and test. That worked, so I installed it on trunk.



Bernd


Re: [Patch] Fix PR 60040

2016-04-28 Thread Senthil Kumar Selvaraj

Joern Wolfgang Rennecke writes:

> On 28/04/16 07:57, Senthil Kumar Selvaraj wrote:
>> diff --git libcilkrts/ChangeLog libcilkrts/ChangeLog
>> index 8fada8a..ed26a3a 100644
>> --- libcilkrts/ChangeLog
>> +++ libcilkrts/ChangeLog
>> @@ -1,9 +1,3 @@
>> -2016-04-26  Rainer Orth  
>> -
>> -PR target/60290
>> -* Makefile.am (GENERAL_FLAGS): Add -funwind-tables.
>> -* Makefile.in: Regenerate.
>> -
>>   2015-11-09  Igor Zamyatin  
>>   
>>  PR target/66326
> This does not appear related, and it would be the wrong way to back out 
> a patch in the FSF repo
> even if you wanted to, so I suppose this must be unintentional.

Yup, I should have removed those. Sorry about that.

Here's the patch with the extra bits removed.

gcc/ChangeLog

2016-04-28  Senthil Kumar Selvaraj  

PR target/60040
* reload1.c (reload): Call finish_spills before
restarting reload loop. Skip select_reload_regs
if update_eliminables_and_spill returns true.

gcc/testsuite/ChangeLog

2016-04-28  Sebastian Huber  
Matthijs Kooijman  
Senthil Kumar Selvaraj  

PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: New.


diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..d6fcece 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -981,7 +981,8 @@ reload (rtx_insn *first, int global)
   /* If we allocated another stack slot, redo elimination bookkeeping.  */
   if (something_was_spilled || starting_frame_size != get_frame_size ())
{
- update_eliminables_and_spill ();
+ if (update_eliminables_and_spill ())
+   finish_spills (global);
  continue;
}
 
@@ -1021,10 +1022,12 @@ reload (rtx_insn *first, int global)
  did_spill = 1;
  something_changed = 1;
}
-
-  select_reload_regs ();
-  if (failure)
-   goto failed;
+  else
+   {
+ select_reload_regs ();
+ if (failure)
+   goto failed;
+   }
 
   if (insns_need_reload != 0 || did_spill)
something_changed |= finish_spills (global);
diff --git gcc/testsuite/gcc.target/avr/pr60040-1.c 
gcc/testsuite/gcc.target/avr/pr60040-1.c
new file mode 100644
index 000..4fe296b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+
+float dhistory[10];
+float test;
+
+float getSlope(float history[]) {
+  float sumx = 0;
+  float sumy = 0;
+  float sumxy = 0;
+  float sumxsq = 0;
+  float rate = 0;
+  int n = 10;
+
+  int i;
+  for (i=1; i< 11; i++) {
+sumx = sumx + i;
+sumy = sumy + history[i-1];
+sumy = sumy + history[i-1];
+sumxsq = sumxsq + (i*i);
+  }
+
+  rate = sumy+sumx+sumxsq;
+  return rate;
+}
+
+void loop() {
+  test = getSlope(dhistory);
+}
diff --git gcc/testsuite/gcc.target/avr/pr60040-2.c 
gcc/testsuite/gcc.target/avr/pr60040-2.c
new file mode 100644
index 000..c40d49f
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-2.c
@@ -0,0 +1,112 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned char __uint8_t;
+typedef short unsigned int __uint16_t;
+typedef long unsigned int __uint32_t;
+typedef __uint8_t uint8_t ;
+typedef __uint16_t uint16_t ;
+typedef __uint32_t uint32_t ;
+typedef __builtin_va_list __gnuc_va_list;
+typedef __gnuc_va_list va_list;
+typedef enum rtems_blkdev_request_op {
+  RTEMS_BLKDEV_REQ_READ,
+} rtems_fdisk_segment_desc;
+typedef struct rtems_fdisk_driver_handlers
+{
+  int (*blank) (const rtems_fdisk_segment_desc* sd,
+uint32_t device,
+uint32_t segment,
+uint32_t offset,
+uint32_t size);
+} rtems_fdisk_driver_handlers;
+typedef struct rtems_fdisk_page_desc
+{
+  uint16_t flags;
+  uint32_t block;
+} rtems_fdisk_page_desc;
+typedef struct rtems_fdisk_segment_ctl
+{
+  rtems_fdisk_page_desc* page_descriptors;
+  uint32_t pages_active;
+} rtems_fdisk_segment_ctl;
+typedef struct rtems_fdisk_segment_ctl_queue
+{
+} rtems_fdisk_segment_ctl_queue;
+typedef struct rtems_fdisk_device_ctl
+{
+  uint32_t flags;
+  uint8_t* copy_buffer;
+} rtems_flashdisk;
+
+extern void rtems_fdisk_error (const char *, ...);
+extern int rtems_fdisk_seg_write(const rtems_flashdisk*,
+ rtems_fdisk_segment_ctl*,
+ uint32_t,
+ const rtems_fdisk_page_desc* page_desc,
+uint32_t);
+
+void rtems_fdisk_printf (const rtems_flashdisk* fd, const char *format, ...)
+{
+  {
+va_list args;
+__builtin_va_start(args,format);
+  }
+}
+static int
+rtems_fdisk_seg_blank_check (const rtems_flashdisk* fd,
+ rtems_fdisk_segment_ctl* sc,
+ uint32_t offset,
+ uint32_t size)
+{
+  uint32_t device;
+  uint32_t segment;
+  const rtems_fdisk_segment_desc* sd;
+  const rtems_fdisk_driver_h

Re: [Patch] Fix PR 60040

2016-04-28 Thread Joern Wolfgang Rennecke



On 28/04/16 07:57, Senthil Kumar Selvaraj wrote:

diff --git libcilkrts/ChangeLog libcilkrts/ChangeLog
index 8fada8a..ed26a3a 100644
--- libcilkrts/ChangeLog
+++ libcilkrts/ChangeLog
@@ -1,9 +1,3 @@
-2016-04-26  Rainer Orth  
-
-   PR target/60290
-   * Makefile.am (GENERAL_FLAGS): Add -funwind-tables.
-   * Makefile.in: Regenerate.
-
  2015-11-09  Igor Zamyatin  
  
  	PR target/66326
This does not appear related, and it would be the wrong way to back out 
a patch in the FSF repo

even if you wanted to, so I suppose this must be unintentional.


Re: [Patch] Fix PR 60040

2016-04-27 Thread Senthil Kumar Selvaraj

Bernd Schmidt writes:

> On 04/15/2016 02:52 PM, Senthil Kumar Selvaraj wrote:
>>
>> For both testcases in the PR, reload fails to take into account that
>> FP-SP elimination can no longer be performed, and tries to find reload
>> regs for an rtx generated when FP-SP elimination was valid.
>>
>> 1. reload initializes elim table with FP->SP elimination enabled.
>> 2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
>> reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
>> something_was_spilled to true.
>> 3. The main reload loop starts, and it resets something_was_spilled to false.
>> 4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
>> (mem(SP + offset)).
>> 5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
>> SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
>> 6. update_eliminables_and_spill calls targetm.frame_pointer_required,
>> which returns true. That causes can_eliminate for FP->SP to be reset
>> to zero, and FP to be added to bad_spill_regs_global. For the AVR,
>> FP is Y, one of the 3 pointer regs. reload also notes that something
>> has changed, and that the loop needs to run again.
>> 7. reload still calls select_reload_regs, and find_regs fails to find a
>> pointer reg to reload SP, which is unnecessary as FP->SP elimination
>> had been disabled anyway in (6).
>>
>> IOW, reload fails to find pointer regs for an RTL expression that was
>> created when FP->SP elimination was true, even after it turns out that
>> the elimination can't be done after all. The patch tries to detect that
>> - if it knows the loop is going to run again, it silences the failure.
>>
>> Also note that at a different point in the loop, the reload loop starts
>> over if something_was_spilled (line 982-986). If set outside the reload
>> loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
>> "continue" after update_eliminables_and_spill (line 1019-1022) would
>> also work - haven't tested it though.
>
> That's what I was going to ask next. I think if that works for you, I 
> think that's an approach that would make more sense.
>
> However, it looks like after this call, and the other one in the same 
> loop, should probably be calling finish_spills. It looks like an 
> oversight in the existing code that this doesn't happen for the existing 
> continue. Please try adding it in both places.

Tried that out - that works too. Bootstrap and regression tests on
x86_64 went fine, and there were no regressions for the avr target as
well - the extra three passes that showed up with the initial patch show
up now too.

I couldn't put a continue before select_reload_regs though.
save_call_clobbered_regs sets up a few things that are cleaned up in
delete_caller_save_insns a little later in the loop. So I only exclude
select_reload_regs - finish_spills and cleanup will happen in both cases.

Does this patch look ok? If yes, could you commit it for me please? I
don't have commit access.

Regards
Senthil

gcc/ChangeLog

2016-04-28  Senthil Kumar Selvaraj  

PR target/60040
* reload1.c (reload): Call finish_spills before
restarting reload loop. Skip select_reload_regs
if update_eliminables_and_spill returns true.

gcc/testsuite/ChangeLog

2016-04-28  Sebastian Huber  
Matthijs Kooijman  
Senthil Kumar Selvaraj  

PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: New.


diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..d6fcece 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -981,7 +981,8 @@ reload (rtx_insn *first, int global)
   /* If we allocated another stack slot, redo elimination bookkeeping.  */
   if (something_was_spilled || starting_frame_size != get_frame_size ())
{
- update_eliminables_and_spill ();
+ if (update_eliminables_and_spill ())
+   finish_spills (global);
  continue;
}
 
@@ -1021,10 +1022,12 @@ reload (rtx_insn *first, int global)
  did_spill = 1;
  something_changed = 1;
}
-
-  select_reload_regs ();
-  if (failure)
-   goto failed;
+  else
+   {
+ select_reload_regs ();
+ if (failure)
+   goto failed;
+   }
 
   if (insns_need_reload != 0 || did_spill)
something_changed |= finish_spills (global);
diff --git gcc/testsuite/ChangeLog gcc/testsuite/ChangeLog
index 65fe0d0..bbaa3a2 100644
--- gcc/testsuite/ChangeLog
+++ gcc/testsuite/ChangeLog
@@ -1,7 +1,3 @@
-2016-04-26  Bernd Schmidt  
-
-   * gcc.target/i386/lzcnt-1.c: Allow a different lzcntw output register.
-
 2016-04-25  Richard Biener  
 
PR tree-optimization/70780
diff --git gcc/testsuite/gcc.target/avr/pr60040-1.c 
gcc/testsuite/gcc.target/avr/pr60040-1.c
new file mode 100644
index 000..4fe296b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr

Re: [Patch] Fix PR 60040

2016-04-25 Thread Bernd Schmidt

On 04/15/2016 02:52 PM, Senthil Kumar Selvaraj wrote:


For both testcases in the PR, reload fails to take into account that
FP-SP elimination can no longer be performed, and tries to find reload
regs for an rtx generated when FP-SP elimination was valid.

1. reload initializes elim table with FP->SP elimination enabled.
2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
something_was_spilled to true.
3. The main reload loop starts, and it resets something_was_spilled to false.
4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
(mem(SP + offset)).
5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
6. update_eliminables_and_spill calls targetm.frame_pointer_required,
which returns true. That causes can_eliminate for FP->SP to be reset
to zero, and FP to be added to bad_spill_regs_global. For the AVR,
FP is Y, one of the 3 pointer regs. reload also notes that something
has changed, and that the loop needs to run again.
7. reload still calls select_reload_regs, and find_regs fails to find a
pointer reg to reload SP, which is unnecessary as FP->SP elimination
had been disabled anyway in (6).

IOW, reload fails to find pointer regs for an RTL expression that was
created when FP->SP elimination was true, even after it turns out that
the elimination can't be done after all. The patch tries to detect that
- if it knows the loop is going to run again, it silences the failure.

Also note that at a different point in the loop, the reload loop starts
over if something_was_spilled (line 982-986). If set outside the reload
loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
"continue" after update_eliminables_and_spill (line 1019-1022) would
also work - haven't tested it though.


That's what I was going to ask next. I think if that works for you, I 
think that's an approach that would make more sense.


However, it looks like after this call, and the other one in the same 
loop, should probably be calling finish_spills. It looks like an 
oversight in the existing code that this doesn't happen for the existing 
continue. Please try adding it in both places.



Bernd


Re: [Patch] Fix PR 60040

2016-04-25 Thread Senthil Kumar Selvaraj

Ping!

Regards
Senthil

Senthil Kumar Selvaraj writes:

> Bernd Schmidt writes:
>
>> On 04/07/2016 01:52 PM, Senthil Kumar Selvaraj wrote:
>>>The below patch fixes PR 60040 by not halting with a hard error on
>>>a spill failure, if reload knows that it has to run again anyway.
>>
>> Some additional information as to how this situation creates a spill 
>> failure would be useful. It's hard to tell whether this patch just 
>> papers over a problem that can still trigger in other circumstances.
>
> For both testcases in the PR, reload fails to take into account that
> FP-SP elimination can no longer be performed, and tries to find reload
> regs for an rtx generated when FP-SP elimination was valid.
>
> 1. reload initializes elim table with FP->SP elimination enabled.
> 2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
>reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
>something_was_spilled to true.
> 3. The main reload loop starts, and it resets something_was_spilled to false.
> 4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
>(mem(SP + offset)).
> 5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
>SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
> 6. update_eliminables_and_spill calls targetm.frame_pointer_required,
>which returns true. That causes can_eliminate for FP->SP to be reset
>to zero, and FP to be added to bad_spill_regs_global. For the AVR,
>FP is Y, one of the 3 pointer regs. reload also notes that something
>has changed, and that the loop needs to run again.
> 7. reload still calls select_reload_regs, and find_regs fails to find a
>pointer reg to reload SP, which is unnecessary as FP->SP elimination
>had been disabled anyway in (6).
>
> IOW, reload fails to find pointer regs for an RTL expression that was
> created when FP->SP elimination was true, even after it turns out that
> the elimination can't be done after all. The patch tries to detect that
> - if it knows the loop is going to run again, it silences the failure.
>
> Also note that at a different point in the loop, the reload loop starts
> over if something_was_spilled (line 982-986). If set outside the reload
> loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
> "continue" after update_eliminables_and_spill (line 1019-1022) would
> also work - haven't tested it though.
>
> What do you think?
>
>
>>
>>> -   spill_failure (chain->insn, rld[r].rclass);
>>> -   failure = 1;
>>> -   return;
>>> +   if (!tentative)
>>> +   {
>>> +   spill_failure (chain->insn, rld[r].rclass);
>>> +   failure = 1;
>>> +   return;
>>> +   }
>>>   }
>>
>> The indentation looks all wrong.
>>
>
> Fixed now - mixed up tabs and spaces.
>
> gcc/ChangeLog
>
> 2016-04-07  Joern Rennecke  
> Senthil Kumar Selvaraj  
>
> PR target/60040
> * reload1.c (find_reload_regs): Add tentative parameter.
> and don't report spill failure if param set.
> (reload): Propagate something_changed to
> select_reload_regs.
> (select_reload_regs): Add tentative parameter.
>
> gcc/testsuite/ChangeLog
>
> 2016-04-07  Sebastian Huber  
> Matthijs Kooijman  
> Senthil Kumar Selvaraj  
>
> PR target/60040
> * gcc.target/avr/pr60040-1.c: New.
> * gcc.target/avr/pr60040-2.c: Likewise.
>
> diff --git gcc/reload1.c gcc/reload1.c
> index c2800f8..58993a3 100644
> --- gcc/reload1.c
> +++ gcc/reload1.c
> @@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
>  static void copy_reloads (struct insn_chain *);
>  static void calculate_needs_all_insns (int);
>  static int find_reg (struct insn_chain *, int);
> -static void find_reload_regs (struct insn_chain *);
> -static void select_reload_regs (void);
> +static void find_reload_regs (struct insn_chain *, bool);
> +static void select_reload_regs (bool);
>  static void delete_caller_save_insns (void);
>  
>  static void spill_failure (rtx_insn *, enum reg_class);
> @@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
> something_changed = 1;
>   }
>  
> -  select_reload_regs ();
> +  select_reload_regs (something_changed);
>if (failure)
>   goto failed;
>  
> @@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
> is given by CHAIN.
> Do it by ascending class number, since otherwise a reg
> might be spilled for a big class and might fail to count
> -   for a smaller class even though it belongs to that class.  */
> +   for a smaller class even though it belongs to that class.
> +   TENTATIVE means that we had some changes that might have invalidated
> +   the reloads and that we are going to loop again anyway, so don't give
> +   a hard error on failure to find a reload reg. */
>  
>  static v

Re: [Patch] Fix PR 60040

2016-04-15 Thread Senthil Kumar Selvaraj

Bernd Schmidt writes:

> On 04/07/2016 01:52 PM, Senthil Kumar Selvaraj wrote:
>>The below patch fixes PR 60040 by not halting with a hard error on
>>a spill failure, if reload knows that it has to run again anyway.
>
> Some additional information as to how this situation creates a spill 
> failure would be useful. It's hard to tell whether this patch just 
> papers over a problem that can still trigger in other circumstances.

For both testcases in the PR, reload fails to take into account that
FP-SP elimination can no longer be performed, and tries to find reload
regs for an rtx generated when FP-SP elimination was valid.

1. reload initializes elim table with FP->SP elimination enabled.
2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
   reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
   something_was_spilled to true.
3. The main reload loop starts, and it resets something_was_spilled to false.
4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
   (mem(SP + offset)).
5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
   SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
6. update_eliminables_and_spill calls targetm.frame_pointer_required,
   which returns true. That causes can_eliminate for FP->SP to be reset
   to zero, and FP to be added to bad_spill_regs_global. For the AVR,
   FP is Y, one of the 3 pointer regs. reload also notes that something
   has changed, and that the loop needs to run again.
7. reload still calls select_reload_regs, and find_regs fails to find a
   pointer reg to reload SP, which is unnecessary as FP->SP elimination
   had been disabled anyway in (6).

IOW, reload fails to find pointer regs for an RTL expression that was
created when FP->SP elimination was true, even after it turns out that
the elimination can't be done after all. The patch tries to detect that
- if it knows the loop is going to run again, it silences the failure.

Also note that at a different point in the loop, the reload loop starts
over if something_was_spilled (line 982-986). If set outside the reload
loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
"continue" after update_eliminables_and_spill (line 1019-1022) would
also work - haven't tested it though.

What do you think?


>
>> -spill_failure (chain->insn, rld[r].rclass);
>> -failure = 1;
>> -return;
>> +if (!tentative)
>> +{
>> +spill_failure (chain->insn, rld[r].rclass);
>> +failure = 1;
>> +return;
>> +}
>>}
>
> The indentation looks all wrong.
>

Fixed now - mixed up tabs and spaces.

gcc/ChangeLog

2016-04-07  Joern Rennecke  
Senthil Kumar Selvaraj  

PR target/60040
* reload1.c (find_reload_regs): Add tentative parameter.
and don't report spill failure if param set.
(reload): Propagate something_changed to
select_reload_regs.
(select_reload_regs): Add tentative parameter.

gcc/testsuite/ChangeLog

2016-04-07  Sebastian Huber  
Matthijs Kooijman  
Senthil Kumar Selvaraj  

PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: Likewise.

diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..58993a3 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
 static void copy_reloads (struct insn_chain *);
 static void calculate_needs_all_insns (int);
 static int find_reg (struct insn_chain *, int);
-static void find_reload_regs (struct insn_chain *);
-static void select_reload_regs (void);
+static void find_reload_regs (struct insn_chain *, bool);
+static void select_reload_regs (bool);
 static void delete_caller_save_insns (void);
 
 static void spill_failure (rtx_insn *, enum reg_class);
@@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
  something_changed = 1;
}
 
-  select_reload_regs ();
+  select_reload_regs (something_changed);
   if (failure)
goto failed;
 
@@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
is given by CHAIN.
Do it by ascending class number, since otherwise a reg
might be spilled for a big class and might fail to count
-   for a smaller class even though it belongs to that class.  */
+   for a smaller class even though it belongs to that class.
+   TENTATIVE means that we had some changes that might have invalidated
+   the reloads and that we are going to loop again anyway, so don't give
+   a hard error on failure to find a reload reg. */
 
 static void
-find_reload_regs (struct insn_chain *chain)
+find_reload_regs (struct insn_chain *chain, bool tentative)
 {
   int i;
 
@@ -2012,9 +2015,12 @@ find_reload_regs (struct insn_chain *chain)
  {
if (dump_file)

Re: [Patch] Fix PR 60040

2016-04-07 Thread Bernd Schmidt

On 04/07/2016 01:52 PM, Senthil Kumar Selvaraj wrote:

   The below patch fixes PR 60040 by not halting with a hard error on
   a spill failure, if reload knows that it has to run again anyway.


Some additional information as to how this situation creates a spill 
failure would be useful. It's hard to tell whether this patch just 
papers over a problem that can still trigger in other circumstances.



-   spill_failure (chain->insn, rld[r].rclass);
-   failure = 1;
-   return;
+   if (!tentative)
+   {
+   spill_failure (chain->insn, rld[r].rclass);
+   failure = 1;
+   return;
+   }
  }


The indentation looks all wrong.


Bernd


[Patch] Fix PR 60040

2016-04-07 Thread Senthil Kumar Selvaraj
Hi,

  The below patch fixes PR 60040 by not halting with a hard error on
  a spill failure, if reload knows that it has to run again anyway.
  It also fixes two reload related ICEs on trunk
  (gcc.c-torture/compile/920625-1.c and gcc.dg/tree-ssa/pr70232.c)
  for the AVR target. I've slighly reworked the patch - the original
  patch by Joern Rennecke did not skip the setting of failure to 1; it
  never gets reset afterwards.

  Bootstrapped and regtested on/for x86_64-linux with no regressions.
  Thee avr target shows additional PASSes for the above two testcases
  and no other regressions.

  If ok, could someone commit please? I don't have commit access.

Regards
Senthil

gcc/ChangeLog

2016-04-07  Joern Rennecke  
Senthil Kumar Selvaraj  

PR target/60040
* reload1.c (find_reload_regs): Add tentative parameter.
and don't report spill failure if param set.
(reload): Propagate something_changed to
select_reload_regs.
(select_reload_regs): Add tentative parameter.

gcc/testsuite/ChangeLog

2016-04-07  Sebastian Huber  
Matthijs Kooijman  
Senthil Kumar Selvaraj  

PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: Likewise.


diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..58f58a9 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
 static void copy_reloads (struct insn_chain *);
 static void calculate_needs_all_insns (int);
 static int find_reg (struct insn_chain *, int);
-static void find_reload_regs (struct insn_chain *);
-static void select_reload_regs (void);
+static void find_reload_regs (struct insn_chain *, bool);
+static void select_reload_regs (bool);
 static void delete_caller_save_insns (void);
 
 static void spill_failure (rtx_insn *, enum reg_class);
@@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
  something_changed = 1;
}
 
-  select_reload_regs ();
+  select_reload_regs (something_changed);
   if (failure)
goto failed;
 
@@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
is given by CHAIN.
Do it by ascending class number, since otherwise a reg
might be spilled for a big class and might fail to count
-   for a smaller class even though it belongs to that class.  */
+   for a smaller class even though it belongs to that class.
+   TENTATIVE means that we had some changes that might have invalidated
+   the reloads and that we are going to loop again anyway, so don't give
+   a hard error on failure to find a reload reg. */
 
 static void
-find_reload_regs (struct insn_chain *chain)
+find_reload_regs (struct insn_chain *chain, bool tentative)
 {
   int i;
 
@@ -2012,9 +2015,12 @@ find_reload_regs (struct insn_chain *chain)
  {
if (dump_file)
  fprintf (dump_file, "reload failure for reload %d\n", r);
-   spill_failure (chain->insn, rld[r].rclass);
-   failure = 1;
-   return;
+   if (!tentative)
+   {
+   spill_failure (chain->insn, rld[r].rclass);
+   failure = 1;
+   return;
+   }
  }
 }
 
@@ -2025,14 +2031,14 @@ find_reload_regs (struct insn_chain *chain)
 }
 
 static void
-select_reload_regs (void)
+select_reload_regs (bool tentative)
 {
   struct insn_chain *chain;
 
   /* Try to satisfy the needs for each insn.  */
   for (chain = insns_need_reload; chain != 0;
chain = chain->next_need_reload)
-find_reload_regs (chain);
+find_reload_regs (chain, tentative);
 }
 
 /* Delete all insns that were inserted by emit_caller_save_insns during
diff --git gcc/testsuite/gcc.target/avr/pr60040-1.c 
gcc/testsuite/gcc.target/avr/pr60040-1.c
new file mode 100644
index 000..4fe296b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+
+float dhistory[10];
+float test;
+
+float getSlope(float history[]) {
+  float sumx = 0;
+  float sumy = 0;
+  float sumxy = 0;
+  float sumxsq = 0;
+  float rate = 0;
+  int n = 10;
+
+  int i;
+  for (i=1; i< 11; i++) {
+sumx = sumx + i;
+sumy = sumy + history[i-1];
+sumy = sumy + history[i-1];
+sumxsq = sumxsq + (i*i);
+  }
+
+  rate = sumy+sumx+sumxsq;
+  return rate;
+}
+
+void loop() {
+  test = getSlope(dhistory);
+}
diff --git gcc/testsuite/gcc.target/avr/pr60040-2.c 
gcc/testsuite/gcc.target/avr/pr60040-2.c
new file mode 100644
index 000..c40d49f
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-2.c
@@ -0,0 +1,112 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned char __uint8_t;
+typedef short unsigned int __uint16_t;
+typedef long unsigned int __uint32_t;
+typedef __uint8_t uint8_t ;
+typedef __uint16_t uint16_t ;
+typedef __uint32_t uint32_t ;
+typedef __builtin_va_lis