diff mbox

Fixing PR60773

Message ID CAK=A3=2CDfbQWmySY-402wgs2Or96d2MUEk63PTJse-ebgKCkg@mail.gmail.com
State New
Headers show

Commit Message

Cong Hou April 7, 2014, 7:16 p.m. UTC 
In the patch of
PR60656(http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01668.html), the
test case requires GCC to vectorize the widen-mult pattern from si to
di types. This may result in test failures on some platforms that
don't support this pattern. This patch adds a new target
vect_widen_mult_si_to_di_pattern to fix this issue.

Bootstrapped and tested on x86_64.

OK for trunk?


thanks,
Cong




+# Return 1 if the target plus current options supports a vector
 # widening shift, 0 otherwise.
 #
 # This won't change for different subtargets so cache the result.

Comments

Jakub Jelinek April 8, 2014, 7:07 a.m. UTC | #1 
On Mon, Apr 07, 2014 at 12:16:12PM -0700, Cong Hou wrote:
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,11 @@
> +2014-04-07  Cong Hou  <congh@google.com>
> +
> + PR testsuite/60773
> + * testsuite/lib/target-supports.exp:
> + Add check_effective_target_vect_widen_mult_si_to_di_pattern.

No testsuite/ prefix here.  Please write it as:
	* lib/target-supports.exp
	(check_effective_target_vect_widen_si_to_di_pattern): New.

> --- a/gcc/testsuite/gcc.dg/vect/pr60656.c
> +++ b/gcc/testsuite/gcc.dg/vect/pr60656.c
> @@ -1,5 +1,7 @@
>  /* { dg-require-effective-target vect_int } */
> +/* { dg-require-effective-target vect_long } */
> 
> +#include <stdarg.h>

I fail to see why you need this include, neither your test nor tree-vect.h
uses va_*.

Otherwise looks good to me.

	Jakub
Rainer Orth April 8, 2014, 7:28 a.m. UTC | #2 
Cong Hou <congh@google.com> writes:

> In the patch of
> PR60656(http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01668.html), the
> test case requires GCC to vectorize the widen-mult pattern from si to
> di types. This may result in test failures on some platforms that
> don't support this pattern. This patch adds a new target
> vect_widen_mult_si_to_di_pattern to fix this issue.

Please document the new keyword in gcc/doc/sourcebuild.texi.

> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 414a745..ea860e7 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,11 @@
> +2014-04-07  Cong Hou  <congh@google.com>
> +
> + PR testsuite/60773
> + * testsuite/lib/target-supports.exp:
> + Add check_effective_target_vect_widen_mult_si_to_di_pattern.
> + * gcc.dg/vect/pr60656.c: Update the test by checking if the targets
> + vect_widen_mult_si_to_di_pattern and vect_long are supported.
> +

Your mailer is broken: it swallows tabs and breaks long lines.  If you
can't fix it, please attach patches instead of sending them inline.

Thanks.

	Rainer
Cong Hou April 9, 2014, 2:40 a.m. UTC | #3 
On Tue, Apr 8, 2014 at 12:07 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Apr 07, 2014 at 12:16:12PM -0700, Cong Hou wrote:
>> --- a/gcc/testsuite/ChangeLog
>> +++ b/gcc/testsuite/ChangeLog
>> @@ -1,3 +1,11 @@
>> +2014-04-07  Cong Hou  <congh@google.com>
>> +
>> + PR testsuite/60773
>> + * testsuite/lib/target-supports.exp:
>> + Add check_effective_target_vect_widen_mult_si_to_di_pattern.
>
> No testsuite/ prefix here.  Please write it as:
>         * lib/target-supports.exp
>         (check_effective_target_vect_widen_si_to_di_pattern): New.

Thank you for pointing it out. Corrected.


>
>> --- a/gcc/testsuite/gcc.dg/vect/pr60656.c
>> +++ b/gcc/testsuite/gcc.dg/vect/pr60656.c
>> @@ -1,5 +1,7 @@
>>  /* { dg-require-effective-target vect_int } */
>> +/* { dg-require-effective-target vect_long } */
>>
>> +#include <stdarg.h>
>
> I fail to see why you need this include, neither your test nor tree-vect.h
> uses va_*.

I have removed this include.


