* New warning for expanded vector operations
@ 2011-10-04 22:25 Artem Shinkarov
2011-10-05 8:40 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-04 22:25 UTC (permalink / raw)
To: GCC Patches
[-- 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-04 22:25 New warning for expanded vector operations Artem Shinkarov
@ 2011-10-05 8:40 ` Richard Guenther
2011-10-05 11:31 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-05 8:40 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-05 8:40 ` Richard Guenther
@ 2011-10-05 11:31 ` Artem Shinkarov
2011-10-05 11:37 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-05 11:31 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
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.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-05 11:31 ` Artem Shinkarov
@ 2011-10-05 11:37 ` Richard Guenther
2011-10-07 7:13 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-05 11:37 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: GCC Patches
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.
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-05 11:37 ` Richard Guenther
@ 2011-10-07 7:13 ` Artem Shinkarov
2011-10-07 8:01 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-07 7:13 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
[-- 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 "
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-07 7:13 ` Artem Shinkarov
@ 2011-10-07 8:01 ` Artem Shinkarov
2011-10-10 11:15 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-07 8:01 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-07 8:01 ` Artem Shinkarov
@ 2011-10-10 11:15 ` Richard Guenther
2011-10-10 13:27 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-10 11:15 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: GCC Patches
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.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-10 11:15 ` Richard Guenther
@ 2011-10-10 13:27 ` Artem Shinkarov
2011-10-11 11:40 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-10 13:27 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
[-- 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++)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-10 13:27 ` Artem Shinkarov
@ 2011-10-11 11:40 ` Richard Guenther
2011-10-11 17:26 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-11 11:40 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: GCC Patches
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.
>>>
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-11 11:40 ` Richard Guenther
@ 2011-10-11 17:26 ` Artem Shinkarov
2011-10-12 16:25 ` H.J. Lu
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-11 17:26 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-11 17:26 ` Artem Shinkarov
@ 2011-10-12 16:25 ` H.J. Lu
2011-10-12 22:10 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2011-10-12 16:25 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: Richard Guenther, GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-12 16:25 ` H.J. Lu
@ 2011-10-12 22:10 ` Artem Shinkarov
2011-10-13 9:32 ` Mike Stump
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-12 22:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Guenther, GCC Patches
[-- 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-12 22:10 ` Artem Shinkarov
@ 2011-10-13 9:32 ` Mike Stump
2011-10-13 10:17 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Mike Stump @ 2011-10-13 9:32 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: H.J. Lu, Richard Guenther, GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-13 9:32 ` Mike Stump
@ 2011-10-13 10:17 ` Richard Guenther
2011-10-13 10:18 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-13 10:17 UTC (permalink / raw)
To: Mike Stump; +Cc: Artem Shinkarov, H.J. Lu, GCC Patches
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 }
?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-13 10:17 ` Richard Guenther
@ 2011-10-13 10:18 ` Artem Shinkarov
2011-10-14 14:02 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-13 10:18 UTC (permalink / raw)
To: Richard Guenther; +Cc: Mike Stump, H.J. Lu, GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-13 10:18 ` Artem Shinkarov
@ 2011-10-14 14:02 ` Artem Shinkarov
2011-10-14 14:15 ` Richard Guenther
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-14 14:02 UTC (permalink / raw)
To: Richard Guenther; +Cc: Mike Stump, H.J. Lu, GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-14 14:02 ` Artem Shinkarov
@ 2011-10-14 14:15 ` Richard Guenther
2011-10-14 16:05 ` Artem Shinkarov
0 siblings, 1 reply; 19+ messages in thread
From: Richard Guenther @ 2011-10-14 14:15 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: Mike Stump, H.J. Lu, GCC Patches
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.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-14 14:15 ` Richard Guenther
@ 2011-10-14 16:05 ` Artem Shinkarov
2011-10-14 18:31 ` Mike Stump
0 siblings, 1 reply; 19+ messages in thread
From: Artem Shinkarov @ 2011-10-14 16:05 UTC (permalink / raw)
To: Richard Guenther; +Cc: Mike Stump, H.J. Lu, GCC Patches
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.
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: New warning for expanded vector operations
2011-10-14 16:05 ` Artem Shinkarov
@ 2011-10-14 18:31 ` Mike Stump
0 siblings, 0 replies; 19+ messages in thread
From: Mike Stump @ 2011-10-14 18:31 UTC (permalink / raw)
To: Artem Shinkarov; +Cc: Richard Guenther, H.J. Lu, GCC Patches
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-10-14 18:03 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-04 22:25 New warning for expanded vector operations Artem Shinkarov
2011-10-05 8:40 ` Richard Guenther
2011-10-05 11:31 ` Artem Shinkarov
2011-10-05 11:37 ` Richard Guenther
2011-10-07 7:13 ` Artem Shinkarov
2011-10-07 8:01 ` Artem Shinkarov
2011-10-10 11:15 ` Richard Guenther
2011-10-10 13:27 ` Artem Shinkarov
2011-10-11 11:40 ` Richard Guenther
2011-10-11 17:26 ` Artem Shinkarov
2011-10-12 16:25 ` H.J. Lu
2011-10-12 22:10 ` Artem Shinkarov
2011-10-13 9:32 ` Mike Stump
2011-10-13 10:17 ` Richard Guenther
2011-10-13 10:18 ` Artem Shinkarov
2011-10-14 14:02 ` Artem Shinkarov
2011-10-14 14:15 ` Richard Guenther
2011-10-14 16:05 ` Artem Shinkarov
2011-10-14 18:31 ` Mike Stump
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).