Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Fri, Nov 15, 2013 at 09:22:01AM +0000, Matthew Leach wrote:
> Martin Jambor <mjambor@suse.cz> writes:
> 
> > Hi,
> >
> > On Thu, Nov 14, 2013 at 12:18:24PM +0000, Matthew Leach wrote:
> >> Martin Jambor <mjambor@suse.cz> writes:
> >> 
> >> > Hi,
> >> 
> >> Hi Martin,
> >> 
> >> [...]
> >> 
> >> >
> >> > 2013-11-04  Martin Jambor  <mjambor@suse.cz>
> >> >
> >> >         PR rtl-optimization/10474
> >> >         * ira.c (interesting_dest_for_shprep): New function.
> >> >         (split_live_ranges_for_shrink_wrap): Likewise.
> >> >         (find_moveable_pseudos): Move calculation of dominance info,
> >> >         df_analysios and the final anlyses to...
> >> >         (ira): ...here, call split_live_ranges_for_shrink_wrap.
> >> >
> >> > testsuite/
> >> >         * gcc.dg/pr10474.c: New testcase.
> >> >         * gcc.dg/ira-shrinkwrap-prep-1.c: Likewise.
> >> >         * gcc.dg/ira-shrinkwrap-prep-2.c: Likewise.
> >> 
> >> This patch seems to breaks stage-3 bootstrap for
> >> armv7l-unknown-linux-gnueabihf. genmddeps and genhooks seem to be in
> >> an infinite loop (I've left them running for approx 1h 30m now).
> >> 
> >> My configure flags are:
> >> 
> >> ../gcc/configure --with-cpu=cortex-a15 --with-tune=cortex-a15 --disable-nla --enable-shared --with-float=hard --with-fpu=neon-vfpv4
> >> 
> >
> > I hope all the issues are just different manisfestations of PR 59099.
> > I hope to have fixed that by the patch below but I've just only
> > started bootstrapping it (and I'd like to do that on multiple
> > platforms before proposing it).  But of course everybody can test if
> > it helps with their issues (and does not create new ones), I'd be
> > grateful if you do.
> 
> I've put this patch through a bootstrap on an ARM chromebook and it
> seems to fix the problem I was seeing..
> 

Perfect, thanks a lot.  The patch has also passed bootstrap and
testing on x86_64-linux (all languages and Ada) and passed bootstrap
on ppc64 (all languages) and ia64 (C, C++ and Fortran), I do not yet
have results from testsuite runs from the latter two platforms.  Just
fro the record, I've also just started an i686 bootstrap.

Vlad, the patch below is the exactly the same one + a testcase.  It
basically does what you suggested in your first email, moves the
transformation to before all the analyses and undoes some code
movement from find_moveable_pseudos() to ira().  Is it OK for trunk?

Thanks,

Martin


2013-11-15  Martin Jambor  <mjambor@suse.cz>

	* ira.c (find_moveable_pseudos): Put back various analyses from ira()
	here.
	(ira): Move init_reg_equiv and call to
	split_live_ranges_for_shrink_wrap up, remove analyses around call
	to find_moveable_pseudos.

testsuite/
	* gcc.target/i386/pr59099.c: New test.

diff --git a/gcc/ira.c b/gcc/ira.c
index 2ef69cb..a171761 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -4515,6 +4515,9 @@ find_moveable_pseudos (void)
   pseudo_replaced_reg.release ();
   pseudo_replaced_reg.safe_grow_cleared (max_regs);
 
+  df_analyze ();
+  calculate_dominance_info (CDI_DOMINATORS);
+
   i = 0;
   bitmap_initialize (&live, 0);
   bitmap_initialize (&used, 0);
@@ -4827,6 +4830,14 @@ find_moveable_pseudos (void)
   free (bb_moveable_reg_sets);
 
   last_moveable_pseudo = max_reg_num ();
+
+  fix_reg_equiv_init ();
+  expand_reg_info ();
+  regstat_free_n_sets_and_refs ();
+  regstat_free_ri ();
+  regstat_init_n_sets_and_refs ();
+  regstat_compute_ri ();
+  free_dominance_info (CDI_DOMINATORS);
 }
 
 
@@ -5187,7 +5198,19 @@ ira (FILE *f)
 #endif
   df_analyze ();
 
+  init_reg_equiv ();
+  if (ira_conflicts_p)
+    {
+      calculate_dominance_info (CDI_DOMINATORS);
+
+      if (split_live_ranges_for_shrink_wrap ())
+	df_analyze ();
+
+      free_dominance_info (CDI_DOMINATORS);
+    }
+
   df_clear_flags (DF_NO_INSN_RESCAN);
+
   regstat_init_n_sets_and_refs ();
   regstat_compute_ri ();
 
@@ -5205,7 +5228,6 @@ ira (FILE *f)
   if (resize_reg_info () && flag_ira_loop_pressure)
     ira_set_pseudo_classes (true, ira_dump_file);
 
-  init_reg_equiv ();
   rebuild_p = update_equiv_regs ();
   setup_reg_equiv ();
   setup_reg_equiv_init ();
@@ -5228,22 +5250,7 @@ ira (FILE *f)
      allocation because of -O0 usage or because the function is too
      big.  */
   if (ira_conflicts_p)
-    {
-      df_analyze ();
-      calculate_dominance_info (CDI_DOMINATORS);
-
-      find_moveable_pseudos ();
-      if (split_live_ranges_for_shrink_wrap ())
-	df_analyze ();
-
-      fix_reg_equiv_init ();
-      expand_reg_info ();
-      regstat_free_n_sets_and_refs ();
-      regstat_free_ri ();
-      regstat_init_n_sets_and_refs ();
-      regstat_compute_ri ();
-      free_dominance_info (CDI_DOMINATORS);
-    }
+    find_moveable_pseudos ();
 
   max_regno_before_ira = max_reg_num ();
   ira_setup_eliminable_regset (true);
diff --git a/gcc/testsuite/gcc.target/i386/pr59099.c b/gcc/testsuite/gcc.target/i386/pr59099.c
new file mode 100644
index 0000000..7dc12ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr59099.c
@@ -0,0 +1,76 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fPIC -m32" } */
+
+void (*pfn)(void);
+
+struct s
+{
+  void** q;
+  int h;
+  int t;
+  int s;
+};
+
+
+void* f (struct s *, struct s *) __attribute__ ((noinline, regparm(1)));
+
+void*
+__attribute__ ((regparm(1)))
+f (struct s *p, struct s *p2)
+{
+  void *gp, *gp1;
+  int t, h, s, t2, h2, c, i;
+
+  if (p2->h == p2->t)
+    return 0;
+
+  (*pfn) ();
+
+  h = p->h;
+  t = p->t;
+  s = p->s;
+
+  h2 = p2->h;
+  t2 = p2->t;
+
+  gp = p2->q[h2++];
+
+  c = (t2 - h2) / 2;
+  for (i = 0; i != c; i++)
+    {
+      if (t == h || (h == 0 && t == s - 1))
+	break;
+      gp1 = p2->q[h2++];
+      p->q[t++] = gp1;
+      if (t == s)
+	t = 0;
+    }
+
+  p2->h = h2;
+  return gp;
+}
+
+static void gn () { }
+
+int
+main()
+{
+  struct s s1, s2;
+  void *q[10];
+
+  pfn = gn;
+
+  s1.q = q;
+  s1.h = 0;
+  s1.t = 2;
+  s1.s = 4;
+
+  s2.q = q;
+  s2.h = 0;
+  s2.t = 4;
+  s2.s = 2;
+
+  f (&s1, &s2);
+
+  return 0;
+}