New warning for expanded vector operations

New warning for expanded vector operations

[Artem Shinkarov]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Hi

Here is a patch to inform a programmer about the expanded vector operation.
Bootstrapped on x86-unknown-linux-gnu.

ChangeLog:

        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
          produce the warning.
          (expand_vector_parallel): Adjust to produce the warning.
          (lower_vec_shuffle): Adjust to produce the warning.
        * gcc/common.opt: New warning Wvector-operation-expanded.
        * gcc/doc/invoke.texi: Document the wawning.


Ok?


Thanks,
Artem Shinkarov.

P.S. It is hard to write a reasonable testcase for the patch, because
one needs to guess which architecture would expand a given vector
operation. But the patch is trivial.

[-- Attachment #2: vector-op-warning.diff --]
[-- Type: text/plain, Size: 4277 bytes --]

Index: gcc/tree-vect-generic.c
===================================================================
--- gcc/tree-vect-generic.c	(revision 179464)
+++ gcc/tree-vect-generic.c	(working copy)
@@ -235,6 +235,10 @@ expand_vector_piecewise (gimple_stmt_ite
   int delta = tree_low_cst (part_width, 1)
 	      / tree_low_cst (TYPE_SIZE (TREE_TYPE (type)), 1);
   int i;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  warning_at (loc, OPT_Wvector_operation_expanded,
+	      "vector operation will be expanded piecewise");
 
   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
   for (i = 0; i < nunits;
@@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
   tree result, compute_type;
   enum machine_mode mode;
   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  warning_at (loc, OPT_Wvector_operation_expanded,
+	      "vector operation will be expanded in parallel");
 
   /* We have three strategies.  If the type is already correct, just do
      the operation an element at a time.  Else, if the vector is wider than
@@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
 {
   int parts_per_word = UNITS_PER_WORD
 	  	       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
+  location_t loc = gimple_location (gsi_stmt (*gsi));
 
   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
       && parts_per_word >= 4
       && TYPE_VECTOR_SUBPARTS (type) >= 4)
-    return expand_vector_parallel (gsi, f_parallel,
-				   type, a, b, code);
+    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
   else
-    return expand_vector_piecewise (gsi, f,
-				    type, TREE_TYPE (type),
-				    a, b, code);
+    return expand_vector_piecewise (gsi, f, type,
+				    TREE_TYPE (type), a, b, code);
 }
 
 /* Check if vector VEC consists of all the equal elements and
@@ -400,8 +407,8 @@ expand_vector_operation (gimple_stmt_ite
       case PLUS_EXPR:
       case MINUS_EXPR:
         if (!TYPE_OVERFLOW_TRAPS (type))
-          return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
-		      		         gimple_assign_rhs1 (assign),
+	  return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
+					 gimple_assign_rhs1 (assign),
 					 gimple_assign_rhs2 (assign), code);
 	break;
 
@@ -622,6 +629,8 @@ lower_vec_shuffle (gimple_stmt_iterator
       return true;
     }
 
+  warning_at (loc, OPT_Wvector_operation_expanded,
+	      "vector shuffling operation will be expanded piecewise");
   if (operand_equal_p (vec0, vec1, 0))
     {
       unsigned i;
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 179464)
+++ gcc/common.opt	(working copy)
@@ -694,6 +694,10 @@ Wcoverage-mismatch
 Common Var(warn_coverage_mismatch) Init(1) Warning
 Warn in case profiles in -fprofile-use do not match
 
+Wvector-operation-expanded
+Common Var(warn_vector_operation_expanded) Warning
+Warn when a vector operation is expanded piecewise
+
 Xassembler
 Driver Separate
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 179464)
+++ gcc/doc/invoke.texi	(working copy)
@@ -271,7 +271,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
 -Wunused-but-set-parameter -Wunused-but-set-variable @gol
--Wvariadic-macros -Wvla -Wvolatile-register-var  -Wwrite-strings}
+-Wvariadic-macros -Wvector-operation-expanded -Wvla 
+-Wvolatile-register-var  -Wwrite-strings}
 
 @item C and Objective-C-only Warning Options
 @gccoptlist{-Wbad-function-cast  -Wmissing-declarations @gol
@@ -4532,6 +4533,12 @@ Warn if variadic macros are used in peda
 alternate syntax when in pedantic ISO C99 mode.  This is default.
 To inhibit the warning messages, use @option{-Wno-variadic-macros}.
 
+@item -Wvector-operation-expanded
+@opindex Wvector-operation-expanded
+@opindex Wno-vector-operation-expanded
+Warn if vector operation is not implemented via SIMD capabilities of the
+architecture. Mainly useful for the performance tuning.
+
 @item -Wvla
 @opindex Wvla
 @opindex Wno-vla

Re: New warning for expanded vector operations

[Richard Guenther]

On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> Hi
>
> Here is a patch to inform a programmer about the expanded vector operation.
> Bootstrapped on x86-unknown-linux-gnu.
>
> ChangeLog:
>
>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>          produce the warning.
>          (expand_vector_parallel): Adjust to produce the warning.

Entries start without gcc/, they are relative to the gcc/ChangeLog file.

>          (lower_vec_shuffle): Adjust to produce the warning.
>        * gcc/common.opt: New warning Wvector-operation-expanded.
>        * gcc/doc/invoke.texi: Document the wawning.
>
>
> Ok?

I don't like the name -Wvector-operation-expanded.  We emit a
similar warning for missed inline expansions with -Winline, so
maybe -Wvector-extensions (that's the name that appears
in the C extension documentation).

+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  warning_at (loc, OPT_Wvector_operation_expanded,
+	      "vector operation will be expanded piecewise");

   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
   for (i = 0; i < nunits;
@@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
   tree result, compute_type;
   enum machine_mode mode;
   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  warning_at (loc, OPT_Wvector_operation_expanded,
+	      "vector operation will be expanded in parallel");

what's the difference between 'piecewise' and 'in parallel'?

@@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
 {
   int parts_per_word = UNITS_PER_WORD
 	  	       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
+  location_t loc = gimple_location (gsi_stmt (*gsi));

   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
       && parts_per_word >= 4
       && TYPE_VECTOR_SUBPARTS (type) >= 4)
-    return expand_vector_parallel (gsi, f_parallel,
-				   type, a, b, code);
+    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
   else
-    return expand_vector_piecewise (gsi, f,
-				    type, TREE_TYPE (type),
-				    a, b, code);
+    return expand_vector_piecewise (gsi, f, type,
+				    TREE_TYPE (type), a, b, code);
 }

 /* Check if vector VEC consists of all the equal elements and

unless i miss something loc is unused here.  Please avoid random
whitespace changes (just review your patch yourself before posting
and revert pieces that do nothing).

+@item -Wvector-operation-expanded
+@opindex Wvector-operation-expanded
+@opindex Wno-vector-operation-expanded
+Warn if vector operation is not implemented via SIMD capabilities of the
+architecture. Mainly useful for the performance tuning.

I'd mention that this is for vector operations as of the C extension
documented in "Vector Extensions".

The vectorizer can produce some operations that will need further
lowering - we probably should make sure to _not_ warn about those.
Try running the vect.exp testsuite with the new warning turned on
(eventually disabling SSE), like with

obj/gcc> make check-gcc
RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
vect.exp"

> P.S. It is hard to write a reasonable testcase for the patch, because
> one needs to guess which architecture would expand a given vector
> operation. But the patch is trivial.

You can create an aritificial large vector type for example, or put a
testcase under gcc.target/i386 and disable SSE.  We should have
a testcase for this.

Thanks,
Richard.

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> Hi
>>
>> Here is a patch to inform a programmer about the expanded vector operation.
>> Bootstrapped on x86-unknown-linux-gnu.
>>
>> ChangeLog:
>>
>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>          produce the warning.
>>          (expand_vector_parallel): Adjust to produce the warning.
>
> Entries start without gcc/, they are relative to the gcc/ChangeLog file.

Sure, sorry.

>>          (lower_vec_shuffle): Adjust to produce the warning.
>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>        * gcc/doc/invoke.texi: Document the wawning.
>>
>>
>> Ok?
>
> I don't like the name -Wvector-operation-expanded.  We emit a
> similar warning for missed inline expansions with -Winline, so
> maybe -Wvector-extensions (that's the name that appears
> in the C extension documentation).

Hm, I don't care much about the name, unless it gets clear what the
warning is used for.  I am not really sure that Wvector-extensions
makes it clear.  Also, I don't see anything bad if the warning will
pop up during the vectorisation. Any vector operation performed
outside the SIMD accelerator looks suspicious, because it actually
doesn't improve performance.  Such a warning during the vectorisation
could mean that a programmer forgot some flag, or the constant
propagation failed to deliver a constant, or something else.

Conceptually the text I am producing is not really a warning, it is
more like an information, but I am not aware of the mechanisms that
would allow me to introduce a flag triggering inform () or something
similar.

What I think we really need to avoid is including this warning in the
standard Ox.

> +  location_t loc = gimple_location (gsi_stmt (*gsi));
> +
> +  warning_at (loc, OPT_Wvector_operation_expanded,
> +             "vector operation will be expanded piecewise");
>
>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>   for (i = 0; i < nunits;
> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>   tree result, compute_type;
>   enum machine_mode mode;
>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
> +  location_t loc = gimple_location (gsi_stmt (*gsi));
> +
> +  warning_at (loc, OPT_Wvector_operation_expanded,
> +             "vector operation will be expanded in parallel");
>
> what's the difference between 'piecewise' and 'in parallel'?

Parallel is a little bit better for performance than piecewise.

> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>  {
>   int parts_per_word = UNITS_PER_WORD
>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>
>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>       && parts_per_word >= 4
>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
> -    return expand_vector_parallel (gsi, f_parallel,
> -                                  type, a, b, code);
> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>   else
> -    return expand_vector_piecewise (gsi, f,
> -                                   type, TREE_TYPE (type),
> -                                   a, b, code);
> +    return expand_vector_piecewise (gsi, f, type,
> +                                   TREE_TYPE (type), a, b, code);
>  }
>
>  /* Check if vector VEC consists of all the equal elements and
>
> unless i miss something loc is unused here.  Please avoid random
> whitespace changes (just review your patch yourself before posting
> and revert pieces that do nothing).

Yes you are right, sorry.

> +@item -Wvector-operation-expanded
> +@opindex Wvector-operation-expanded
> +@opindex Wno-vector-operation-expanded
> +Warn if vector operation is not implemented via SIMD capabilities of the
> +architecture. Mainly useful for the performance tuning.
>
> I'd mention that this is for vector operations as of the C extension
> documented in "Vector Extensions".
>
> The vectorizer can produce some operations that will need further
> lowering - we probably should make sure to _not_ warn about those.
> Try running the vect.exp testsuite with the new warning turned on
> (eventually disabling SSE), like with
>
> obj/gcc> make check-gcc
> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
> vect.exp"

Again, see the comment above. I think, if the warning can be triggered
only manually, then we are fine. But I'll check anyway how many
warnings I'll get from vect.exp.

>> P.S. It is hard to write a reasonable testcase for the patch, because
>> one needs to guess which architecture would expand a given vector
>> operation. But the patch is trivial.
>
> You can create an aritificial large vector type for example, or put a
> testcase under gcc.target/i386 and disable SSE.  We should have
> a testcase for this.

Yeah, disabling SSE should help.


Thanks,
Artem.
> Thanks,
> Richard.
>

Re: New warning for expanded vector operations

[Richard Guenther]

On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> Hi
>>>
>>> Here is a patch to inform a programmer about the expanded vector operation.
>>> Bootstrapped on x86-unknown-linux-gnu.
>>>
>>> ChangeLog:
>>>
>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>          produce the warning.
>>>          (expand_vector_parallel): Adjust to produce the warning.
>>
>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>
> Sure, sorry.
>
>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>
>>>
>>> Ok?
>>
>> I don't like the name -Wvector-operation-expanded.  We emit a
>> similar warning for missed inline expansions with -Winline, so
>> maybe -Wvector-extensions (that's the name that appears
>> in the C extension documentation).
>
> Hm, I don't care much about the name, unless it gets clear what the
> warning is used for.  I am not really sure that Wvector-extensions
> makes it clear.  Also, I don't see anything bad if the warning will
> pop up during the vectorisation. Any vector operation performed
> outside the SIMD accelerator looks suspicious, because it actually
> doesn't improve performance.  Such a warning during the vectorisation
> could mean that a programmer forgot some flag, or the constant
> propagation failed to deliver a constant, or something else.
>
> Conceptually the text I am producing is not really a warning, it is
> more like an information, but I am not aware of the mechanisms that
> would allow me to introduce a flag triggering inform () or something
> similar.
>
> What I think we really need to avoid is including this warning in the
> standard Ox.
>
>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>> +
>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>> +             "vector operation will be expanded piecewise");
>>
>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>   for (i = 0; i < nunits;
>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>   tree result, compute_type;
>>   enum machine_mode mode;
>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>> +
>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>> +             "vector operation will be expanded in parallel");
>>
>> what's the difference between 'piecewise' and 'in parallel'?
>
> Parallel is a little bit better for performance than piecewise.

I see.  That difference should probably be documented, maybe with
an example.

Richard.

>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>  {
>>   int parts_per_word = UNITS_PER_WORD
>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>
>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>       && parts_per_word >= 4
>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>> -    return expand_vector_parallel (gsi, f_parallel,
>> -                                  type, a, b, code);
>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>   else
>> -    return expand_vector_piecewise (gsi, f,
>> -                                   type, TREE_TYPE (type),
>> -                                   a, b, code);
>> +    return expand_vector_piecewise (gsi, f, type,
>> +                                   TREE_TYPE (type), a, b, code);
>>  }
>>
>>  /* Check if vector VEC consists of all the equal elements and
>>
>> unless i miss something loc is unused here.  Please avoid random
>> whitespace changes (just review your patch yourself before posting
>> and revert pieces that do nothing).
>
> Yes you are right, sorry.
>
>> +@item -Wvector-operation-expanded
>> +@opindex Wvector-operation-expanded
>> +@opindex Wno-vector-operation-expanded
>> +Warn if vector operation is not implemented via SIMD capabilities of the
>> +architecture. Mainly useful for the performance tuning.
>>
>> I'd mention that this is for vector operations as of the C extension
>> documented in "Vector Extensions".
>>
>> The vectorizer can produce some operations that will need further
>> lowering - we probably should make sure to _not_ warn about those.
>> Try running the vect.exp testsuite with the new warning turned on
>> (eventually disabling SSE), like with
>>
>> obj/gcc> make check-gcc
>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>> vect.exp"
>
> Again, see the comment above. I think, if the warning can be triggered
> only manually, then we are fine. But I'll check anyway how many
> warnings I'll get from vect.exp.
>
>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>> one needs to guess which architecture would expand a given vector
>>> operation. But the patch is trivial.
>>
>> You can create an aritificial large vector type for example, or put a
>> testcase under gcc.target/i386 and disable SSE.  We should have
>> a testcase for this.
>
> Yeah, disabling SSE should help.
>
>
> Thanks,
> Artem.
>> Thanks,
>> Richard.
>>
>

Re: New warning for expanded vector operations

[Artem Shinkarov]

[-- Attachment #1: Type: text/plain, Size: 6389 bytes --]

On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>> <artyom.shinkaroff@gmail.com> wrote:
>>>> Hi
>>>>
>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>
>>>> ChangeLog:
>>>>
>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>          produce the warning.
>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>
>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>
>> Sure, sorry.
>>
>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>
>>>>
>>>> Ok?
>>>
>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>> similar warning for missed inline expansions with -Winline, so
>>> maybe -Wvector-extensions (that's the name that appears
>>> in the C extension documentation).
>>
>> Hm, I don't care much about the name, unless it gets clear what the
>> warning is used for.  I am not really sure that Wvector-extensions
>> makes it clear.  Also, I don't see anything bad if the warning will
>> pop up during the vectorisation. Any vector operation performed
>> outside the SIMD accelerator looks suspicious, because it actually
>> doesn't improve performance.  Such a warning during the vectorisation
>> could mean that a programmer forgot some flag, or the constant
>> propagation failed to deliver a constant, or something else.
>>
>> Conceptually the text I am producing is not really a warning, it is
>> more like an information, but I am not aware of the mechanisms that
>> would allow me to introduce a flag triggering inform () or something
>> similar.
>>
>> What I think we really need to avoid is including this warning in the
>> standard Ox.
>>
>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>> +
>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>> +             "vector operation will be expanded piecewise");
>>>
>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>   for (i = 0; i < nunits;
>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>   tree result, compute_type;
>>>   enum machine_mode mode;
>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>> +
>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>> +             "vector operation will be expanded in parallel");
>>>
>>> what's the difference between 'piecewise' and 'in parallel'?
>>
>> Parallel is a little bit better for performance than piecewise.
>
> I see.  That difference should probably be documented, maybe with
> an example.
>
> Richard.
>
>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>  {
>>>   int parts_per_word = UNITS_PER_WORD
>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>
>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>       && parts_per_word >= 4
>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>> -    return expand_vector_parallel (gsi, f_parallel,
>>> -                                  type, a, b, code);
>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>   else
>>> -    return expand_vector_piecewise (gsi, f,
>>> -                                   type, TREE_TYPE (type),
>>> -                                   a, b, code);
>>> +    return expand_vector_piecewise (gsi, f, type,
>>> +                                   TREE_TYPE (type), a, b, code);
>>>  }
>>>
>>>  /* Check if vector VEC consists of all the equal elements and
>>>
>>> unless i miss something loc is unused here.  Please avoid random
>>> whitespace changes (just review your patch yourself before posting
>>> and revert pieces that do nothing).
>>
>> Yes you are right, sorry.
>>
>>> +@item -Wvector-operation-expanded
>>> +@opindex Wvector-operation-expanded
>>> +@opindex Wno-vector-operation-expanded
>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>> +architecture. Mainly useful for the performance tuning.
>>>
>>> I'd mention that this is for vector operations as of the C extension
>>> documented in "Vector Extensions".
>>>
>>> The vectorizer can produce some operations that will need further
>>> lowering - we probably should make sure to _not_ warn about those.
>>> Try running the vect.exp testsuite with the new warning turned on
>>> (eventually disabling SSE), like with
>>>
>>> obj/gcc> make check-gcc
>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>> vect.exp"
>>
>> Again, see the comment above. I think, if the warning can be triggered
>> only manually, then we are fine. But I'll check anyway how many
>> warnings I'll get from vect.exp.
>>
>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>> one needs to guess which architecture would expand a given vector
>>>> operation. But the patch is trivial.
>>>
>>> You can create an aritificial large vector type for example, or put a
>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>> a testcase for this.
>>
>> Yeah, disabling SSE should help.
>>
>>
>> Thanks,
>> Artem.
>>> Thanks,
>>> Richard.
>>>
>>
>

New version of the patch in the attachment with the test-cases.
Bootstrapped on  x86_64-apple-darwin10.8.0.
Currently is being tested.


Richard, I've checked the vect.exp case, as you suggested.  It caused
a lot of failures, but not because of the new warning.  The main
reason is -mno-sse.  The target is capable to vectorize, so the dg
option expects tests to pass, but the artificial option makes them
fail.  Checking the new warning on vect.exp without -mno-sse, it
didn't cause any new failures.  Anyway, we should be pretty much safe,
cause the warning is not a part of -O3.

Thanks,
Artem.

[-- Attachment #2: vector-op-warning-1.diff --]
[-- Type: application/octet-stream, Size: 9719 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 179636)
+++ gcc/doc/invoke.texi	(working copy)
@@ -271,7 +271,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
 -Wunused-but-set-parameter -Wunused-but-set-variable @gol
--Wvariadic-macros -Wvla -Wvolatile-register-var  -Wwrite-strings}
+-Wvariadic-macros -Wvector-operation-performance -Wvla 
+-Wvolatile-register-var  -Wwrite-strings}
 
 @item C and Objective-C-only Warning Options
 @gccoptlist{-Wbad-function-cast  -Wmissing-declarations @gol
@@ -4535,6 +4536,18 @@ Warn if variadic macros are used in peda
 alternate syntax when in pedantic ISO C99 mode.  This is default.
 To inhibit the warning messages, use @option{-Wno-variadic-macros}.
 
+@item -Wvector-operation-performance
+@opindex Wvector-operation-performance
+@opindex Wno-vector-operation-performance
+Warn if vector operation is not implemented via SIMD capabilities of the
+architecture.  Mainly useful for the performance tuning.
+Vector operation can be implemented @code{piecewise} which means that the
+scalar operation is performed on every vector element; 
+@code{in parallel} which means that the vector operation is implemented
+using scalars of wider type, which normally is more performance efficient;
+and @code{as a single scalar} which means that vector fits into a
+scalar type.
+
 @item -Wvla
 @opindex Wvla
 @opindex Wno-vla
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-3.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (8, short) v0 = {argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (8, short) v1 = {-4, argc, 2, 11, 1, 17, -8, argc};
+  vector (8, short) res[] = 
+  {
+    v0 + v1,	      /* { dg-warning "expanded in parallel" }  */
+    v0 - v1,          /* { dg-warning "expanded in parallel" }  */
+    v0 > v1,          /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,          /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),	      /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)    /* { dg-warning "expanded piecewise" }  */
+  };
+  
+  return res[argc][argc];
+}
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (4, int) v0 = {argc, 1, 15, 38};
+  vector (4, int) v1 = {-4, argc, 2, 11};
+  vector (4, int) res[] = 
+  {
+    v0 + v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 - v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 > v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,	  /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),	    /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)  /* { dg-warning "expanded piecewise" }  */  
+  };
+
+  return res[argc][argc];
+}
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (16, signed char) v0 = {argc, 1, 15, 38, 12, -1, argc, 2, 
+				 argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (16, signed char) v1 = {-4, argc, 2, 11, 1, 17, -8, argc,
+				 argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (16, signed char) res[] = 
+  {
+    v0 + v1,		  /* { dg-warning "expanded in parallel" }  */
+    v0 - v1,              /* { dg-warning "expanded in parallel" }  */
+    v0 > v1,              /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,              /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),        /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)     /* { dg-warning "expanded piecewise" }  */
+  };
+ 
+  return res[argc][argc];
+}
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 179636)
+++ gcc/common.opt	(working copy)
@@ -694,6 +694,10 @@ Wcoverage-mismatch
 Common Var(warn_coverage_mismatch) Init(1) Warning
 Warn in case profiles in -fprofile-use do not match
 
