public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
@ 2013-11-06 16:43 Martin Jambor
  2013-11-08 21:14 ` Vladimir Makarov
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Martin Jambor @ 2013-11-06 16:43 UTC (permalink / raw)
  To: GCC Patches; +Cc: Vladimir Makarov, Steven Bosscher

Hi,

last Thursday I had to revert a previous version of this patch because
it has caused a lot of trouble on various platforms I did not test it
on.  The patch is still very similar to its previous iteration
(http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02183.html) the only
major difference is that I moved more after-the-fact re-analyzing from
find_moveable_pseudos to a point when my transformation also finished.

The problem with it was that REG_N_REFS of the pseudos introduced by
my code was not set and thus reload ignored and let it slip through
instead of generating spills.  This is fixed by moving the
re-computation of regstat_n_sets_and_refs in
regstat_init_n_sets_and_refs to after my transformation.  In order to
avoid similar surprises I have moved all other re-computations from
find_moveable_pseudos to the caller in ira, namely:

      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 ();

I have also bootstrapped and tested the patch not only on x86_64-linux
but also on i686-linux and ppc64-linux (without Ada though).  I have
made sure that the reported problem does not occur on cris-elf and
sh-none-elf cross compilers.  (I could not reproduce it on arm, and do
not have access to sparc but it was also reported there.)

Another minor change which I erroneously omitted the last time is that
the testcases are run on x86_64 only because that is the only platform
where I know the transformation currently takes place.  The reason why
I did not move them to a target-specific directory is that I believe
the transformation can be beneficial on other platforms as well.  For
example, PR 10474 was actually filed against PPC but this patch does
not work there because the initial move of the parameter is done in a
parallel insn:

        (parallel [
            (set (reg:CC 124)
                (compare:CC (reg:DI 3 3 [ i ])
                    (const_int 0 [0])))
            (set (reg/v/f:DI 123 [ i ])
                (reg:DI 3 3 [ i ]))
        ])

which fails my single_set test.  However, the mechanism can be
extended to handle these situations as well and afterwards we could
run the test also on ppc64.

So, Vlad, Steven, do you think that this time I have re-computed all
that is necessary?  Do you think the patch is OK?

Thanks a lot and sorry for the breakage,

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.

diff --git a/gcc/ira.c b/gcc/ira.c
index 9e94704..70fe13b 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -4514,9 +4514,6 @@ 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);
@@ -4829,14 +4826,196 @@ 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);
+
+/* If insn is interesting for parameter range-splitting shring-wrapping
+   preparation, i.e. it is a single set from a hard register to a pseudo, which
+   is live at CALL_DOM, return the destination.  Otherwise return NULL.  */
+
+static rtx
+interesting_dest_for_shprep (rtx insn, basic_block call_dom)
+{
+  rtx set = single_set (insn);
+  if (!set)
+    return NULL;
+  rtx src = SET_SRC (set);
+  rtx dest = SET_DEST (set);
+  if (!REG_P (src) || !HARD_REGISTER_P (src)
+      || !REG_P (dest) || HARD_REGISTER_P (dest)
+      || (call_dom && !bitmap_bit_p (df_get_live_in (call_dom), REGNO (dest))))
+    return NULL;
+  return dest;
+}
+
+/* Split live ranges of pseudos that are loaded from hard registers in the
+   first BB in a BB that dominates all non-sibling call if such a BB can be
+   found and is not in a loop.  Return true if the function has made any
+   changes.  */
+
+static bool
+split_live_ranges_for_shrink_wrap (void)
+{
+  basic_block bb, call_dom = NULL;
+  basic_block first = single_succ (ENTRY_BLOCK_PTR);
+  rtx insn, last_interesting_insn = NULL;
+  bitmap_head need_new, reachable;
+  vec<basic_block> queue;
+
+  if (!flag_shrink_wrap)
+    return false;
+
+  bitmap_initialize (&need_new, 0);
+  bitmap_initialize (&reachable, 0);
+  queue.create (n_basic_blocks);
+
+  FOR_EACH_BB (bb)
+    FOR_BB_INSNS (bb, insn)
+      if (CALL_P (insn) && !SIBLING_CALL_P (insn))
+	{
+	  if (bb == first)
+	    {
+	      bitmap_clear (&need_new);
+	      bitmap_clear (&reachable);
+	      queue.release ();
+	      return false;
+	    }
+
+	  bitmap_set_bit (&need_new, bb->index);
+	  bitmap_set_bit (&reachable, bb->index);
+	  queue.quick_push (bb);
+	  break;
+	}
+
+  if (queue.is_empty ())
+    {
+      bitmap_clear (&need_new);
+      bitmap_clear (&reachable);
+      queue.release ();
+      return false;
+    }
+
+  while (!queue.is_empty ())
+    {
+      edge e;
+      edge_iterator ei;
+
+      bb = queue.pop ();
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	if (e->dest != EXIT_BLOCK_PTR
+	    && bitmap_set_bit (&reachable, e->dest->index))
+	  queue.quick_push (e->dest);
+    }
+  queue.release ();
+
+  FOR_BB_INSNS (first, insn)
+    {
+      rtx dest = interesting_dest_for_shprep (insn, NULL);
+      if (!dest)
+	continue;
+
+      if (DF_REG_DEF_COUNT (REGNO (dest)) > 1)
+	{
+	  bitmap_clear (&need_new);
+	  bitmap_clear (&reachable);
+	  return false;
+	}
+
+      for (df_ref use = DF_REG_USE_CHAIN (REGNO(dest));
+	   use;
+	   use = DF_REF_NEXT_REG (use))
+	{
+	  if (NONDEBUG_INSN_P (DF_REF_INSN (use))
+	      && GET_CODE (DF_REF_REG (use)) == SUBREG)
+	    {
+	      /* This is necessary to avoid hitting an assert at
+		 postreload.c:2294 in libstc++ testcases on x86_64-linux.  I'm
+		 not really sure what the probblem actually is there.  */
+	      bitmap_clear (&need_new);
+	      bitmap_clear (&reachable);
+	      return false;
+	    }
+
+	  int ubbi = DF_REF_BB (use)->index;
+	  if (bitmap_bit_p (&reachable, ubbi))
+	    bitmap_set_bit (&need_new, ubbi);
+	}
+      last_interesting_insn = insn;
+    }
+
+  bitmap_clear (&reachable);
+  if (!last_interesting_insn)
+    {
+      bitmap_clear (&need_new);
+      return false;
+    }
+
+  call_dom = nearest_common_dominator_for_set (CDI_DOMINATORS, &need_new);
+  bitmap_clear (&need_new);
+  if (call_dom == first)
+    return false;
+
+  loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
+  while (bb_loop_depth (call_dom) > 0)
+    call_dom = get_immediate_dominator (CDI_DOMINATORS, call_dom);
+  loop_optimizer_finalize ();
+
+  if (call_dom == first)
+    return false;
+
+  calculate_dominance_info (CDI_POST_DOMINATORS);
+  if (dominated_by_p (CDI_POST_DOMINATORS, first, call_dom))
+    {
+      free_dominance_info (CDI_POST_DOMINATORS);
+      return false;
+    }
+  free_dominance_info (CDI_POST_DOMINATORS);
+
+  if (dump_file)
+    fprintf (dump_file, "Will split live ranges of parameters at BB %i\n",
+	     call_dom->index);
+
+  bool ret = false;
+  FOR_BB_INSNS (first, insn)
+    {
+      rtx dest = interesting_dest_for_shprep (insn, call_dom);
+      if (!dest)
+	continue;
+
+      rtx newreg = NULL_RTX;
+      df_ref use, next;
+      for (use = DF_REG_USE_CHAIN (REGNO(dest)); use; use = next)
+	{
+	  rtx uin = DF_REF_INSN (use);
+	  next = DF_REF_NEXT_REG (use);
+
+	  basic_block ubb = BLOCK_FOR_INSN (uin);
+	  if (ubb == call_dom
+	      || dominated_by_p (CDI_DOMINATORS, ubb, call_dom))
+	    {
+	      if (!newreg)
+		newreg = ira_create_new_reg (dest);
+	      validate_change (uin, DF_REF_LOC (use), newreg, true);
+	    }
+	}
+
+      if (newreg)
+	{
+	  rtx new_move = gen_move_insn (newreg, dest);
+	  emit_insn_after (new_move, bb_note (call_dom));
+	  if (dump_file)
+	    {
+	      fprintf (dump_file, "Split live-range of register ");
+	      print_rtl_single (dump_file, dest);
+	    }
+	  ret = true;
+	}
+
+      if (insn == last_interesting_insn)
+	break;
+    }
+  apply_change_group ();
+  return ret;
 }
 
 /* Perform the second half of the transformation started in
@@ -5048,7 +5227,22 @@ ira (FILE *f)
      allocation because of -O0 usage or because the function is too
      big.  */
   if (ira_conflicts_p)
-    find_moveable_pseudos ();
+    {
+      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);
+    }
 
   max_regno_before_ira = max_reg_num ();
   ira_setup_eliminable_regset (true);
diff --git a/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-1.c b/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-1.c
new file mode 100644
index 0000000..16095e3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-1.c
@@ -0,0 +1,31 @@
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O3 -fdump-rtl-ira -fdump-rtl-pro_and_epilogue"  } */
+
+int __attribute__((noinline, noclone))
+foo (int a)
+{
+  return a + 5;
+}
+
+static int g;
+
+int __attribute__((noinline, noclone))
+bar (int a)
+{
+  int r;
+
+  if (a)
+    {
+      r = foo (a);
+      g = r + a;
+    }
+  else
+    r = a+1;
+  return r;
+}
+
+/* { dg-final { scan-rtl-dump "Will split live ranges of parameters" "ira"  } } */
+/* { dg-final { scan-rtl-dump "Split live-range of register" "ira"  } } */
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" "pro_and_epilogue"  } } */
+/* { dg-final { cleanup-rtl-dump "ira" } } */
+/* { dg-final { cleanup-rtl-dump "pro_and_epilogue" } } */
diff --git a/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-2.c b/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-2.c
new file mode 100644
index 0000000..2b00c5b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ira-shrinkwrap-prep-2.c
@@ -0,0 +1,36 @@
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O3 -fdump-rtl-ira -fdump-rtl-pro_and_epilogue"  } */
+
+int __attribute__((noinline, noclone))
+foo (int a)
+{
+  return a + 5;
+}
+
+static int g;
+
+int __attribute__((noinline, noclone))
+bar (int a)
+{
+  int r;
+
+  if (a)
+    {
+      r = a;
+      while (r < 500)
+	if (r % 2)
+	  r = foo (r);
+	else
+	  r = foo (r+1);
+      g = r + a;
+    }
+  else
+    r = g+1;
+  return r;
+}
+
+/* { dg-final { scan-rtl-dump "Will split live ranges of parameters" "ira"  } } */
+/* { dg-final { scan-rtl-dump "Split live-range of register" "ira"  } } */
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" "pro_and_epilogue"  } } */
+/* { dg-final { cleanup-rtl-dump "ira" } } */
+/* { dg-final { cleanup-rtl-dump "pro_and_epilogue" } } */
diff --git a/gcc/testsuite/gcc.dg/pr10474.c b/gcc/testsuite/gcc.dg/pr10474.c
new file mode 100644
index 0000000..4fcd75d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr10474.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O3 -fdump-rtl-pro_and_epilogue"  } */
+
+void f(int *i)
+{
+	if (!i)
+		return;
+	else
+	{
+		__builtin_printf("Hi");
+		*i=0;
+	}
+}
+
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" "pro_and_epilogue"  } } */
+/* { dg-final { cleanup-rtl-dump "pro_and_epilogue" } } */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-06 16:43 [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping Martin Jambor
@ 2013-11-08 21:14 ` Vladimir Makarov
  2013-11-13 16:57 ` H.J. Lu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Vladimir Makarov @ 2013-11-08 21:14 UTC (permalink / raw)
  To: GCC Patches; +Cc: Steven Bosscher

On 11/6/2013, 11:26 AM, Martin Jambor wrote:
> So, Vlad, Steven, do you think that this time I have re-computed all
> that is necessary?  Do you think the patch is OK?
It is ok for me.  Thanks.
> Thanks a lot and sorry for the breakage,
>
Only who does nothing does not make mistakes.
>
>
> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-06 16:43 [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping Martin Jambor
  2013-11-08 21:14 ` Vladimir Makarov
