| Message ID | 20170926113950.5472-5-krebbel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | vect testsuite adjustments for S/390 | expand |
- vect_nopeel renamed to vect_no_peel
- documentation added.
gcc/testsuite/ChangeLog:
2017-09-26 Andreas Krebbel <krebbel@linux.vnet.ibm.com>
* doc/sourcebuild.texi: Document vect_no_peel.
gcc/testsuite/ChangeLog:
2017-09-26 Andreas Krebbel <krebbel@linux.vnet.ibm.com>
* g++.dg/vect/slp-pr56812.cc: Check vect_nopeel.
* lib/target-supports.exp (check_effective_target_vect_nopeel):
New proc.
---
gcc/doc/sourcebuild.texi | 3 +++
gcc/testsuite/g++.dg/vect/slp-pr56812.cc | 4 +++-
gcc/testsuite/lib/target-supports.exp | 22 ++++++++++++++++++++++
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 307c726..3acfd85 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1398,6 +1398,9 @@ Target supports a vector misalign access.
@item vect_no_align
Target does not support a vector alignment mechanism.
+@item vect_no_peel
+Target does not require any loop peeling for alignment purposes.
+
@item vect_no_int_min_max
Target does not support a vector min and max instruction on @code{int}.
diff --git a/gcc/testsuite/g++.dg/vect/slp-pr56812.cc b/gcc/testsuite/g++.dg/vect/slp-pr56812.cc
index 80bdcdd..3dbaf76 100644
--- a/gcc/testsuite/g++.dg/vect/slp-pr56812.cc
+++ b/gcc/testsuite/g++.dg/vect/slp-pr56812.cc
@@ -17,4 +17,6 @@ void mydata::Set (float x)
data[i] = x;
}
-/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" } } */
+/* For targets without vector loop peeling the loop becomes cheap
+ enough to be vectorized. */
+/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" { target { ! vect_no_peel }
} } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7fdfbbb..31e802d 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3199,6 +3199,28 @@ proc check_effective_target_vect_floatuint_cvt { } {
return $et_vect_floatuint_cvt_saved($et_index)
}
+# Return 1 if peeling for alignment is never profitable on the target
+#
+
+proc check_effective_target_vect_no_peel { } {
+ global et_vect_no_peel_saved
+ global et_index
+
+ if [info exists et_vect_no_peel_saved($et_index)] {
+ verbose "check_effective_target_vect_no_peel: using cached result" 2
+ } else {
+ set et_vect_no_peel_saved($et_index) 0
+ if { ([istarget s390*-*-*]
+ && [check_effective_target_s390_vx]) } {
+ set et_vect_no_peel_saved($et_index) 1
+ }
+ }
+
+ verbose "check_effective_target_vect_no_peel:\
+ returning $et_vect_no_peel_saved($et_index)" 2
+ return $et_vect_no_peel_saved($et_index)
+}
+
# Return 1 if the target supports #pragma omp declare simd, 0 otherwise.
#
# This won't change for different subtargets so cache the result.
Hi Andreas, > diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi > index 307c726..3acfd85 100644 > --- a/gcc/doc/sourcebuild.texi > +++ b/gcc/doc/sourcebuild.texi > @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. > @item vect_no_align > Target does not support a vector alignment mechanism. > > +@item vect_no_peel > +Target does not require any loop peeling for alignment purposes. > + > @item vect_no_int_min_max > Target does not support a vector min and max instruction on @code{int}. please keep the items sorted alphabetically. Thanks. Rainer
Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes: > - vect_nopeel renamed to vect_no_peel > - documentation added. > > gcc/testsuite/ChangeLog: > > 2017-09-26 Andreas Krebbel <krebbel@linux.vnet.ibm.com> > > * doc/sourcebuild.texi: Document vect_no_peel. > > gcc/testsuite/ChangeLog: > > 2017-09-26 Andreas Krebbel <krebbel@linux.vnet.ibm.com> > > * g++.dg/vect/slp-pr56812.cc: Check vect_nopeel. > * lib/target-supports.exp (check_effective_target_vect_nopeel): > New proc. Sorry for the bikeshedding, but how about having a positive test like vect_can_peel instead? ! vect_no... can be hard to read in complex conditions. (There's already that problem with existing vect_no...s.) > -/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" } } */ > > +/* For targets without vector loop peeling the loop becomes cheap > > + enough to be vectorized. */ > > +/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" { target { ! vect_no_peel } > } } } */ How about an xfail instead? Then it'll be noticeable (via an XPASS) if we fail to vectorise the loop when we should. Thanks, Richard
On 09/26/2017 02:26 PM, Rainer Orth wrote: > Hi Andreas, > >> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >> index 307c726..3acfd85 100644 >> --- a/gcc/doc/sourcebuild.texi >> +++ b/gcc/doc/sourcebuild.texi >> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >> @item vect_no_align >> Target does not support a vector alignment mechanism. >> >> +@item vect_no_peel >> +Target does not require any loop peeling for alignment purposes. >> + >> @item vect_no_int_min_max >> Target does not support a vector min and max instruction on @code{int}. > > please keep the items sorted alphabetically. The items do not appear to be sorted alphabetically. -Andreas-
Hi Andreas, > On 09/26/2017 02:26 PM, Rainer Orth wrote: >> Hi Andreas, >> >>> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >>> index 307c726..3acfd85 100644 >>> --- a/gcc/doc/sourcebuild.texi >>> +++ b/gcc/doc/sourcebuild.texi >>> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >>> @item vect_no_align >>> Target does not support a vector alignment mechanism. >>> >>> +@item vect_no_peel >>> +Target does not require any loop peeling for alignment purposes. >>> + >>> @item vect_no_int_min_max >>> Target does not support a vector min and max instruction on @code{int}. >> >> please keep the items sorted alphabetically. > > The items do not appear to be sorted alphabetically. they should be. Your patch makes the ordering even more random. Patch to fix this preapproved ;-) Thanks. Rainer
On 09/27/2017 10:10 AM, Rainer Orth wrote: > Hi Andreas, > >> On 09/26/2017 02:26 PM, Rainer Orth wrote: >>> Hi Andreas, >>> >>>> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >>>> index 307c726..3acfd85 100644 >>>> --- a/gcc/doc/sourcebuild.texi >>>> +++ b/gcc/doc/sourcebuild.texi >>>> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >>>> @item vect_no_align >>>> Target does not support a vector alignment mechanism. >>>> >>>> +@item vect_no_peel >>>> +Target does not require any loop peeling for alignment purposes. >>>> + >>>> @item vect_no_int_min_max >>>> Target does not support a vector min and max instruction on @code{int}. >>> >>> please keep the items sorted alphabetically. >> >> The items do not appear to be sorted alphabetically. > > they should be. Your patch makes the ordering even more random. > > Patch to fix this preapproved ;-) The items rather appear to be arranged by subject. Does it really make sense do pull items like this apart just to have it in alphabetical order? @item vect_intfloat_cvt Target supports conversion from @code{signed int} to @code{float}. @item vect_uintfloat_cvt Target supports conversion from @code{unsigned int} to @code{float}. @item vect_floatint_cvt Target supports conversion from @code{float} to @code{signed int}. @item vect_floatuint_cvt Target supports conversion from @code{float} to @code{unsigned int}. I've added the no_peel item intentionally to the hw_misalign/no_align block. -Andreas-
Hi Andreas, > On 09/27/2017 10:10 AM, Rainer Orth wrote: >> Hi Andreas, >> >>> On 09/26/2017 02:26 PM, Rainer Orth wrote: >>>> Hi Andreas, >>>> >>>>> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >>>>> index 307c726..3acfd85 100644 >>>>> --- a/gcc/doc/sourcebuild.texi >>>>> +++ b/gcc/doc/sourcebuild.texi >>>>> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >>>>> @item vect_no_align >>>>> Target does not support a vector alignment mechanism. >>>>> >>>>> +@item vect_no_peel >>>>> +Target does not require any loop peeling for alignment purposes. >>>>> + >>>>> @item vect_no_int_min_max >>>>> Target does not support a vector min and max instruction on @code{int}. >>>> >>>> please keep the items sorted alphabetically. >>> >>> The items do not appear to be sorted alphabetically. >> >> they should be. Your patch makes the ordering even more random. >> >> Patch to fix this preapproved ;-) > The items rather appear to be arranged by subject. Does it really make > sense do pull items like this > apart just to have it in alphabetical order? > > @item vect_intfloat_cvt > Target supports conversion from @code{signed int} to @code{float}. > > @item vect_uintfloat_cvt > Target supports conversion from @code{unsigned int} to @code{float}. > > @item vect_floatint_cvt > Target supports conversion from @code{float} to @code{signed int}. > > @item vect_floatuint_cvt > Target supports conversion from @code{float} to @code{unsigned int}. > > > I've added the no_peel item intentionally to the hw_misalign/no_align block. granted, there are some attempts at that, but I find it hard to make my way through that longish list. The way it is, you have to skip through the whole list beginning to end. Texinfo seems to have no subsubsection which would allow to make the sub-grouping explicit... Let's hear what Sandra thinks. Rainer
On 09/27/2017 03:05 AM, Rainer Orth wrote: > Hi Andreas, > >> On 09/27/2017 10:10 AM, Rainer Orth wrote: >>> Hi Andreas, >>> >>>> On 09/26/2017 02:26 PM, Rainer Orth wrote: >>>>> Hi Andreas, >>>>> >>>>>> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >>>>>> index 307c726..3acfd85 100644 >>>>>> --- a/gcc/doc/sourcebuild.texi >>>>>> +++ b/gcc/doc/sourcebuild.texi >>>>>> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >>>>>> @item vect_no_align >>>>>> Target does not support a vector alignment mechanism. >>>>>> >>>>>> +@item vect_no_peel >>>>>> +Target does not require any loop peeling for alignment purposes. >>>>>> + >>>>>> @item vect_no_int_min_max >>>>>> Target does not support a vector min and max instruction on @code{int}. >>>>> >>>>> please keep the items sorted alphabetically. >>>> >>>> The items do not appear to be sorted alphabetically. >>> >>> they should be. Your patch makes the ordering even more random. >>> >>> Patch to fix this preapproved ;-) >> The items rather appear to be arranged by subject. Does it really make >> sense do pull items like this >> apart just to have it in alphabetical order? >> >> @item vect_intfloat_cvt >> Target supports conversion from @code{signed int} to @code{float}. >> >> @item vect_uintfloat_cvt >> Target supports conversion from @code{unsigned int} to @code{float}. >> >> @item vect_floatint_cvt >> Target supports conversion from @code{float} to @code{signed int}. >> >> @item vect_floatuint_cvt >> Target supports conversion from @code{float} to @code{unsigned int}. >> >> >> I've added the no_peel item intentionally to the hw_misalign/no_align block. > > granted, there are some attempts at that, but I find it hard to make my > way through that longish list. The way it is, you have to skip through > the whole list beginning to end. Texinfo seems to have no subsubsection > which would allow to make the sub-grouping explicit... > > Let's hear what Sandra thinks. Ummmm. There is no common convention in the GCC documentation and other parts of the manual do deliberately diverge from alphabetization in places. There's a perpetual tension between putting the most commonly-needed information first vs grouping things by related concepts vs alphabetize vs the tendency of people to insert new items at random places in an existing list regardless of how it's previously been organized. :-( Alphabetical lists are useful when you already know the name of the thing you are searching for, but almost everybody reads the documentation in a web browser or PDF viewer with a search feature nowadays so you can find the term no matter how the list is sorted. So I'd say we shouldn't alphabetize as a matter of policy if there is some other organization that makes sense. In this case, the section is already broken into multiple sublists by topic, most of the sublists are fairly short, and where there's some discernible sort order within the sublists, it seems to be grouping related things together rather than alphabetical. So I wouldn't insist on alphabetizing this particular sublist either. -Sandra
On 09/26/2017 06:49 PM, Richard Sandiford wrote: > Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes: ... > Sorry for the bikeshedding, but how about having a positive test > like vect_can_peel instead? ! vect_no... can be hard to read in > complex conditions. (There's already that problem with existing > vect_no...s.) Done. Updated patch here: https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01867.html Bye, -Andreas-
On 09/27/2017 07:30 PM, Sandra Loosemore wrote: > On 09/27/2017 03:05 AM, Rainer Orth wrote: >> Hi Andreas, >> >>> On 09/27/2017 10:10 AM, Rainer Orth wrote: >>>> Hi Andreas, >>>> >>>>> On 09/26/2017 02:26 PM, Rainer Orth wrote: >>>>>> Hi Andreas, >>>>>> >>>>>>> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi >>>>>>> index 307c726..3acfd85 100644 >>>>>>> --- a/gcc/doc/sourcebuild.texi >>>>>>> +++ b/gcc/doc/sourcebuild.texi >>>>>>> @@ -1398,6 +1398,9 @@ Target supports a vector misalign access. >>>>>>> @item vect_no_align >>>>>>> Target does not support a vector alignment mechanism. >>>>>>> >>>>>>> +@item vect_no_peel >>>>>>> +Target does not require any loop peeling for alignment purposes. >>>>>>> + >>>>>>> @item vect_no_int_min_max >>>>>>> Target does not support a vector min and max instruction on @code{int}. >>>>>> >>>>>> please keep the items sorted alphabetically. >>>>> >>>>> The items do not appear to be sorted alphabetically. >>>> >>>> they should be. Your patch makes the ordering even more random. >>>> >>>> Patch to fix this preapproved ;-) >>> The items rather appear to be arranged by subject. Does it really make >>> sense do pull items like this >>> apart just to have it in alphabetical order? >>> >>> @item vect_intfloat_cvt >>> Target supports conversion from @code{signed int} to @code{float}. >>> >>> @item vect_uintfloat_cvt >>> Target supports conversion from @code{unsigned int} to @code{float}. >>> >>> @item vect_floatint_cvt >>> Target supports conversion from @code{float} to @code{signed int}. >>> >>> @item vect_floatuint_cvt >>> Target supports conversion from @code{float} to @code{unsigned int}. >>> >>> >>> I've added the no_peel item intentionally to the hw_misalign/no_align block. >> >> granted, there are some attempts at that, but I find it hard to make my >> way through that longish list. The way it is, you have to skip through >> the whole list beginning to end. Texinfo seems to have no subsubsection >> which would allow to make the sub-grouping explicit... >> >> Let's hear what Sandra thinks. > > Ummmm. There is no common convention in the GCC documentation and other > parts of the manual do deliberately diverge from alphabetization in > places. There's a perpetual tension between putting the most > commonly-needed information first vs grouping things by related concepts > vs alphabetize vs the tendency of people to insert new items at random > places in an existing list regardless of how it's previously been > organized. :-( > > Alphabetical lists are useful when you already know the name of the > thing you are searching for, but almost everybody reads the > documentation in a web browser or PDF viewer with a search feature > nowadays so you can find the term no matter how the list is sorted. So > I'd say we shouldn't alphabetize as a matter of policy if there is some > other organization that makes sense. > > In this case, the section is already broken into multiple sublists by > topic, most of the sublists are fairly short, and where there's some > discernible sort order within the sublists, it seems to be grouping > related things together rather than alphabetical. So I wouldn't insist > on alphabetizing this particular sublist either. > > -Sandra Ok thanks for the clarification. I'll try to fit the documentation updates into the existing structure. Updated patchset here: https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01862.html Bye, -Andreas-
diff --git a/gcc/testsuite/g++.dg/vect/slp-pr56812.cc b/gcc/testsuite/g++.dg/vect/slp-pr56812.cc index 80bdcdd..955b2ef 100644 --- a/gcc/testsuite/g++.dg/vect/slp-pr56812.cc +++ b/gcc/testsuite/g++.dg/vect/slp-pr56812.cc @@ -17,4 +17,6 @@ void mydata::Set (float x) data[i] = x; } -/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" } } */ +/* For targets without vector loop peeling the loop becomes cheap + enough to be vectorized. */ +/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp1" { target { ! vect_nopeel } } } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 7fdfbbb..686465a 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -3199,6 +3199,28 @@ proc check_effective_target_vect_floatuint_cvt { } { return $et_vect_floatuint_cvt_saved($et_index) } +# Return 1 if peeling for alignment is never profitable on the target +# + +proc check_effective_target_vect_nopeel { } { + global et_vect_nopeel_saved + global et_index + + if [info exists et_vect_nopeel_saved($et_index)] { + verbose "check_effective_target_vect_nopeel: using cached result" 2 + } else { + set et_vect_nopeel_saved($et_index) 0 + if { ([istarget s390*-*-*] + && [check_effective_target_s390_vx]) } { + set et_vect_nopeel_saved($et_index) 1 + } + } + + verbose "check_effective_target_vect_nopeel:\ + returning $et_vect_nopeel_saved($et_index)" 2 + return $et_vect_nopeel_saved($et_index) +} + # Return 1 if the target supports #pragma omp declare simd, 0 otherwise. # # This won't change for different subtargets so cache the result.