+Wvector-operation-performance
+Common Var(warn_vector_operation_performance) Warning
+Warn when a vector operation is compiled outside the SIMD
+
 Xassembler
 Driver Separate
 
Index: gcc/tree-vect-generic.c
===================================================================
--- gcc/tree-vect-generic.c	(revision 179636)
+++ gcc/tree-vect-generic.c	(working copy)
@@ -235,6 +235,14 @@ expand_vector_piecewise (gimple_stmt_ite
   int delta = tree_low_cst (part_width, 1)
 	      / tree_low_cst (TYPE_SIZE (TREE_TYPE (type)), 1);
   int i;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  if (gimple_expr_type (gsi_stmt (*gsi)) == type)
+    warning_at (loc, OPT_Wvector_operation_performance,
+		"vector operation will be expanded piecewise");
+  else
+    warning_at (loc, OPT_Wvector_operation_performance,
+		"vector operation will be expanded in parallel");
 
   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
   for (i = 0; i < nunits;
@@ -260,6 +268,7 @@ expand_vector_parallel (gimple_stmt_iter
   tree result, compute_type;
   enum machine_mode mode;
   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
 
   /* We have three strategies.  If the type is already correct, just do
      the operation an element at a time.  Else, if the vector is wider than
@@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
       compute_type = lang_hooks.types.type_for_mode (mode, 1);
       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
+      warning_at (loc, OPT_Wvector_operation_performance,
+	          "vector operation will be expanded with a "
+		  "single scalar operation");
     }
 
   return result;
@@ -308,7 +320,7 @@ expand_vector_addition (gimple_stmt_iter
     return expand_vector_parallel (gsi, f_parallel,
 				   type, a, b, code);
   else
-    return expand_vector_piecewise (gsi, f,
+    return expand_vector_piecewise (gsi, f, 
 				    type, TREE_TYPE (type),
 				    a, b, code);
 }
@@ -400,8 +412,8 @@ expand_vector_operation (gimple_stmt_ite
       case PLUS_EXPR:
       case MINUS_EXPR:
         if (!TYPE_OVERFLOW_TRAPS (type))
-          return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
-		      		         gimple_assign_rhs1 (assign),
+	  return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
+					 gimple_assign_rhs1 (assign),
 					 gimple_assign_rhs2 (assign), code);
 	break;
 
@@ -626,10 +638,14 @@ lower_vec_shuffle (gimple_stmt_iterator 
   tree constr, t, si, i_val;
   tree vec0tmp = NULL_TREE, vec1tmp = NULL_TREE, masktmp = NULL_TREE;
   bool two_operand_p = !operand_equal_p (vec0, vec1, 0);
+  location_t loc = gimple_location (gsi_stmt (*gsi));
   unsigned i;
 
   if (expand_vec_shuffle_expr_p (TYPE_MODE (vect_type), vec0, vec1, mask))
     return;
+  
+  warning_at (loc, OPT_Wvector_operation_performance,
+              "vector shuffling operation will be expanded piecewise");
 
   v = VEC_alloc (constructor_elt, gc, elements);
   for (i = 0; i < elements; i++)
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 179636)
+++ gcc/c-parser.c	(working copy)
@@ -6533,17 +6533,22 @@ c_parser_postfix_expression (c_parser *p
 	      }
 
 	    if (VEC_length (c_expr_t, cexpr_list) == 2)
-	      expr.value =
-		c_build_vec_shuffle_expr
-		  (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
-		   NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
-
+	      {
+		expr.value =
+		  c_build_vec_shuffle_expr
+		    (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
+		     NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
+		SET_EXPR_LOCATION (expr.value, loc);
+	      }
 	    else if (VEC_length (c_expr_t, cexpr_list) == 3)
-	      expr.value =
-		c_build_vec_shuffle_expr
-		  (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
-		   VEC_index (c_expr_t, cexpr_list, 1)->value,
-		   VEC_index (c_expr_t, cexpr_list, 2)->value);
+	      {
+		expr.value =
+		  c_build_vec_shuffle_expr
+		    (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
+		     VEC_index (c_expr_t, cexpr_list, 1)->value,
+		     VEC_index (c_expr_t, cexpr_list, 2)->value);
+		SET_EXPR_LOCATION (expr.value, loc);
+	      }
 	    else
 	      {
 		error_at (loc, "wrong number of arguments to "

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Fri, Oct 7, 2011 at 6:22 AM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>> Hi
>>>>>
>>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>>
>>>>> ChangeLog:
>>>>>
>>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>>          produce the warning.
>>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>>
>>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>>
>>> Sure, sorry.
>>>
>>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>>
>>>>>
>>>>> Ok?
>>>>
>>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>>> similar warning for missed inline expansions with -Winline, so
>>>> maybe -Wvector-extensions (that's the name that appears
>>>> in the C extension documentation).
>>>
>>> Hm, I don't care much about the name, unless it gets clear what the
>>> warning is used for.  I am not really sure that Wvector-extensions
>>> makes it clear.  Also, I don't see anything bad if the warning will
>>> pop up during the vectorisation. Any vector operation performed
>>> outside the SIMD accelerator looks suspicious, because it actually
>>> doesn't improve performance.  Such a warning during the vectorisation
>>> could mean that a programmer forgot some flag, or the constant
>>> propagation failed to deliver a constant, or something else.
>>>
>>> Conceptually the text I am producing is not really a warning, it is
>>> more like an information, but I am not aware of the mechanisms that
>>> would allow me to introduce a flag triggering inform () or something
>>> similar.
>>>
>>> What I think we really need to avoid is including this warning in the
>>> standard Ox.
>>>
>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>> +
>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>> +             "vector operation will be expanded piecewise");
>>>>
>>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>>   for (i = 0; i < nunits;
>>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>>   tree result, compute_type;
>>>>   enum machine_mode mode;
>>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>> +
>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>> +             "vector operation will be expanded in parallel");
>>>>
>>>> what's the difference between 'piecewise' and 'in parallel'?
>>>
>>> Parallel is a little bit better for performance than piecewise.
>>
>> I see.  That difference should probably be documented, maybe with
>> an example.
>>
>> Richard.
>>
>>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>>  {
>>>>   int parts_per_word = UNITS_PER_WORD
>>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>
>>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>>       && parts_per_word >= 4
>>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>>> -    return expand_vector_parallel (gsi, f_parallel,
>>>> -                                  type, a, b, code);
>>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>>   else
>>>> -    return expand_vector_piecewise (gsi, f,
>>>> -                                   type, TREE_TYPE (type),
>>>> -                                   a, b, code);
>>>> +    return expand_vector_piecewise (gsi, f, type,
>>>> +                                   TREE_TYPE (type), a, b, code);
>>>>  }
>>>>
>>>>  /* Check if vector VEC consists of all the equal elements and
>>>>
>>>> unless i miss something loc is unused here.  Please avoid random
>>>> whitespace changes (just review your patch yourself before posting
>>>> and revert pieces that do nothing).
>>>
>>> Yes you are right, sorry.
>>>
>>>> +@item -Wvector-operation-expanded
>>>> +@opindex Wvector-operation-expanded
>>>> +@opindex Wno-vector-operation-expanded
>>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>>> +architecture. Mainly useful for the performance tuning.
>>>>
>>>> I'd mention that this is for vector operations as of the C extension
>>>> documented in "Vector Extensions".
>>>>
>>>> The vectorizer can produce some operations that will need further
>>>> lowering - we probably should make sure to _not_ warn about those.
>>>> Try running the vect.exp testsuite with the new warning turned on
>>>> (eventually disabling SSE), like with
>>>>
>>>> obj/gcc> make check-gcc
>>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>>> vect.exp"
>>>
>>> Again, see the comment above. I think, if the warning can be triggered
>>> only manually, then we are fine. But I'll check anyway how many
>>> warnings I'll get from vect.exp.
>>>
>>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>>> one needs to guess which architecture would expand a given vector
>>>>> operation. But the patch is trivial.
>>>>
>>>> You can create an aritificial large vector type for example, or put a
>>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>>> a testcase for this.
>>>
>>> Yeah, disabling SSE should help.
>>>
>>>
>>> Thanks,
>>> Artem.
>>>> Thanks,
>>>> Richard.
>>>>
>>>
>>
>
> New version of the patch in the attachment with the test-cases.
> Bootstrapped on  x86_64-apple-darwin10.8.0.
> Currently is being tested.
>
>
> Richard, I've checked the vect.exp case, as you suggested.  It caused
> a lot of failures, but not because of the new warning.  The main
> reason is -mno-sse.  The target is capable to vectorize, so the dg
> option expects tests to pass, but the artificial option makes them
> fail.  Checking the new warning on vect.exp without -mno-sse, it
> didn't cause any new failures.  Anyway, we should be pretty much safe,
> cause the warning is not a part of -O3.
>
> Thanks,
> Artem.
>

Successfully regression-tested on x86_64-apple-darwin10.8.0.

ChangeLog:
	gcc/
	* doc/invoke.texi: Document new warning.
	* common.opt (Wvector-operation-performance): Define new warning.
	* tree-vect-generic.c (expand_vector_piecewise): Warn about expanded
	vector operation.
	(exapnd_vector_parallel): Warn about expanded vector operation.
	(lower_vec_shuffle): Warn about expanded vector operation.
	* c-parser.c (c_parser_postfix_expression): Assign correct location
	when creating VEC_SHUFFLE_EXPR.
	
	gcc/testsuite/
	* gcc.target/i386/warn-vect-op-3.c: New test.
	* gcc.target/i386/warn-vect-op-1.c: New test.
	* gcc.target/i386/warn-vect-op-2.c: New test.

Ok for trunk?


Thanks,
Artem.

Re: New warning for expanded vector operations

[Richard Guenther]

On Fri, Oct 7, 2011 at 9:44 AM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Fri, Oct 7, 2011 at 6:22 AM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
>>> <artyom.shinkaroff@gmail.com> wrote:
>>>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>> Hi
>>>>>>
>>>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>>>
>>>>>> ChangeLog:
>>>>>>
>>>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>>>          produce the warning.
>>>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>>>
>>>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>>>
>>>> Sure, sorry.
>>>>
>>>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>>>
>>>>>>
>>>>>> Ok?
>>>>>
>>>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>>>> similar warning for missed inline expansions with -Winline, so
>>>>> maybe -Wvector-extensions (that's the name that appears
>>>>> in the C extension documentation).
>>>>
>>>> Hm, I don't care much about the name, unless it gets clear what the
>>>> warning is used for.  I am not really sure that Wvector-extensions
>>>> makes it clear.  Also, I don't see anything bad if the warning will
>>>> pop up during the vectorisation. Any vector operation performed
>>>> outside the SIMD accelerator looks suspicious, because it actually
>>>> doesn't improve performance.  Such a warning during the vectorisation
>>>> could mean that a programmer forgot some flag, or the constant
>>>> propagation failed to deliver a constant, or something else.
>>>>
>>>> Conceptually the text I am producing is not really a warning, it is
>>>> more like an information, but I am not aware of the mechanisms that
>>>> would allow me to introduce a flag triggering inform () or something
>>>> similar.
>>>>
>>>> What I think we really need to avoid is including this warning in the
>>>> standard Ox.
>>>>
>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>> +
>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>> +             "vector operation will be expanded piecewise");
>>>>>
>>>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>>>   for (i = 0; i < nunits;
>>>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>>>   tree result, compute_type;
>>>>>   enum machine_mode mode;
>>>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>> +
>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>> +             "vector operation will be expanded in parallel");
>>>>>
>>>>> what's the difference between 'piecewise' and 'in parallel'?
>>>>
>>>> Parallel is a little bit better for performance than piecewise.
>>>
>>> I see.  That difference should probably be documented, maybe with
>>> an example.
>>>
>>> Richard.
>>>
>>>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>>>  {
>>>>>   int parts_per_word = UNITS_PER_WORD
>>>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>
>>>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>>>       && parts_per_word >= 4
>>>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>>>> -    return expand_vector_parallel (gsi, f_parallel,
>>>>> -                                  type, a, b, code);
>>>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>>>   else
>>>>> -    return expand_vector_piecewise (gsi, f,
>>>>> -                                   type, TREE_TYPE (type),
>>>>> -                                   a, b, code);
>>>>> +    return expand_vector_piecewise (gsi, f, type,
>>>>> +                                   TREE_TYPE (type), a, b, code);
>>>>>  }
>>>>>
>>>>>  /* Check if vector VEC consists of all the equal elements and
>>>>>
>>>>> unless i miss something loc is unused here.  Please avoid random
>>>>> whitespace changes (just review your patch yourself before posting
>>>>> and revert pieces that do nothing).
>>>>
>>>> Yes you are right, sorry.
>>>>
>>>>> +@item -Wvector-operation-expanded
>>>>> +@opindex Wvector-operation-expanded
>>>>> +@opindex Wno-vector-operation-expanded
>>>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>>>> +architecture. Mainly useful for the performance tuning.
>>>>>
>>>>> I'd mention that this is for vector operations as of the C extension
>>>>> documented in "Vector Extensions".
>>>>>
>>>>> The vectorizer can produce some operations that will need further
>>>>> lowering - we probably should make sure to _not_ warn about those.
>>>>> Try running the vect.exp testsuite with the new warning turned on
>>>>> (eventually disabling SSE), like with
>>>>>
>>>>> obj/gcc> make check-gcc
>>>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>>>> vect.exp"
>>>>
>>>> Again, see the comment above. I think, if the warning can be triggered
>>>> only manually, then we are fine. But I'll check anyway how many
>>>> warnings I'll get from vect.exp.
>>>>
>>>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>>>> one needs to guess which architecture would expand a given vector
>>>>>> operation. But the patch is trivial.
>>>>>
>>>>> You can create an aritificial large vector type for example, or put a
>>>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>>>> a testcase for this.
>>>>
>>>> Yeah, disabling SSE should help.
>>>>
>>>>
>>>> Thanks,
>>>> Artem.
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>
>>>
>>
>> New version of the patch in the attachment with the test-cases.
>> Bootstrapped on  x86_64-apple-darwin10.8.0.
>> Currently is being tested.
>>
>>
>> Richard, I've checked the vect.exp case, as you suggested.  It caused
>> a lot of failures, but not because of the new warning.  The main
>> reason is -mno-sse.  The target is capable to vectorize, so the dg
>> option expects tests to pass, but the artificial option makes them
>> fail.  Checking the new warning on vect.exp without -mno-sse, it
>> didn't cause any new failures.  Anyway, we should be pretty much safe,
>> cause the warning is not a part of -O3.
>>
>> Thanks,
>> Artem.
>>
>
> Successfully regression-tested on x86_64-apple-darwin10.8.0.
>
> ChangeLog:
>        gcc/
>        * doc/invoke.texi: Document new warning.
>        * common.opt (Wvector-operation-performance): Define new warning.
>        * tree-vect-generic.c (expand_vector_piecewise): Warn about expanded
>        vector operation.
>        (exapnd_vector_parallel): Warn about expanded vector operation.
>        (lower_vec_shuffle): Warn about expanded vector operation.
>        * c-parser.c (c_parser_postfix_expression): Assign correct location
>        when creating VEC_SHUFFLE_EXPR.
>
>        gcc/testsuite/
>        * gcc.target/i386/warn-vect-op-3.c: New test.
>        * gcc.target/i386/warn-vect-op-1.c: New test.
>        * gcc.target/i386/warn-vect-op-2.c: New test.
>
> Ok for trunk?

+  if (gimple_expr_type (gsi_stmt (*gsi)) == type)
+    warning_at (loc, OPT_Wvector_operation_performance,
+               "vector operation will be expanded piecewise");
+  else
+    warning_at (loc, OPT_Wvector_operation_performance,
+               "vector operation will be expanded in parallel");

we should not check for exact type equivalence here.  Please
use types_compatible_p (gimple_expr_type (gsi_stmt (*gsi)), type)
instead.  We could also consider to pass down the kind of lowering
from the caller (or warn in the callers).

@@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
       compute_type = lang_hooks.types.type_for_mode (mode, 1);
       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
+      warning_at (loc, OPT_Wvector_operation_performance,
+                 "vector operation will be expanded with a "
+                 "single scalar operation");

That means it will be fast, no?  Why warn for it at all?

@@ -308,7 +320,7 @@ expand_vector_addition (gimple_stmt_iter
     return expand_vector_parallel (gsi, f_parallel,
                                   type, a, b, code);
   else
-    return expand_vector_piecewise (gsi, f,
+    return expand_vector_piecewise (gsi, f,
                                    type, TREE_TYPE (type),
                                    a, b, code);
 }

You add trailing space here ... (please review your patches yourself
for this kind of errors)

+             {
+               expr.value =
+                 c_build_vec_shuffle_expr
+                   (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
+                    NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
+               SET_EXPR_LOCATION (expr.value, loc);

That looks odd - see the 'loc' argument passed to c_build_vec_shuffle_expr.
If then that routine needs fixing.

Thanks,
Richard.

>
> Thanks,
> Artem.
>

Re: New warning for expanded vector operations

[Artem Shinkarov]

[-- Attachment #1: Type: text/plain, Size: 10498 bytes --]

On Mon, Oct 10, 2011 at 12:02 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Fri, Oct 7, 2011 at 9:44 AM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Fri, Oct 7, 2011 at 6:22 AM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>>> Hi
>>>>>>>
>>>>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>>>>
>>>>>>> ChangeLog:
>>>>>>>
>>>>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>>>>          produce the warning.
>>>>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>>>>
>>>>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>>>>
>>>>> Sure, sorry.
>>>>>
>>>>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>>>>
>>>>>>>
>>>>>>> Ok?
>>>>>>
>>>>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>>>>> similar warning for missed inline expansions with -Winline, so
>>>>>> maybe -Wvector-extensions (that's the name that appears
>>>>>> in the C extension documentation).
>>>>>
>>>>> Hm, I don't care much about the name, unless it gets clear what the
>>>>> warning is used for.  I am not really sure that Wvector-extensions
>>>>> makes it clear.  Also, I don't see anything bad if the warning will
>>>>> pop up during the vectorisation. Any vector operation performed
>>>>> outside the SIMD accelerator looks suspicious, because it actually
>>>>> doesn't improve performance.  Such a warning during the vectorisation
>>>>> could mean that a programmer forgot some flag, or the constant
>>>>> propagation failed to deliver a constant, or something else.
>>>>>
>>>>> Conceptually the text I am producing is not really a warning, it is
>>>>> more like an information, but I am not aware of the mechanisms that
>>>>> would allow me to introduce a flag triggering inform () or something
>>>>> similar.
>>>>>
>>>>> What I think we really need to avoid is including this warning in the
>>>>> standard Ox.
>>>>>
>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>> +
>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>> +             "vector operation will be expanded piecewise");
>>>>>>
>>>>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>>>>   for (i = 0; i < nunits;
>>>>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>>>>   tree result, compute_type;
>>>>>>   enum machine_mode mode;
>>>>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>> +
>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>> +             "vector operation will be expanded in parallel");
>>>>>>
>>>>>> what's the difference between 'piecewise' and 'in parallel'?
>>>>>
>>>>> Parallel is a little bit better for performance than piecewise.
>>>>
>>>> I see.  That difference should probably be documented, maybe with
>>>> an example.
>>>>
>>>> Richard.
>>>>
>>>>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>>>>  {
>>>>>>   int parts_per_word = UNITS_PER_WORD
>>>>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>
>>>>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>>>>       && parts_per_word >= 4
>>>>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>>>>> -    return expand_vector_parallel (gsi, f_parallel,
>>>>>> -                                  type, a, b, code);
>>>>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>>>>   else
>>>>>> -    return expand_vector_piecewise (gsi, f,
>>>>>> -                                   type, TREE_TYPE (type),
>>>>>> -                                   a, b, code);
>>>>>> +    return expand_vector_piecewise (gsi, f, type,
>>>>>> +                                   TREE_TYPE (type), a, b, code);
>>>>>>  }
>>>>>>
>>>>>>  /* Check if vector VEC consists of all the equal elements and
>>>>>>
>>>>>> unless i miss something loc is unused here.  Please avoid random
>>>>>> whitespace changes (just review your patch yourself before posting
>>>>>> and revert pieces that do nothing).
>>>>>
>>>>> Yes you are right, sorry.
>>>>>
>>>>>> +@item -Wvector-operation-expanded
>>>>>> +@opindex Wvector-operation-expanded
>>>>>> +@opindex Wno-vector-operation-expanded
>>>>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>>>>> +architecture. Mainly useful for the performance tuning.
>>>>>>
>>>>>> I'd mention that this is for vector operations as of the C extension
>>>>>> documented in "Vector Extensions".
>>>>>>
>>>>>> The vectorizer can produce some operations that will need further
>>>>>> lowering - we probably should make sure to _not_ warn about those.
>>>>>> Try running the vect.exp testsuite with the new warning turned on
>>>>>> (eventually disabling SSE), like with
>>>>>>
>>>>>> obj/gcc> make check-gcc
>>>>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>>>>> vect.exp"
>>>>>
>>>>> Again, see the comment above. I think, if the warning can be triggered
>>>>> only manually, then we are fine. But I'll check anyway how many
>>>>> warnings I'll get from vect.exp.
>>>>>
>>>>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>>>>> one needs to guess which architecture would expand a given vector
>>>>>>> operation. But the patch is trivial.
>>>>>>
>>>>>> You can create an aritificial large vector type for example, or put a
>>>>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>>>>> a testcase for this.
>>>>>
>>>>> Yeah, disabling SSE should help.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Artem.
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>
>>>>
>>>
>>> New version of the patch in the attachment with the test-cases.
>>> Bootstrapped on  x86_64-apple-darwin10.8.0.
>>> Currently is being tested.
>>>
>>>
>>> Richard, I've checked the vect.exp case, as you suggested.  It caused
>>> a lot of failures, but not because of the new warning.  The main
>>> reason is -mno-sse.  The target is capable to vectorize, so the dg
>>> option expects tests to pass, but the artificial option makes them
>>> fail.  Checking the new warning on vect.exp without -mno-sse, it
>>> didn't cause any new failures.  Anyway, we should be pretty much safe,
>>> cause the warning is not a part of -O3.
>>>
>>> Thanks,
>>> Artem.
>>>
>>
>> Successfully regression-tested on x86_64-apple-darwin10.8.0.
>>
>> ChangeLog:
>>        gcc/
>>        * doc/invoke.texi: Document new warning.
>>        * common.opt (Wvector-operation-performance): Define new warning.
>>        * tree-vect-generic.c (expand_vector_piecewise): Warn about expanded
>>        vector operation.
>>        (exapnd_vector_parallel): Warn about expanded vector operation.
>>        (lower_vec_shuffle): Warn about expanded vector operation.
>>        * c-parser.c (c_parser_postfix_expression): Assign correct location
>>        when creating VEC_SHUFFLE_EXPR.
>>
>>        gcc/testsuite/
>>        * gcc.target/i386/warn-vect-op-3.c: New test.
>>        * gcc.target/i386/warn-vect-op-1.c: New test.
>>        * gcc.target/i386/warn-vect-op-2.c: New test.
>>
>> Ok for trunk?
>
> +  if (gimple_expr_type (gsi_stmt (*gsi)) == type)
> +    warning_at (loc, OPT_Wvector_operation_performance,
> +               "vector operation will be expanded piecewise");
> +  else
> +    warning_at (loc, OPT_Wvector_operation_performance,
> +               "vector operation will be expanded in parallel");
>
> we should not check for exact type equivalence here.  Please
> use types_compatible_p (gimple_expr_type (gsi_stmt (*gsi)), type)
> instead.  We could also consider to pass down the kind of lowering
> from the caller (or warn in the callers).

Ok, Fixed.
>
> @@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
>       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
>       compute_type = lang_hooks.types.type_for_mode (mode, 1);
>       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
> +      warning_at (loc, OPT_Wvector_operation_performance,
> +                 "vector operation will be expanded with a "
> +                 "single scalar operation");
>
> That means it will be fast, no?  Why warn for it at all?

Most likely it means sower.  Eventually it is a different kind of the
expansion.  This situation could happen when you work with MMX
vectors, and by some reason instead of a single MMX operation, you
will have register operation + masking.
>
> @@ -308,7 +320,7 @@ expand_vector_addition (gimple_stmt_iter
>     return expand_vector_parallel (gsi, f_parallel,
>                                   type, a, b, code);
>   else
> -    return expand_vector_piecewise (gsi, f,
> +    return expand_vector_piecewise (gsi, f,
>                                    type, TREE_TYPE (type),
>                                    a, b, code);
>  }
>
> You add trailing space here ... (please review your patches yourself
> for this kind of errors)
>
> +             {
> +               expr.value =
> +                 c_build_vec_shuffle_expr
> +                   (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
> +                    NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
> +               SET_EXPR_LOCATION (expr.value, loc);
>
> That looks odd - see the 'loc' argument passed to c_build_vec_shuffle_expr.
> If then that routine needs fixing.

Ok, moved to c-typeck.c.


The new version is in the attachment.  Boostrapped on x86_64-apple-darwin10.8.0.
Ok?


Thanks,
Artem.


> Thanks,
> Richard.
>
>>
>> Thanks,
>> Artem.
>>
>

[-- Attachment #2: vector-op-warning-2.diff --]
[-- Type: application/octet-stream, Size: 8500 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 179744)
+++ gcc/doc/invoke.texi	(working copy)
@@ -271,7 +271,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
 -Wunused-but-set-parameter -Wunused-but-set-variable @gol
--Wvariadic-macros -Wvla -Wvolatile-register-var  -Wwrite-strings}
+-Wvariadic-macros -Wvector-operation-performance -Wvla 
+-Wvolatile-register-var  -Wwrite-strings}
 
 @item C and Objective-C-only Warning Options
 @gccoptlist{-Wbad-function-cast  -Wmissing-declarations @gol
@@ -4535,6 +4536,18 @@ Warn if variadic macros are used in peda
 alternate syntax when in pedantic ISO C99 mode.  This is default.
 To inhibit the warning messages, use @option{-Wno-variadic-macros}.
 
+@item -Wvector-operation-performance
+@opindex Wvector-operation-performance
+@opindex Wno-vector-operation-performance
+Warn if vector operation is not implemented via SIMD capabilities of the
+architecture.  Mainly useful for the performance tuning.
+Vector operation can be implemented @code{piecewise} which means that the
+scalar operation is performed on every vector element; 
+@code{in parallel} which means that the vector operation is implemented
+using scalars of wider type, which normally is more performance efficient;
+and @code{as a single scalar} which means that vector fits into a
+scalar type.
+
 @item -Wvla
 @opindex Wvla
 @opindex Wno-vla
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-3.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (8, short) v0 = {argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (8, short) v1 = {-4, argc, 2, 11, 1, 17, -8, argc};
+  vector (8, short) res[] = 
+  {
+    v0 + v1,	      /* { dg-warning "expanded in parallel" }  */
+    v0 - v1,          /* { dg-warning "expanded in parallel" }  */
+    v0 > v1,          /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,          /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),	      /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)    /* { dg-warning "expanded piecewise" }  */
+  };
+  
+  return res[argc][argc];
+}
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (4, int) v0 = {argc, 1, 15, 38};
+  vector (4, int) v1 = {-4, argc, 2, 11};
+  vector (4, int) res[] = 
+  {
+    v0 + v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 - v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 > v1,	  /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,	  /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),	    /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)  /* { dg-warning "expanded piecewise" }  */  
+  };
+
+  return res[argc][argc];
+}
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile }  */
+/* { dg-options "-mno-sse -Wvector-operation-performance" }  */
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+int main (int argc, char *argv[])
+{
+  vector (16, signed char) v0 = {argc, 1, 15, 38, 12, -1, argc, 2, 
+				 argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (16, signed char) v1 = {-4, argc, 2, 11, 1, 17, -8, argc,
+				 argc, 1, 15, 38, 12, -1, argc, 2};
+  vector (16, signed char) res[] = 
+  {
+    v0 + v1,		  /* { dg-warning "expanded in parallel" }  */
+    v0 - v1,              /* { dg-warning "expanded in parallel" }  */
+    v0 > v1,              /* { dg-warning "expanded piecewise" }  */
+    v0 & v1,              /* { dg-warning "expanded in parallel" }  */
+    __builtin_shuffle (v0, v1),        /* { dg-warning "expanded piecewise" }  */
+    __builtin_shuffle (v0, v1, v1)     /* { dg-warning "expanded piecewise" }  */
+  };
+ 
+  return res[argc][argc];
+}
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 179744)
+++ gcc/c-typeck.c	(working copy)
@@ -2934,7 +2934,8 @@ c_build_vec_perm_expr (location_t loc, t
 
   if (!wrap)
     ret = c_wrap_maybe_const (ret, true);
-
+  
+  SET_EXPR_LOCATION (ret, loc);
   return ret;
 }
 \f
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 179744)
+++ gcc/common.opt	(working copy)
@@ -694,6 +694,10 @@ Wcoverage-mismatch
 Common Var(warn_coverage_mismatch) Init(1) Warning
 Warn in case profiles in -fprofile-use do not match
 