thanks,
Cong


>
> Otherwise looks good to me.
>
>         Jakub
Jakub Jelinek April 9, 2014, 5:29 a.m. UTC | #4 
On Tue, Apr 08, 2014 at 07:40:09PM -0700, Cong Hou wrote:
> On Tue, Apr 8, 2014 at 12:07 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Mon, Apr 07, 2014 at 12:16:12PM -0700, Cong Hou wrote:
> >> --- a/gcc/testsuite/ChangeLog
> >> +++ b/gcc/testsuite/ChangeLog
> >> @@ -1,3 +1,11 @@
> >> +2014-04-07  Cong Hou  <congh@google.com>
> >> +
> >> + PR testsuite/60773
> >> + * testsuite/lib/target-supports.exp:
> >> + Add check_effective_target_vect_widen_mult_si_to_di_pattern.
> >
> > No testsuite/ prefix here.  Please write it as:
> >         * lib/target-supports.exp
> >         (check_effective_target_vect_widen_si_to_di_pattern): New.
> 
> Thank you for pointing it out. Corrected.

Only partially, I see there just the #include removed and testsuite/
prefix removed, but not the using the style where in () is the name
of function that has been added/modified and after ): what has been done
with it.

Anyway, please commit it with the above proposed change, this is a P1
and thus urgent.
Thanks.

	Jakub
diff mbox

Patch

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 414a745..ea860e7 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2014-04-07  Cong Hou  <congh@google.com>
+
+ PR testsuite/60773
+ * testsuite/lib/target-supports.exp:
+ Add check_effective_target_vect_widen_mult_si_to_di_pattern.
+ * gcc.dg/vect/pr60656.c: Update the test by checking if the targets
+ vect_widen_mult_si_to_di_pattern and vect_long are supported.
+
 2014-03-28  Cong Hou  <congh@google.com>

  PR tree-optimization/60656
diff --git a/gcc/testsuite/gcc.dg/vect/pr60656.c
b/gcc/testsuite/gcc.dg/vect/pr60656.c
index ebaab62..b80e008 100644
--- a/gcc/testsuite/gcc.dg/vect/pr60656.c
+++ b/gcc/testsuite/gcc.dg/vect/pr60656.c
@@ -1,5 +1,7 @@ 
 /* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target vect_long } */

+#include <stdarg.h>
 #include "tree-vect.h"

 __attribute__ ((noinline)) long
@@ -12,7 +14,7 @@  foo ()
   for(i = 0; i < 4; ++i)
     {
       long P = v[i];
-      s += P*P*P;
+      s += P * P * P;
     }
   return s;
 }
@@ -27,7 +29,7 @@  bar ()
   for(i = 0; i < 4; ++i)
     {
       long P = v[i];
-      s += P*P*P;
+      s += P * P * P;
       __asm__ volatile ("");
     }
   return s;
@@ -35,11 +37,12 @@  bar ()

 int main()
 {
+  check_vect ();
+
   if (foo () != bar ())
     abort ();
   return 0;
 }

-/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
target vect_widen_mult_si_to_di_pattern } } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
-
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index bee8471..6d9d689 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3732,6 +3732,27 @@  proc
check_effective_target_vect_widen_mult_hi_to_si_pattern { } {
 }

 # Return 1 if the target plus current options supports a vector
+# widening multiplication of *int* args into *long* result, 0 otherwise.
+#
+# This won't change for different subtargets so cache the result.
+
+proc check_effective_target_vect_widen_mult_si_to_di_pattern { } {
+    global et_vect_widen_mult_si_to_di_pattern
+
+    if [info exists et_vect_widen_mult_si_to_di_pattern_saved] {
+        verbose
"check_effective_target_vect_widen_mult_si_to_di_pattern: using cached
result" 2
+    } else {
+        if {[istarget ia64-*-*]
+              || [istarget i?86-*-*]
+      || [istarget x86_64-*-*] } {
+            set et_vect_widen_mult_si_to_di_pattern_saved 1
+        }
+    }
+    verbose "check_effective_target_vect_widen_mult_si_to_di_pattern:
returning $et_vect_widen_mult_si_to_di_pattern_saved" 2
+    return $et_vect_widen_mult_si_to_di_pattern_saved
+}
+