diff mbox

: [Bug tree-optimization/61917] [4.9/5 Regression] ICE on valid code at -O3 on x86_64-linux-gnu in vectorizable_reduction, at tree-vect-loop.c:4913

Message ID CAEwic4bmgzFQDMyGb6sq+VOU788dx54UJ2X0_bXkPv+WAmP4Yw@mail.gmail.com
State New
Headers show

Commit Message

Kai Tietz Feb. 25, 2015, 10:06 a.m. UTC
Hello,

ChangeLog

2015-02-25  Kai Tietz  <ktietz@redhat.com>

    PR tree-optimization/61917
    * tree-vect-loop.c (vectorizable_reduction): Allow
    vect_internal_def without reduction to exit graceful.

ChagneLog testsuite/

2015-02-25  Kai Tietz  <ktietz@redhat.com>

    PR tree-optimization/61917
    * gcc.dg/vect/vect-pr61917.c: New file.

Tested for x86_64-unkown-linux.  Ok for apply?
Regards,
Kai

Comments

Richard Biener Feb. 25, 2015, 10:57 a.m. UTC | #1
On Wed, Feb 25, 2015 at 11:06 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Hello,
>
> ChangeLog
>
> 2015-02-25  Kai Tietz  <ktietz@redhat.com>
>
>     PR tree-optimization/61917
>     * tree-vect-loop.c (vectorizable_reduction): Allow
>     vect_internal_def without reduction to exit graceful.
>
> ChagneLog testsuite/
>
> 2015-02-25  Kai Tietz  <ktietz@redhat.com>
>
>     PR tree-optimization/61917
>     * gcc.dg/vect/vect-pr61917.c: New file.
>
> Tested for x86_64-unkown-linux.  Ok for apply?

It doesn't make much sense to fail here as said in the comment
because of patterns if the actual case isn't a pattern.  Also
the patch causing this made vect_external_def possible, so
why does this affect vect_internal_def here?

It may paper over the issue - but clearly the fix is bogus.

Richard.

> Regards,
> Kai
>
> Index: tree-vect-loop.c
> ===================================================================
> --- tree-vect-loop.c    (Revision 220958)
> +++ tree-vect-loop.c    (Arbeitskopie)
> @@ -4990,7 +4990,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
>        /* For pattern recognized stmts, orig_stmt might be a reduction,
>       but some helper statements for the pattern might not, or
>       might be COND_EXPRs with reduction uses in the condition.  */
> -      gcc_assert (orig_stmt);
> +      gcc_assert (orig_stmt || dt == vect_internal_def);
>        return false;
>      }
>    if (!found_nested_cycle_def)
> Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
> ===================================================================
> --- /dev/null
> +++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3" } */
> +
> +int a, b, c, d;
> +
> +int
> +fn1 ()
> +{
> +  for (; c; c++)
> +    for (b = 0; b < 2; b++)
> +      d = a - d;
> +  return d;
> +}
Kai Tietz Feb. 25, 2015, 11:05 a.m. UTC | #2
2015-02-25 11:57 GMT+01:00 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Feb 25, 2015 at 11:06 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> ChangeLog
>>
>> 2015-02-25  Kai Tietz  <ktietz@redhat.com>
>>
>>     PR tree-optimization/61917
>>     * tree-vect-loop.c (vectorizable_reduction): Allow
>>     vect_internal_def without reduction to exit graceful.
>>
>> ChagneLog testsuite/
>>
>> 2015-02-25  Kai Tietz  <ktietz@redhat.com>
>>
>>     PR tree-optimization/61917
>>     * gcc.dg/vect/vect-pr61917.c: New file.
>>
>> Tested for x86_64-unkown-linux.  Ok for apply?
>
> It doesn't make much sense to fail here as said in the comment
> because of patterns if the actual case isn't a pattern.  Also
> the patch causing this made vect_external_def possible, so
> why does this affect vect_internal_def here?
>
> It may paper over the issue - but clearly the fix is bogus.

Well, actually I don't see any good reason for failing here at all,
but I assumed that author wanted to get by it some missed cases.
AFAIU the code we actually don't need/want to assert here either on
internal (where it is pretty likely no pattern present), and also on
externals, too.
I would be fine to remove this assert here completely, but I thought
it might be of interest to see that orig_stmt isn't NULL for nested
case

Kai

> Richard.
>
>> Regards,
>> Kai
>>
>> Index: tree-vect-loop.c
>> ===================================================================
>> --- tree-vect-loop.c    (Revision 220958)
>> +++ tree-vect-loop.c    (Arbeitskopie)
>> @@ -4990,7 +4990,7 @@ vectorizable_reduction (gimple stmt, gimple_stmt_i
>>        /* For pattern recognized stmts, orig_stmt might be a reduction,
>>       but some helper statements for the pattern might not, or
>>       might be COND_EXPRs with reduction uses in the condition.  */
>> -      gcc_assert (orig_stmt);
>> +      gcc_assert (orig_stmt || dt == vect_internal_def);
>>        return false;
>>      }
>>    if (!found_nested_cycle_def)
>> Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
>> ===================================================================
>> --- /dev/null
>> +++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
>> @@ -0,0 +1,13 @@
>> +/* { dg-do compile } */
>> +/* { dg-additional-options "-O3" } */
>> +
>> +int a, b, c, d;
>> +
>> +int
>> +fn1 ()
>> +{
>> +  for (; c; c++)
>> +    for (b = 0; b < 2; b++)
>> +      d = a - d;
>> +  return d;
>> +}
diff mbox

Patch

Index: tree-vect-loop.c
===================================================================
--- tree-vect-loop.c    (Revision 220958)
+++ tree-vect-loop.c    (Arbeitskopie)
@@ -4990,7 +4990,7 @@  vectorizable_reduction (gimple stmt, gimple_stmt_i
       /* For pattern recognized stmts, orig_stmt might be a reduction,
      but some helper statements for the pattern might not, or
      might be COND_EXPRs with reduction uses in the condition.  */
-      gcc_assert (orig_stmt);
+      gcc_assert (orig_stmt || dt == vect_internal_def);
       return false;
     }
   if (!found_nested_cycle_def)
Index: gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/vect/vect-pr61917.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-O3" } */
+
+int a, b, c, d;
+
+int
+fn1 ()
+{
+  for (; c; c++)
+    for (b = 0; b < 2; b++)
+      d = a - d;
+  return d;
+}