@ 2013-11-13 16:57 ` H.J. Lu
  2013-11-14 10:09 ` Jakub Jelinek
  2013-11-14 14:22 ` Matthew Leach
  3 siblings, 0 replies; 11+ messages in thread
From: H.J. Lu @ 2013-11-13 16:57 UTC (permalink / raw)
  To: GCC Patches, Vladimir Makarov, Steven Bosscher

On Wed, Nov 6, 2013 at 8:26 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> last Thursday I had to revert a previous version of this patch because
> it has caused a lot of trouble on various platforms I did not test it
> on.  The patch is still very similar to its previous iteration
> (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02183.html) the only
> major difference is that I moved more after-the-fact re-analyzing from
> find_moveable_pseudos to a point when my transformation also finished.
>
> The problem with it was that REG_N_REFS of the pseudos introduced by
> my code was not set and thus reload ignored and let it slip through
> instead of generating spills.  This is fixed by moving the
> re-computation of regstat_n_sets_and_refs in
> regstat_init_n_sets_and_refs to after my transformation.  In order to
> avoid similar surprises I have moved all other re-computations from
> find_moveable_pseudos to the caller in ira, namely:
>
>       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 ();
>
> I have also bootstrapped and tested the patch not only on x86_64-linux
> but also on i686-linux and ppc64-linux (without Ada though).  I have
> made sure that the reported problem does not occur on cris-elf and
> sh-none-elf cross compilers.  (I could not reproduce it on arm, and do
> not have access to sparc but it was also reported there.)
>
> Another minor change which I erroneously omitted the last time is that
> the testcases are run on x86_64 only because that is the only platform
> where I know the transformation currently takes place.  The reason why
> I did not move them to a target-specific directory is that I believe
> the transformation can be beneficial on other platforms as well.  For
> example, PR 10474 was actually filed against PPC but this patch does
> not work there because the initial move of the parameter is done in a
> parallel insn:
>
>         (parallel [
>             (set (reg:CC 124)
>                 (compare:CC (reg:DI 3 3 [ i ])
>                     (const_int 0 [0])))
>             (set (reg/v/f:DI 123 [ i ])
>                 (reg:DI 3 3 [ i ]))
>         ])
>
> which fails my single_set test.  However, the mechanism can be
> extended to handle these situations as well and afterwards we could
> run the test also on ppc64.
>
> So, Vlad, Steven, do you think that this time I have re-computed all
> that is necessary?  Do you think the patch is OK?
>
> Thanks a lot and sorry for the breakage,
>
> 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 caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59099

-- 
H.J.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-06 16:43 [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping Martin Jambor
  2013-11-08 21:14 ` Vladimir Makarov
  2013-11-13 16:57 ` H.J. Lu
@ 2013-11-14 10:09 ` Jakub Jelinek
  2013-11-14 11:05   ` Martin Jambor
  2013-11-14 14:22 ` Matthew Leach
  3 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2013-11-14 10:09 UTC (permalink / raw)
  To: Martin Jambor; +Cc: gcc-patches

