diff mbox

Fix ICE in rtl-optimization/PR61220, PR61225

Message ID CACgzC7Cqb-ZuL4WDJJRZ9SaN27Gy0p8gfH=-4ZbKe9AEh+LrGQ@mail.gmail.com
State New
Headers show

Commit Message

Zhenqiang Chen May 20, 2014, 7:11 a.m. UTC
Hi,

The patch fix ICE issue triggered by shrink-wrapping enhancement.

Bootstrap and no make check regression on X86-64.

OK for trunk?

Thanks!
-Zhenqiang


2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

        PR rtl-optimization/61220
        Part of PR rtl-optimization/61225
        * shrink-wrap.c (move_insn_for_shrink_wrap): Skip SP and FP adjustment
        insn; skip split_edge for a block without only on successor.

testsuite/ChangeLog:
2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

        * gcc.dg/pr61220.c: New test.
        * gcc.dg/shrink-wrap-loop.c: Disable for x86_64 -m32 mode.

Comments

Jeff Law May 20, 2014, 4:54 p.m. UTC | #1
On 05/20/14 01:11, Zhenqiang Chen wrote:
> Hi,
>
> The patch fix ICE issue triggered by shrink-wrapping enhancement.
>
> Bootstrap and no make check regression on X86-64.
>
> OK for trunk?
>
> Thanks!
> -Zhenqiang
>
>
> 2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>
>          PR rtl-optimization/61220
>          Part of PR rtl-optimization/61225
>          * shrink-wrap.c (move_insn_for_shrink_wrap): Skip SP and FP adjustment
>          insn; skip split_edge for a block without only on successor.
>
> testsuite/ChangeLog:
> 2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>
>          * gcc.dg/pr61220.c: New test.
>          * gcc.dg/shrink-wrap-loop.c: Disable for x86_64 -m32 mode.
>
[ ... ]



