Fix PR 60040

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

Commit Message

Senthil Kumar Selvaraj April 7, 2016, 11:52 a.m.
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  <gnu@amylaar.uk>
            Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

        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  <sebastian.huber@embedded-brains.de>
            Matthijs Kooijman  <matthijs@stdin.nl>
            Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

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

Comments

Bernd Schmidt April 7, 2016, 7:47 p.m. | #1
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

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 0000000..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 0000000..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_handlers* ops;
+  return ops->blank (sd, device, segment, offset, size);
+}
+static int
+rtems_fdisk_seg_write_page_desc (const rtems_flashdisk* fd,
+                                 rtems_fdisk_segment_ctl* sc,
+                                 uint32_t page,
+                                 const rtems_fdisk_page_desc* page_desc)
+{
+  uint32_t offset = page * sizeof (rtems_fdisk_page_desc);
+  if ((fd->flags & (1 << 3)))
+  {
+    int ret = rtems_fdisk_seg_blank_check (fd, sc,
+                                           offset,
+                                           sizeof (rtems_fdisk_page_desc));
+  }
+  return rtems_fdisk_seg_write (fd, sc, offset,
+                                page_desc, sizeof (rtems_fdisk_page_desc));
+}
+void
+rtems_fdisk_recycle_segment (rtems_flashdisk* fd,
+                                    rtems_fdisk_segment_ctl* ssc,
+                                    rtems_fdisk_segment_ctl* dsc,
+                                    uint32_t *pages)
+{
+  int ret;
+  uint32_t spage;
+  uint32_t used = 0;
+  uint32_t active = 0;
+  {
+    rtems_fdisk_page_desc* spd = &ssc->page_descriptors[spage];
+    {
+      rtems_fdisk_page_desc* dpd;
+      uint32_t dpage;
+      dpd = &dsc->page_descriptors[dpage];
+      *dpd = *spd;
+      ret = rtems_fdisk_seg_write_page_desc (fd,
+                                             dsc,
+                                             dpage, dpd);
+    }
+  }
+  rtems_fdisk_printf (fd, "ssc end: %d-%d: p=%ld, a=%ld, u=%ld",
+                      pages, active, used);
+  {
+    rtems_fdisk_error ("compacting: ssc pages not 0: %d",
+                       ssc->pages_active);
+  }
+}