+Wvector-operation-performance
+Common Var(warn_vector_operation_performance) Warning
+Warn when a vector operation is compiled outside the SIMD
+
 Xassembler
 Driver Separate
 
Index: gcc/tree-vect-generic.c
===================================================================
--- gcc/tree-vect-generic.c	(revision 179744)
+++ gcc/tree-vect-generic.c	(working copy)
@@ -235,6 +235,14 @@ expand_vector_piecewise (gimple_stmt_ite
   int delta = tree_low_cst (part_width, 1)
 	      / tree_low_cst (TYPE_SIZE (TREE_TYPE (type)), 1);
   int i;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
+
+  if (types_compatible_p (gimple_expr_type (gsi_stmt (*gsi)), type))
+    warning_at (loc, OPT_Wvector_operation_performance,
+		"vector operation will be expanded piecewise");
+  else
+    warning_at (loc, OPT_Wvector_operation_performance,
+		"vector operation will be expanded in parallel");
 
   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
   for (i = 0; i < nunits;
@@ -260,6 +268,7 @@ expand_vector_parallel (gimple_stmt_iter
   tree result, compute_type;
   enum machine_mode mode;
   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
+  location_t loc = gimple_location (gsi_stmt (*gsi));
 
   /* We have three strategies.  If the type is already correct, just do
      the operation an element at a time.  Else, if the vector is wider than
@@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
       compute_type = lang_hooks.types.type_for_mode (mode, 1);
       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
+      warning_at (loc, OPT_Wvector_operation_performance,
+	          "vector operation will be expanded with a "
+		  "single scalar operation");
     }
 
   return result;
@@ -400,8 +412,8 @@ expand_vector_operation (gimple_stmt_ite
       case PLUS_EXPR:
       case MINUS_EXPR:
         if (!TYPE_OVERFLOW_TRAPS (type))
-          return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
-		      		         gimple_assign_rhs1 (assign),
+	  return expand_vector_addition (gsi, do_binop, do_plus_minus, type,
+					 gimple_assign_rhs1 (assign),
 					 gimple_assign_rhs2 (assign), code);
 	break;
 
@@ -626,10 +638,15 @@ lower_vec_perm (gimple_stmt_iterator *gs
   tree constr, t, si, i_val;
   tree vec0tmp = NULL_TREE, vec1tmp = NULL_TREE, masktmp = NULL_TREE;
   bool two_operand_p = !operand_equal_p (vec0, vec1, 0);
+  location_t loc = gimple_location (gsi_stmt (*gsi));
   unsigned i;
 
   if (expand_vec_perm_expr_p (TYPE_MODE (vect_type), vec0, vec1, mask))
     return;
+  
+  warning_at (loc, OPT_Wvector_operation_performance,
+              "vector shuffling operation will be expanded piecewise");
+
 
   v = VEC_alloc (constructor_elt, gc, elements);
   for (i = 0; i < elements; i++)

Re: New warning for expanded vector operations

[Richard Guenther]

On Mon, Oct 10, 2011 at 3:21 PM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 12:02 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Fri, Oct 7, 2011 at 9:44 AM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> On Fri, Oct 7, 2011 at 6:22 AM, Artem Shinkarov
>>> <artyom.shinkaroff@gmail.com> wrote:
>>>> On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>>>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>>>>>
>>>>>>>> ChangeLog:
>>>>>>>>
>>>>>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>>>>>          produce the warning.
>>>>>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>>>>>
>>>>>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>>>>>
>>>>>> Sure, sorry.
>>>>>>
>>>>>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>>>>>
>>>>>>>>
>>>>>>>> Ok?
>>>>>>>
>>>>>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>>>>>> similar warning for missed inline expansions with -Winline, so
>>>>>>> maybe -Wvector-extensions (that's the name that appears
>>>>>>> in the C extension documentation).
>>>>>>
>>>>>> Hm, I don't care much about the name, unless it gets clear what the
>>>>>> warning is used for.  I am not really sure that Wvector-extensions
>>>>>> makes it clear.  Also, I don't see anything bad if the warning will
>>>>>> pop up during the vectorisation. Any vector operation performed
>>>>>> outside the SIMD accelerator looks suspicious, because it actually
>>>>>> doesn't improve performance.  Such a warning during the vectorisation
>>>>>> could mean that a programmer forgot some flag, or the constant
>>>>>> propagation failed to deliver a constant, or something else.
>>>>>>
>>>>>> Conceptually the text I am producing is not really a warning, it is
>>>>>> more like an information, but I am not aware of the mechanisms that
>>>>>> would allow me to introduce a flag triggering inform () or something
>>>>>> similar.
>>>>>>
>>>>>> What I think we really need to avoid is including this warning in the
>>>>>> standard Ox.
>>>>>>
>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>> +
>>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>>> +             "vector operation will be expanded piecewise");
>>>>>>>
>>>>>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>>>>>   for (i = 0; i < nunits;
>>>>>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>>>>>   tree result, compute_type;
>>>>>>>   enum machine_mode mode;
>>>>>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>> +
>>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>>> +             "vector operation will be expanded in parallel");
>>>>>>>
>>>>>>> what's the difference between 'piecewise' and 'in parallel'?
>>>>>>
>>>>>> Parallel is a little bit better for performance than piecewise.
>>>>>
>>>>> I see.  That difference should probably be documented, maybe with
>>>>> an example.
>>>>>
>>>>> Richard.
>>>>>
>>>>>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>>>>>  {
>>>>>>>   int parts_per_word = UNITS_PER_WORD
>>>>>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>>
>>>>>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>>>>>       && parts_per_word >= 4
>>>>>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>>>>>> -    return expand_vector_parallel (gsi, f_parallel,
>>>>>>> -                                  type, a, b, code);
>>>>>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>>>>>   else
>>>>>>> -    return expand_vector_piecewise (gsi, f,
>>>>>>> -                                   type, TREE_TYPE (type),
>>>>>>> -                                   a, b, code);
>>>>>>> +    return expand_vector_piecewise (gsi, f, type,
>>>>>>> +                                   TREE_TYPE (type), a, b, code);
>>>>>>>  }
>>>>>>>
>>>>>>>  /* Check if vector VEC consists of all the equal elements and
>>>>>>>
>>>>>>> unless i miss something loc is unused here.  Please avoid random
>>>>>>> whitespace changes (just review your patch yourself before posting
>>>>>>> and revert pieces that do nothing).
>>>>>>
>>>>>> Yes you are right, sorry.
>>>>>>
>>>>>>> +@item -Wvector-operation-expanded
>>>>>>> +@opindex Wvector-operation-expanded
>>>>>>> +@opindex Wno-vector-operation-expanded
>>>>>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>>>>>> +architecture. Mainly useful for the performance tuning.
>>>>>>>
>>>>>>> I'd mention that this is for vector operations as of the C extension
>>>>>>> documented in "Vector Extensions".
>>>>>>>
>>>>>>> The vectorizer can produce some operations that will need further
>>>>>>> lowering - we probably should make sure to _not_ warn about those.
>>>>>>> Try running the vect.exp testsuite with the new warning turned on
>>>>>>> (eventually disabling SSE), like with
>>>>>>>
>>>>>>> obj/gcc> make check-gcc
>>>>>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>>>>>> vect.exp"
>>>>>>
>>>>>> Again, see the comment above. I think, if the warning can be triggered
>>>>>> only manually, then we are fine. But I'll check anyway how many
>>>>>> warnings I'll get from vect.exp.
>>>>>>
>>>>>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>>>>>> one needs to guess which architecture would expand a given vector
>>>>>>>> operation. But the patch is trivial.
>>>>>>>
>>>>>>> You can create an aritificial large vector type for example, or put a
>>>>>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>>>>>> a testcase for this.
>>>>>>
>>>>>> Yeah, disabling SSE should help.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Artem.
>>>>>>> Thanks,
>>>>>>> Richard.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>> New version of the patch in the attachment with the test-cases.
>>>> Bootstrapped on  x86_64-apple-darwin10.8.0.
>>>> Currently is being tested.
>>>>
>>>>
>>>> Richard, I've checked the vect.exp case, as you suggested.  It caused
>>>> a lot of failures, but not because of the new warning.  The main
>>>> reason is -mno-sse.  The target is capable to vectorize, so the dg
>>>> option expects tests to pass, but the artificial option makes them
>>>> fail.  Checking the new warning on vect.exp without -mno-sse, it
>>>> didn't cause any new failures.  Anyway, we should be pretty much safe,
>>>> cause the warning is not a part of -O3.
>>>>
>>>> Thanks,
>>>> Artem.
>>>>
>>>
>>> Successfully regression-tested on x86_64-apple-darwin10.8.0.
>>>
>>> ChangeLog:
>>>        gcc/
>>>        * doc/invoke.texi: Document new warning.
>>>        * common.opt (Wvector-operation-performance): Define new warning.
>>>        * tree-vect-generic.c (expand_vector_piecewise): Warn about expanded
>>>        vector operation.
>>>        (exapnd_vector_parallel): Warn about expanded vector operation.
>>>        (lower_vec_shuffle): Warn about expanded vector operation.
>>>        * c-parser.c (c_parser_postfix_expression): Assign correct location
>>>        when creating VEC_SHUFFLE_EXPR.
>>>
>>>        gcc/testsuite/
>>>        * gcc.target/i386/warn-vect-op-3.c: New test.
>>>        * gcc.target/i386/warn-vect-op-1.c: New test.
>>>        * gcc.target/i386/warn-vect-op-2.c: New test.
>>>
>>> Ok for trunk?
>>
>> +  if (gimple_expr_type (gsi_stmt (*gsi)) == type)
>> +    warning_at (loc, OPT_Wvector_operation_performance,
>> +               "vector operation will be expanded piecewise");
>> +  else
>> +    warning_at (loc, OPT_Wvector_operation_performance,
>> +               "vector operation will be expanded in parallel");
>>
>> we should not check for exact type equivalence here.  Please
>> use types_compatible_p (gimple_expr_type (gsi_stmt (*gsi)), type)
>> instead.  We could also consider to pass down the kind of lowering
>> from the caller (or warn in the callers).
>
> Ok, Fixed.
>>
>> @@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
>>       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
>>       compute_type = lang_hooks.types.type_for_mode (mode, 1);
>>       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
>> +      warning_at (loc, OPT_Wvector_operation_performance,
>> +                 "vector operation will be expanded with a "
>> +                 "single scalar operation");
>>
>> That means it will be fast, no?  Why warn for it at all?
>
> Most likely it means sower.  Eventually it is a different kind of the
> expansion.  This situation could happen when you work with MMX
> vectors, and by some reason instead of a single MMX operation, you
> will have register operation + masking.
>>
>> @@ -308,7 +320,7 @@ expand_vector_addition (gimple_stmt_iter
>>     return expand_vector_parallel (gsi, f_parallel,
>>                                   type, a, b, code);
>>   else
>> -    return expand_vector_piecewise (gsi, f,
>> +    return expand_vector_piecewise (gsi, f,
>>                                    type, TREE_TYPE (type),
>>                                    a, b, code);
>>  }
>>
>> You add trailing space here ... (please review your patches yourself
>> for this kind of errors)
>>
>> +             {
>> +               expr.value =
>> +                 c_build_vec_shuffle_expr
>> +                   (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
>> +                    NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
>> +               SET_EXPR_LOCATION (expr.value, loc);
>>
>> That looks odd - see the 'loc' argument passed to c_build_vec_shuffle_expr.
>> If then that routine needs fixing.
>
> Ok, moved to c-typeck.c.
>
>
> The new version is in the attachment.  Boostrapped on x86_64-apple-darwin10.8.0.
> Ok?

Ok with

@@ -2934,7 +2934,8 @@ c_build_vec_perm_expr (location_t loc, t

   if (!wrap)
     ret = c_wrap_maybe_const (ret, true);
-
+
+  SET_EXPR_LOCATION (ret, loc);
   return ret;

instead of this use build3_loc (loc, ...) when building the VEC_PERM_EXPR
in the line before this hunk.

Thanks,
Richard.

>
> Thanks,
> Artem.
>
>
>> Thanks,
>> Richard.
>>
>>>
>>> Thanks,
>>> Artem.
>>>
>>
>

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Tue, Oct 11, 2011 at 11:52 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 3:21 PM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Mon, Oct 10, 2011 at 12:02 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Fri, Oct 7, 2011 at 9:44 AM, Artem Shinkarov
>>> <artyom.shinkaroff@gmail.com> wrote:
>>>> On Fri, Oct 7, 2011 at 6:22 AM, Artem Shinkarov
>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>> On Wed, Oct 5, 2011 at 12:35 PM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Wed, Oct 5, 2011 at 1:28 PM, Artem Shinkarov
>>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>>> On Wed, Oct 5, 2011 at 9:40 AM, Richard Guenther
>>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>>> On Wed, Oct 5, 2011 at 12:18 AM, Artem Shinkarov
>>>>>>>> <artyom.shinkaroff@gmail.com> wrote:
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> Here is a patch to inform a programmer about the expanded vector operation.
>>>>>>>>> Bootstrapped on x86-unknown-linux-gnu.
>>>>>>>>>
>>>>>>>>> ChangeLog:
>>>>>>>>>
>>>>>>>>>        * gcc/tree-vect-generic.c (expand_vector_piecewise): Adjust to
>>>>>>>>>          produce the warning.
>>>>>>>>>          (expand_vector_parallel): Adjust to produce the warning.
>>>>>>>>
>>>>>>>> Entries start without gcc/, they are relative to the gcc/ChangeLog file.
>>>>>>>
>>>>>>> Sure, sorry.
>>>>>>>
>>>>>>>>>          (lower_vec_shuffle): Adjust to produce the warning.
>>>>>>>>>        * gcc/common.opt: New warning Wvector-operation-expanded.
>>>>>>>>>        * gcc/doc/invoke.texi: Document the wawning.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Ok?
>>>>>>>>
>>>>>>>> I don't like the name -Wvector-operation-expanded.  We emit a
>>>>>>>> similar warning for missed inline expansions with -Winline, so
>>>>>>>> maybe -Wvector-extensions (that's the name that appears
>>>>>>>> in the C extension documentation).
>>>>>>>
>>>>>>> Hm, I don't care much about the name, unless it gets clear what the
>>>>>>> warning is used for.  I am not really sure that Wvector-extensions
>>>>>>> makes it clear.  Also, I don't see anything bad if the warning will
>>>>>>> pop up during the vectorisation. Any vector operation performed
>>>>>>> outside the SIMD accelerator looks suspicious, because it actually
>>>>>>> doesn't improve performance.  Such a warning during the vectorisation
>>>>>>> could mean that a programmer forgot some flag, or the constant
>>>>>>> propagation failed to deliver a constant, or something else.
>>>>>>>
>>>>>>> Conceptually the text I am producing is not really a warning, it is
>>>>>>> more like an information, but I am not aware of the mechanisms that
>>>>>>> would allow me to introduce a flag triggering inform () or something
>>>>>>> similar.
>>>>>>>
>>>>>>> What I think we really need to avoid is including this warning in the
>>>>>>> standard Ox.
>>>>>>>
>>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>>> +
>>>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>>>> +             "vector operation will be expanded piecewise");
>>>>>>>>
>>>>>>>>   v = VEC_alloc(constructor_elt, gc, (nunits + delta - 1) / delta);
>>>>>>>>   for (i = 0; i < nunits;
>>>>>>>> @@ -260,6 +264,10 @@ expand_vector_parallel (gimple_stmt_iter
>>>>>>>>   tree result, compute_type;
>>>>>>>>   enum machine_mode mode;
>>>>>>>>   int n_words = tree_low_cst (TYPE_SIZE_UNIT (type), 1) / UNITS_PER_WORD;
>>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>>> +
>>>>>>>> +  warning_at (loc, OPT_Wvector_operation_expanded,
>>>>>>>> +             "vector operation will be expanded in parallel");
>>>>>>>>
>>>>>>>> what's the difference between 'piecewise' and 'in parallel'?
>>>>>>>
>>>>>>> Parallel is a little bit better for performance than piecewise.
>>>>>>
>>>>>> I see.  That difference should probably be documented, maybe with
>>>>>> an example.
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>>> @@ -301,16 +309,15 @@ expand_vector_addition (gimple_stmt_iter
>>>>>>>>  {
>>>>>>>>   int parts_per_word = UNITS_PER_WORD
>>>>>>>>                       / tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type)), 1);
>>>>>>>> +  location_t loc = gimple_location (gsi_stmt (*gsi));
>>>>>>>>
>>>>>>>>   if (INTEGRAL_TYPE_P (TREE_TYPE (type))
>>>>>>>>       && parts_per_word >= 4
>>>>>>>>       && TYPE_VECTOR_SUBPARTS (type) >= 4)
>>>>>>>> -    return expand_vector_parallel (gsi, f_parallel,
>>>>>>>> -                                  type, a, b, code);
>>>>>>>> +    return expand_vector_parallel (gsi, f_parallel, type, a, b, code);
>>>>>>>>   else
>>>>>>>> -    return expand_vector_piecewise (gsi, f,
>>>>>>>> -                                   type, TREE_TYPE (type),
>>>>>>>> -                                   a, b, code);
>>>>>>>> +    return expand_vector_piecewise (gsi, f, type,
>>>>>>>> +                                   TREE_TYPE (type), a, b, code);
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  /* Check if vector VEC consists of all the equal elements and
>>>>>>>>
>>>>>>>> unless i miss something loc is unused here.  Please avoid random
>>>>>>>> whitespace changes (just review your patch yourself before posting
>>>>>>>> and revert pieces that do nothing).
>>>>>>>
>>>>>>> Yes you are right, sorry.
>>>>>>>
>>>>>>>> +@item -Wvector-operation-expanded
>>>>>>>> +@opindex Wvector-operation-expanded
>>>>>>>> +@opindex Wno-vector-operation-expanded
>>>>>>>> +Warn if vector operation is not implemented via SIMD capabilities of the
>>>>>>>> +architecture. Mainly useful for the performance tuning.
>>>>>>>>
>>>>>>>> I'd mention that this is for vector operations as of the C extension
>>>>>>>> documented in "Vector Extensions".
>>>>>>>>
>>>>>>>> The vectorizer can produce some operations that will need further
>>>>>>>> lowering - we probably should make sure to _not_ warn about those.
>>>>>>>> Try running the vect.exp testsuite with the new warning turned on
>>>>>>>> (eventually disabling SSE), like with
>>>>>>>>
>>>>>>>> obj/gcc> make check-gcc
>>>>>>>> RUNTESTFLAGS="--target_board=unix/-Wvector-extensions/-mno-sse
>>>>>>>> vect.exp"
>>>>>>>
>>>>>>> Again, see the comment above. I think, if the warning can be triggered
>>>>>>> only manually, then we are fine. But I'll check anyway how many
>>>>>>> warnings I'll get from vect.exp.
>>>>>>>
>>>>>>>>> P.S. It is hard to write a reasonable testcase for the patch, because
>>>>>>>>> one needs to guess which architecture would expand a given vector
>>>>>>>>> operation. But the patch is trivial.
>>>>>>>>
>>>>>>>> You can create an aritificial large vector type for example, or put a
>>>>>>>> testcase under gcc.target/i386 and disable SSE.  We should have
>>>>>>>> a testcase for this.
>>>>>>>
>>>>>>> Yeah, disabling SSE should help.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Artem.
>>>>>>>> Thanks,
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> New version of the patch in the attachment with the test-cases.
>>>>> Bootstrapped on  x86_64-apple-darwin10.8.0.
>>>>> Currently is being tested.
>>>>>
>>>>>
>>>>> Richard, I've checked the vect.exp case, as you suggested.  It caused
>>>>> a lot of failures, but not because of the new warning.  The main
>>>>> reason is -mno-sse.  The target is capable to vectorize, so the dg
>>>>> option expects tests to pass, but the artificial option makes them
>>>>> fail.  Checking the new warning on vect.exp without -mno-sse, it
>>>>> didn't cause any new failures.  Anyway, we should be pretty much safe,
>>>>> cause the warning is not a part of -O3.
>>>>>
>>>>> Thanks,
>>>>> Artem.
>>>>>
>>>>
>>>> Successfully regression-tested on x86_64-apple-darwin10.8.0.
>>>>
>>>> ChangeLog:
>>>>        gcc/
>>>>        * doc/invoke.texi: Document new warning.
>>>>        * common.opt (Wvector-operation-performance): Define new warning.
>>>>        * tree-vect-generic.c (expand_vector_piecewise): Warn about expanded
>>>>        vector operation.
>>>>        (exapnd_vector_parallel): Warn about expanded vector operation.
>>>>        (lower_vec_shuffle): Warn about expanded vector operation.
>>>>        * c-parser.c (c_parser_postfix_expression): Assign correct location
>>>>        when creating VEC_SHUFFLE_EXPR.
>>>>
>>>>        gcc/testsuite/
>>>>        * gcc.target/i386/warn-vect-op-3.c: New test.
>>>>        * gcc.target/i386/warn-vect-op-1.c: New test.
>>>>        * gcc.target/i386/warn-vect-op-2.c: New test.
>>>>
>>>> Ok for trunk?
>>>
>>> +  if (gimple_expr_type (gsi_stmt (*gsi)) == type)
>>> +    warning_at (loc, OPT_Wvector_operation_performance,
>>> +               "vector operation will be expanded piecewise");
>>> +  else
>>> +    warning_at (loc, OPT_Wvector_operation_performance,
>>> +               "vector operation will be expanded in parallel");
>>>
>>> we should not check for exact type equivalence here.  Please
>>> use types_compatible_p (gimple_expr_type (gsi_stmt (*gsi)), type)
>>> instead.  We could also consider to pass down the kind of lowering
>>> from the caller (or warn in the callers).
>>
>> Ok, Fixed.
>>>
>>> @@ -284,6 +293,9 @@ expand_vector_parallel (gimple_stmt_iter
>>>       mode = mode_for_size (tree_low_cst (TYPE_SIZE (type), 1), MODE_INT, 0);
>>>       compute_type = lang_hooks.types.type_for_mode (mode, 1);
>>>       result = f (gsi, compute_type, a, b, NULL_TREE, NULL_TREE, code);
>>> +      warning_at (loc, OPT_Wvector_operation_performance,
>>> +                 "vector operation will be expanded with a "
>>> +                 "single scalar operation");
>>>
>>> That means it will be fast, no?  Why warn for it at all?
>>
>> Most likely it means sower.  Eventually it is a different kind of the
>> expansion.  This situation could happen when you work with MMX
>> vectors, and by some reason instead of a single MMX operation, you
>> will have register operation + masking.
>>>
>>> @@ -308,7 +320,7 @@ expand_vector_addition (gimple_stmt_iter
>>>     return expand_vector_parallel (gsi, f_parallel,
>>>                                   type, a, b, code);
>>>   else
>>> -    return expand_vector_piecewise (gsi, f,
>>> +    return expand_vector_piecewise (gsi, f,
>>>                                    type, TREE_TYPE (type),
>>>                                    a, b, code);
>>>  }
>>>
>>> You add trailing space here ... (please review your patches yourself
>>> for this kind of errors)
>>>
>>> +             {
>>> +               expr.value =
>>> +                 c_build_vec_shuffle_expr
>>> +                   (loc, VEC_index (c_expr_t, cexpr_list, 0)->value,
>>> +                    NULL_TREE, VEC_index (c_expr_t, cexpr_list, 1)->value);
>>> +               SET_EXPR_LOCATION (expr.value, loc);
>>>
>>> That looks odd - see the 'loc' argument passed to c_build_vec_shuffle_expr.
>>> If then that routine needs fixing.
>>
>> Ok, moved to c-typeck.c.
>>
>>
>> The new version is in the attachment.  Boostrapped on x86_64-apple-darwin10.8.0.
>> Ok?
>
> Ok with
>
> @@ -2934,7 +2934,8 @@ c_build_vec_perm_expr (location_t loc, t
>
>   if (!wrap)
>     ret = c_wrap_maybe_const (ret, true);
> -
> +
> +  SET_EXPR_LOCATION (ret, loc);
>   return ret;
>
> instead of this use build3_loc (loc, ...) when building the VEC_PERM_EXPR
> in the line before this hunk.
>
> Thanks,
> Richard.
>
>>
>> Thanks,
>> Artem.
>>
>>
>>> Thanks,
>>> Richard.
>>>
>>>>
>>>> Thanks,
>>>> Artem.
>>>>
>>>
>>
>

Committed with the revision 179807.


Artem.

Re: New warning for expanded vector operations

[H.J. Lu]

On Tue, Oct 11, 2011 at 9:11 AM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
>
> Committed with the revision 179807.
>
>

This caused:

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

-- 
H.J.

Re: New warning for expanded vector operations

[Artem Shinkarov]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

This patch fixed PR50704.

gcc/testsuite:
        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
        * gcc.target/i386/warn-vect-op-1.c: Ditto.
        * gcc.target/i386/warn-vect-op-2.c: Ditto.

Ok for trunk?

Artem.

On Wed, Oct 12, 2011 at 4:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Oct 11, 2011 at 9:11 AM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>>
>> Committed with the revision 179807.
>>
>>
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50704
>
> --
> H.J.
>

[-- Attachment #2: fix-performance-tests.diff --]
[-- Type: application/octet-stream, Size: 1527 bytes --]

Index: gcc/testsuite/gcc.target/i386/warn-vect-op-3.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(revision 179807)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-3.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile }  */
+/* { dg-do compile { target { ! { ia32 } } } }  */
 /* { dg-options "-mno-sse -Wvector-operation-performance" }  */
 #define vector(elcount, type)  \
 __attribute__((vector_size((elcount)*sizeof(type)))) type
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(revision 179807)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-1.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile }  */
+/* { dg-do compile { target { ! { ia32 } } } }  */
 /* { dg-options "-mno-sse -Wvector-operation-performance" }  */
 #define vector(elcount, type)  \
 __attribute__((vector_size((elcount)*sizeof(type)))) type
Index: gcc/testsuite/gcc.target/i386/warn-vect-op-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(revision 179807)
+++ gcc/testsuite/gcc.target/i386/warn-vect-op-2.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile }  */
+/* { dg-do compile { target { ! { ia32 } } } }  */
 /* { dg-options "-mno-sse -Wvector-operation-performance" }  */
 #define vector(elcount, type)  \
 __attribute__((vector_size((elcount)*sizeof(type)))) type

Re: New warning for expanded vector operations

[Mike Stump]

On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
> This patch fixed PR50704.
> 
> gcc/testsuite:
>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
> 
> Ok for trunk?

Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.

Re: New warning for expanded vector operations

[Richard Guenther]

On Thu, Oct 13, 2011 at 10:59 AM, Mike Stump <mikestump@comcast.net> wrote:
> On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
>> This patch fixed PR50704.
>>
>> gcc/testsuite:
>>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
>>
>> Ok for trunk?
>
> Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.

I suppose you instead want sth like

{ dg-require-effective-target lp64 }

?

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Thu, Oct 13, 2011 at 10:23 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 10:59 AM, Mike Stump <mikestump@comcast.net> wrote:
>> On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
>>> This patch fixed PR50704.
>>>
>>> gcc/testsuite:
>>>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>>>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>>>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
>>>
>>> Ok for trunk?
>>
>> Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.
>
> I suppose you instead want sth like
>
> { dg-require-effective-target lp64 }
>
> ?
>

See our discussion with HJ here:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50704
/* { dg-do compile { target { ! { ia32 } } } } */ was his idea.  As
far as x32 sets UNITS_PER_WORD to 8, these tests should work fine.

Artem.

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Thu, Oct 13, 2011 at 10:40 AM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 10:23 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, Oct 13, 2011 at 10:59 AM, Mike Stump <mikestump@comcast.net> wrote:
>>> On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
>>>> This patch fixed PR50704.
>>>>
>>>> gcc/testsuite:
>>>>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>>>>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>>>>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
>>>>
>>>> Ok for trunk?
>>>
>>> Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.
>>
>> I suppose you instead want sth like
>>
>> { dg-require-effective-target lp64 }
>>
>> ?
>>
>
> See our discussion with HJ here:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50704
> /* { dg-do compile { target { ! { ia32 } } } } */ was his idea.  As
> far as x32 sets UNITS_PER_WORD to 8, these tests should work fine.
>
> Artem.
>

Ping.

So can I commit the changes?


Thanks,
Artem.

Re: New warning for expanded vector operations

[Richard Guenther]

On Fri, Oct 14, 2011 at 3:42 PM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 10:40 AM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Thu, Oct 13, 2011 at 10:23 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Thu, Oct 13, 2011 at 10:59 AM, Mike Stump <mikestump@comcast.net> wrote:
>>>> On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
>>>>> This patch fixed PR50704.
>>>>>
>>>>> gcc/testsuite:
>>>>>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>>>>>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>>>>>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
>>>>>
>>>>> Ok for trunk?
>>>>
>>>> Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.
>>>
>>> I suppose you instead want sth like
>>>
>>> { dg-require-effective-target lp64 }
>>>
>>> ?
>>>
>>
>> See our discussion with HJ here:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50704
>> /* { dg-do compile { target { ! { ia32 } } } } */ was his idea.  As
>> far as x32 sets UNITS_PER_WORD to 8, these tests should work fine.
>>
>> Artem.
>>
>
> Ping.
>
> So can I commit the changes?

Yes.

Thanks,
Richard.

>
> Thanks,
> Artem.
>

Re: New warning for expanded vector operations

[Artem Shinkarov]

On Fri, Oct 14, 2011 at 2:57 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Fri, Oct 14, 2011 at 3:42 PM, Artem Shinkarov
> <artyom.shinkaroff@gmail.com> wrote:
>> On Thu, Oct 13, 2011 at 10:40 AM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> On Thu, Oct 13, 2011 at 10:23 AM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Thu, Oct 13, 2011 at 10:59 AM, Mike Stump <mikestump@comcast.net> wrote:
>>>>> On Oct 12, 2011, at 2:37 PM, Artem Shinkarov wrote:
>>>>>> This patch fixed PR50704.
>>>>>>
>>>>>> gcc/testsuite:
>>>>>>        * gcc.target/i386/warn-vect-op-3.c: Exclude ia32 target.
>>>>>>        * gcc.target/i386/warn-vect-op-1.c: Ditto.
>>>>>>        * gcc.target/i386/warn-vect-op-2.c: Ditto.
>>>>>>
>>>>>> Ok for trunk?
>>>>>
>>>>> Ok.  Is this x32 clean?  :-)  If not, HJ will offer an even better spelling.
>>>>
>>>> I suppose you instead want sth like
>>>>
>>>> { dg-require-effective-target lp64 }
>>>>
>>>> ?
>>>>
>>>
>>> See our discussion with HJ here:
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50704
>>> /* { dg-do compile { target { ! { ia32 } } } } */ was his idea.  As
>>> far as x32 sets UNITS_PER_WORD to 8, these tests should work fine.
>>>
>>> Artem.
>>>
>>
>> Ping.
>>
>> So can I commit the changes?
>
> Yes.
>
> Thanks,
> Richard.

Committed with 179991.

>>
>> Thanks,
>> Artem.
>>
>

Re: New warning for expanded vector operations

[Mike Stump]

On Oct 14, 2011, at 8:37 AM, Artem Shinkarov wrote:
> Committed with 179991.

Please don't send these...  If you commit for a person, you can send directly to them the fact you committed the work. If people want to know when works goes in, be sure to use a PR and put yourself on the cc list, then, you will get the email it was committed from the version control system, after it hits spinning disk.  Thanks.