On Wed, Nov 06, 2013 at 05:26:08PM +0100, Martin Jambor wrote:
> So, Vlad, Steven, do you think that this time I have re-computed all
> that is necessary?  Do you think the patch is OK?
> 
> Thanks a lot and sorry for the breakage,

I'm afraid there are still issues left.
Last night I was bootstrapping (first r204752, then when it failed
also r204751) with:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01268.html
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01437.html
http://gcc.gnu.org/ml/gcc-patches/2013-11/txturet7Hr1Ws.txt
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01553.html
patches applied on i686-linux (see
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02625.html
on how I configure/build such a gcc on x86_64-linux host),
and while it bootstrapped just fine, there were hundreds of regressions
in the vectorization tests, which worked fine if compiled with
stage1 cc1/cc1plus, but with stage2/3 cc1/cc1plus behaved strangely.
When I've reverted the ira.c part of r204698 and bootstrapped/regtested
with all those patches again, all the problems went away.
Valgrind hasn't revealed any undefined uses caused by those patches.

As it doesn't affect bootstrap/regtest of unpatched gcc, it is not an
immediate blocker (well, would be if/once the patches make it in),
but silent wrong code (sure, just suspected) is always very important.
Can you please have a look?

> 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.

	Jakub

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-14 10:09 ` Jakub Jelinek
@ 2013-11-14 11:05   ` Martin Jambor
  0 siblings, 0 replies; 11+ messages in thread