>     /* Make sure that the source register isn't defined later in BB.  */
> @@ -204,6 +209,10 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
>     /* Create a new basic block on the edge.  */
>     if (EDGE_COUNT (next_block->preds) == 2)
>       {
> +      /* split_edge for a block with only one successor is meaningless.  */
> +      if (EDGE_COUNT (bb->succs) == 1)
> +       return false;
> +
>         next_block = split_edge (live_edge);
So don't you just want to use:

if (EDGE_CRITICAL_P (live_edge))

to replace the two explicit checks of EDGE_COUNT on the preds & succs?

jeff
Zhenqiang Chen May 21, 2014, 2:12 a.m. UTC | #2
On 21 May 2014 00:54, Jeff Law <law@redhat.com> wrote:
> On 05/20/14 01:11, Zhenqiang Chen wrote:
>>
>> Hi,
>>
>> The patch fix ICE issue triggered by shrink-wrapping enhancement.
>>
>> Bootstrap and no make check regression on X86-64.
>>
>> OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>>
>> 2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>          PR rtl-optimization/61220
>>          Part of PR rtl-optimization/61225
>>          * shrink-wrap.c (move_insn_for_shrink_wrap): Skip SP and FP
>> adjustment
>>          insn; skip split_edge for a block without only on successor.
>>
>> testsuite/ChangeLog:
>> 2014-05-20  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>          * gcc.dg/pr61220.c: New test.
>>          * gcc.dg/shrink-wrap-loop.c: Disable for x86_64 -m32 mode.
>>
> [ ... ]
>
>
>
>
>>     /* Make sure that the source register isn't defined later in BB.  */
>> @@ -204,6 +209,10 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
>>     /* Create a new basic block on the edge.  */
>>     if (EDGE_COUNT (next_block->preds) == 2)
>>       {
>> +      /* split_edge for a block with only one successor is meaningless.
>> */
>> +      if (EDGE_COUNT (bb->succs) == 1)
>> +       return false;
>> +
>>         next_block = split_edge (live_edge);
>
> So don't you just want to use:
>
> if (EDGE_CRITICAL_P (live_edge))
>
> to replace the two explicit checks of EDGE_COUNT on the preds & succs?

Thanks for the comment.

In the code, there are 4 combinations of EDGE_COUNT: <1, 1>, <1, 2>,
<2, 1> and <2, 2>.  <2, 1> is "illegal". <2, 2> is legal, but need
split_edge. <1, *> can bypass the second check. EDGE_CRITICAL_P can
only distinguish <2, 2> from others.

So I think two explicit checks is more efficient than EDGE_CRITICAL_P.

Thanks!
-Zhenqiang
Jeff Law May 23, 2014, 6:20 p.m. UTC | #3
On 05/20/14 20:12, Zhenqiang Chen wrote:
> In the code, there are 4 combinations of EDGE_COUNT: <1, 1>, <1, 2>,
> <2, 1> and <2, 2>.  <2, 1> is "illegal". <2, 2> is legal, but need
> split_edge. <1, *> can bypass the second check. EDGE_CRITICAL_P can
> only distinguish <2, 2> from others.
>
> So I think two explicit checks is more efficient than EDGE_CRITICAL_P.
OK.  It was more a question of clarity than efficiency.  Your patch is 
fine as-is.

jeff
diff mbox

Patch

diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index f09cfe7..6f1ddc7 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -179,7 +179,12 @@  move_insn_for_shrink_wrap (basic_block bb, rtx insn,
     return false;
   src = SET_SRC (set);
   dest = SET_DEST (set);
-  if (!REG_P (dest) || !REG_P (src))
+  if (!REG_P (dest) || !REG_P (src)
+      /* STACK or FRAME related adjustment might be part of prologue.
+        So keep them in the entry block.  */
+      || dest == stack_pointer_rtx
+      || dest == frame_pointer_rtx
+      || dest == hard_frame_pointer_rtx)
     return false;

   /* Make sure that the source register isn't defined later in BB.  */
@@ -204,6 +209,10 @@  move_insn_for_shrink_wrap (basic_block bb, rtx insn,
   /* Create a new basic block on the edge.  */
   if (EDGE_COUNT (next_block->preds) == 2)
     {
+      /* split_edge for a block with only one successor is meaningless.  */
+      if (EDGE_COUNT (bb->succs) == 1)
+       return false;
+
       next_block = split_edge (live_edge);

       bitmap_copy (df_get_live_in (next_block), df_get_live_out (bb));
diff --git a/gcc/testsuite/gcc.dg/pr61220.c b/gcc/testsuite/gcc.dg/pr61220.c
new file mode 100644
index 0000000..c503ff1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61220.c
@@ -0,0 +1,39 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+int a, c, d, e, f, g, h, i, j, k;
+
+struct S0
+{
+  int f0;
+  int f1;
+  int f2;
+};
+
+struct S1
+{
+  int f0;
+  int f1;
+  struct S0 f2;
+} b;
+
+void
+fn1 (struct S1 p)
+{
+  for (; k; k++)
+    h = j ? a : a - 1;
+  d &= i;
+}
+
+int
+main ()
+{
+  int l[5] = { 0 };
+  fn1 (b);
+  for (c = 0; c < 3; c++)
+    for (g = 0; g < 3; g++)
+      l[c * 2] = e = l[c];
+  if (f)
+    fn1 (b);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
b/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
index 17dca4e..e72edfa 100644
--- a/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
+++ b/gcc/testsuite/gcc.dg/shrink-wrap-loop.c
@@ -1,4 +1,4 @@ 
-/* { dg-do compile { target { { x86_64-*-* } || { arm_thumb2 } } } } */
+/* { dg-do compile { target { { x86_64-*-* && lp64 } || { arm_thumb2 } } } } */
 /* { dg-options "-O2 -fdump-rtl-pro_and_epilogue"  } */

 int foo (int *p1, int *p2);