Hi Richard,
Thanks for the review!
on 2020/8/6 下午1:52, Richard Sandiford wrote:
> "Kewen.Lin" <li...@linux.ibm.com> writes:
>> diff --git a/gcc/testsuite/gcc.dg/vect/slp-multitypes-11.c
>> b/gcc/testsuite/gcc.dg/vect/slp-multitypes-11.c
>> index 5200ed1cd94..da6fb12eb0d 100644
>> --- a/gcc/testsuite/gcc.dg/vect/slp-multitypes-11.c
>> +++ b/gcc/testsuite/gcc.dg/vect/slp-multitypes-11.c
>> @@ -48,6 +48,9 @@ int main (void)
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
>> target vect_unpack } } } */
>> -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect"
>> { target vect_unpack xfail { vect_variable_length && vect_load_lanes } } }
>> } */
>> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
>> target { vect_unpack && {! vect_partial_vectors_usage_1 } } } } } */
>> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
>> target { vect_unpack && { vect_partial_vectors_usage_1 } } } } } */
>
> I don't understand this bit: don't these two lines reduce back to the
> original vect_unpack one?
Yes, we don't need them. Sorry that I duplicated it for the different
conditions like the one after it,
but forgot to change it back when found it's useless. Thanks for catching!
>
>> +/* The epilogues are vectorized using partial vectors. */
>> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect"
>> { target { vect_unpack && {! vect_partial_vectors_usage_1 } } xfail {
>> vect_variable_length && vect_load_lanes } } } } */
>> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect"
>> { target { vect_unpack && { vect_partial_vectors_usage_1 } } xfail {
>> vect_variable_length && vect_load_lanes } } } } */
>>
>> diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
>> b/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
>> index ca7803ec1a9..af6fe08856f 100644
>> --- a/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
>> +++ b/gcc/testsuite/gcc.dg/vect/slp-perm-1.c
>> @@ -80,8 +80,10 @@ int main (int argc, const char* argv[])
>> }
>>
>> /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
>> target vect_perm } } } */
>> -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect"
>> { target { vect_perm3_int && {! vect_load_lanes } } } } } */
>> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect"
>> { target { vect_perm3_int && { {! vect_load_lanes } && {!
>> vect_partial_vectors_usage_1 } } } } } } */
>> /* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect"
>> { target vect_load_lanes } } } */
>> +/* The epilogues are vectorized using partial vectors. */
>> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect"
>> { target { vect_perm3_int && { {! vect_load_lanes } && {
>> vect_partial_vectors_usage_1 } } } } } } */
>
> Very minor, but I think it'd be better to put this immediately after the
> line you changed above. Same for the other slp-perm-* changes.
>
OK, will fix.
>> diff --git a/gcc/testsuite/lib/target-supports.exp
>> b/gcc/testsuite/lib/target-supports.exp
>> index 57eed3012b9..b571e84d20e 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -7055,6 +7055,27 @@ proc check_effective_target_vect_check_ptrs { } {
>> return [check_effective_target_aarch64_sve2]
>> }
>>
>> +# Return true if loops using partial vectors are supported.
>> +
>> +proc check_effective_target_vect_partial_vectors { } {
>> + return [expr { [check_effective_target_vect_partial_vectors_usage_1]
>> + || [check_effective_target_vect_partial_vectors_usage_2] }]
>> +}
>> +
>> +# Return true if loops using partial vectors are supported and the default
>> +# value of --param=vect-partial-vector-usage is 1.
>> +
>> +proc check_effective_target_vect_partial_vectors_usage_1 { } {
>> + return 0
>> +}
>> +
>> +# Return true if loops using partial vectors are supported and the default
>> +# value of --param=vect-partial-vector-usage is 2.
>> +
>> +proc check_effective_target_vect_partial_vectors_usage_2 { } {
>> + return [expr { [check_effective_target_vect_fully_masked] }]
>> +}
>> +
>
> Could we auto-detect this? What we really care about isn't the default,
> but what's currently being tested.
Yeah, the comments were confusing, its intent is to check which targets
support partial vectors and which usage to be used.
How about to update them like:
"Return true if loops using partial vectors are supported and usage kind is
1/2".
>
> E.g. maybe use check_compile to run gcc with “-Q --help=params” and an
> arbitrary output type (probably assembly). Then use “regexp” on the
> lines to parse the --param=vect-partial-vector-usage value. At that
> point it would be worth caching the result.
Now the default value of this parameter is 2, even for those targets which
don't have the supports with partial vectors. Since we will get the value
2 on those unsupported targets, it looks like we have to set it manually?
>
> The new target-supports keywords need to be documented in sourcebuild.texi.
>
Will add it, thanks for the information!
BR,
Kewen