From: Martin Jambor @ 2013-11-14 11:05 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

Hi,

On Thu, Nov 14, 2013 at 09:24:54AM +0100, Jakub Jelinek wrote:
> On Wed, Nov 06, 2013 at 05:26:08PM +0100, Martin Jambor wrote:
> > So, Vlad, Steven, do you think that this time I have re-computed all
> > that is necessary?  Do you think the patch is OK?
> > 
> > Thanks a lot and sorry for the breakage,
> 
> I'm afraid there are still issues left.
> Last night I was bootstrapping (first r204752, then when it failed
> also r204751) with:
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01268.html
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01437.html
> http://gcc.gnu.org/ml/gcc-patches/2013-11/txturet7Hr1Ws.txt
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01553.html
> patches applied on i686-linux (see
> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02625.html
> on how I configure/build such a gcc on x86_64-linux host),
> and while it bootstrapped just fine, there were hundreds of regressions
> in the vectorization tests, which worked fine if compiled with
> stage1 cc1/cc1plus, but with stage2/3 cc1/cc1plus behaved strangely.
> When I've reverted the ira.c part of r204698 and bootstrapped/regtested
> with all those patches again, all the problems went away.
> Valgrind hasn't revealed any undefined uses caused by those patches.
> 
> As it doesn't affect bootstrap/regtest of unpatched gcc, it is not an
> immediate blocker (well, would be if/once the patches make it in),
> but silent wrong code (sure, just suspected) is always very important.
> Can you please have a look?

I'm already in the process of debugging PR 59099 and at the moment I
believe I am on the right track.  Let's hope it is the same problem
but if not, I will certainly have a look.

Thanks for the report,

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.
> 
> 	Jakub

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-06 16:43 [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping Martin Jambor
                   ` (2 preceding siblings ...)
  2013-11-14 10:09 ` Jakub Jelinek
@ 2013-11-14 14:22 ` Matthew Leach
  2013-11-14 20:33   ` Martin Jambor
  3 siblings, 1 reply; 11+ messages in thread
From: Matthew Leach @ 2013-11-14 14:22 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Patches, Vladimir Makarov, Steven Bosscher

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

Thanks,
Matt Leach

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-14 14:22 ` Matthew Leach
@ 2013-11-14 20:33   ` Martin Jambor
  2013-11-15 10:10     ` Matthew Leach
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Jambor @ 2013-11-14 20:33 UTC (permalink / raw)
  To: Matthew Leach
  Cc: GCC Patches, Vladimir Makarov, Steven Bosscher, Jakub Jelinek

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.

The problem in that PR was with reload (or perhaps I should say LRA)
getting confused by information in array ira_reg_equiv which was not
updated by my patch.  Rather than updating it, I have decided to move
the transformation before its computation.

The reason my previous attempts t this failed was that I use
ira_create_new_reg which resizes that array too and so I needed to
move its allocation upwards as well (what a stupid thing to debug for
a couple of hours, sigh).

Thanks and sorry again, but it is a tough area for me,

Martin


2013-11-14  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.

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);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-14 20:33   ` Martin Jambor
@ 2013-11-15 10:10     ` Matthew Leach
  2013-11-15 16:53       ` Martin Jambor
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew Leach @ 2013-11-15 10:10 UTC (permalink / raw)
  To: GCC Patches; +Cc: Vladimir Makarov, Steven Bosscher, Jakub Jelinek

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..

Thanks,
Matt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-15 10:10     ` Matthew Leach
@ 2013-11-15 16:53       ` Martin Jambor
  2013-11-19 12:20         ` James Greenhalgh
  2013-11-19 16:26         ` Vladimir Makarov
  0 siblings, 2 replies; 11+ messages in thread
From: Martin Jambor @ 2013-11-15 16:53 UTC (permalink / raw)
  To: Matthew Leach
  Cc: GCC Patches, Vladimir Makarov, Steven Bosscher, Jakub Jelinek

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;
+}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-15 16:53       ` Martin Jambor
@ 2013-11-19 12:20         ` James Greenhalgh
  2013-11-19 16:26         ` Vladimir Makarov
  1 sibling, 0 replies; 11+ messages in thread
From: James Greenhalgh @ 2013-11-19 12:20 UTC (permalink / raw)
  To: Matthew Leach, GCC Patches, Vladimir Makarov, Steven Bosscher,
	Jakub Jelinek

*Ping*

This is one of many bugs blocking bootstrap on ARM. It would be
helpful if the fix could be reviewed and, if correct, for it to go
in.

http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01820.html

Thanks,
James

On Fri, Nov 15, 2013 at 03:10:49PM +0000, Martin Jambor wrote:
> 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;
> +}
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping
  2013-11-15 16:53       ` Martin Jambor
  2013-11-19 12:20         ` James Greenhalgh
@ 2013-11-19 16:26         ` Vladimir Makarov
  1 sibling, 0 replies; 11+ messages in thread
From: Vladimir Makarov @ 2013-11-19 16:26 UTC (permalink / raw)
  To: Matthew Leach, GCC Patches, Steven Bosscher, Jakub Jelinek

On 11/15/2013, 10:10 AM, Martin Jambor wrote:
> 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?
>
It looks ok too me, Martin.  I guess it is normal to have a few 
iterations here as RA(IRA/reload/LRA) is very complicated area even to 
me as there are too many details. I frequently ran in situation when 
fixing a bug for one target results in another target problems.

Thanks.
> 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.
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-11-19 15:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-06 16:43 [PATCH, PR 10474] Take two on splitting live-ranges of function arguments to help shrink-wrapping Martin Jambor
2013-11-08 21:14 ` Vladimir Makarov
2013-11-13 16:57 ` H.J. Lu
2013-11-14 10:09 ` Jakub Jelinek
2013-11-14 11:05   ` Martin Jambor
2013-11-14 14:22 ` Matthew Leach
2013-11-14 20:33   ` Martin Jambor
2013-11-15 10:10     ` Matthew Leach
2013-11-15 16:53       ` Martin Jambor
2013-11-19 12:20         ` James Greenhalgh
2013-11-19 16:26         ` Vladimir Makarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).