[testsuite] Fix PR94019 to allow one vector char when !vect_hw_misalign

Hi,

As PR94019 shows, without misaligned vector access support but with
realign load, the vectorized loop will end up with realign scheme.
It generates mask (control vector) with return type vector signed
char which breaks the not check.

The fix is to differentiate powerpc vect_hw_misalign and powerpc
!vect_hw_misalign, permit one vector char occurance for powerpc
!vect_hw_misalign and keep other targets same as before.

Verified it on ppc64-redhat-linux (Power7 BE).

Is it ok for trunk, and backport to GCC 9 after some burn-in time?


BR,
Kewen
--------

gcc/testsuite/ChangeLog

2020-03-04  Kewen Lin  <linkw@gcc.gnu.org>

	PR testsuite/94019
	* gcc.dg/vect/vect-over-widen-17.c: Expect one vector char if it's on
	POWER and without misaligned vector access support.

--------

Hi!

On Wed, Mar 04, 2020 at 03:13:51PM +0800, Kewen.Lin wrote:
> As PR94019 shows, without misaligned vector access support but with
> realign load, the vectorized loop will end up with realign scheme.
> It generates mask (control vector) with return type vector signed
> char which breaks the not check.
> 
> The fix is to differentiate powerpc vect_hw_misalign and powerpc
> !vect_hw_misalign, permit one vector char occurance for powerpc
> !vect_hw_misalign and keep other targets same as before.
> 
> Verified it on ppc64-redhat-linux (Power7 BE).
> 
> Is it ok for trunk, and backport to GCC 9 after some burn-in time?

Why is any of this Power-specific?  Does it work on other targets if
!vect_hw_misalign?

The patch is fine everywhere as far as the rs6000 port is concerned, but
I'd like someone else who actually understands this stuff to have a look
at it as well ;-)


Segher

"Kewen.Lin" <linkw@linux.ibm.com> writes:
> Hi,
>
> As PR94019 shows, without misaligned vector access support but with
> realign load, the vectorized loop will end up with realign scheme.
> It generates mask (control vector) with return type vector signed
> char which breaks the not check.
>
> The fix is to differentiate powerpc vect_hw_misalign and powerpc
> !vect_hw_misalign, permit one vector char occurance for powerpc
> !vect_hw_misalign and keep other targets same as before.
>
> Verified it on ppc64-redhat-linux (Power7 BE).
>
> Is it ok for trunk, and backport to GCC 9 after some burn-in time?
>
>
> BR,
> Kewen
> --------
>
> gcc/testsuite/ChangeLog
>
> 2020-03-04  Kewen Lin  <linkw@gcc.gnu.org>
>
> 	PR testsuite/94019
> 	* gcc.dg/vect/vect-over-widen-17.c: Expect one vector char if it's on
> 	POWER and without misaligned vector access support.
>
> --------
>
> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
> @@ -41,6 +41,10 @@ main (void)
>  }
>
>  /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */
> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */
> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } }
> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access
> +   adopts realign_load scheme.  It requires rs6000_builtin_mask_for_load to
> +   generate mask whose return type is vector char.  */
> +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */

Thanks for looking at this.  The patch is OK as-is.  However, since
vect-over-widen-17.c is a negative test for generic code, there probably
isn't much need for the new scan-tree-dump-times line, and it could start
failing if we make different optimisation decisions in future.  So the
patch is also OK with just the change to the scan-tree-dump-not line,
if you prefer that.  (Please keep the comment either way though --
it's really helpful.)

Richard

>  /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */
>  /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */

Hi Segher,

on 2020/3/5 上午2:44, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Mar 04, 2020 at 03:13:51PM +0800, Kewen.Lin wrote:
>> As PR94019 shows, without misaligned vector access support but with
>> realign load, the vectorized loop will end up with realign scheme.
>> It generates mask (control vector) with return type vector signed
>> char which breaks the not check.
>>
>> The fix is to differentiate powerpc vect_hw_misalign and powerpc
>> !vect_hw_misalign, permit one vector char occurance for powerpc
>> !vect_hw_misalign and keep other targets same as before.
>>
>> Verified it on ppc64-redhat-linux (Power7 BE).
>>
>> Is it ok for trunk, and backport to GCC 9 after some burn-in time?
> 
> Why is any of this Power-specific?  Does it work on other targets if
> !vect_hw_misalign?
> 

Good question, the initial draft was not to take power as specific and
just guard it with !vect_hw_misalign.  But the root cause is due to
builtin_mask_for_load return type, currently rs6000 port is the only
user providing that hook, I thought it's better to use "times" to
keep the coverage somehow.  To avoid any possible new ports with that
hook but different return type and then leads to unexpected vector
char occurrence times, I further guarded it with powerpc specific.

Considering Richard's comments, I think it's fine to remove that
particularity now.

BR,
Kewen

Hi Richard,

on 2020/3/5 上午3:09, Richard Sandiford wrote:
> "Kewen.Lin" <linkw@linux.ibm.com> writes:
>> Hi,
>>
>>
>> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
>> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
>> @@ -41,6 +41,10 @@ main (void)
>>  }
>>
>>  /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */
>> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */
>> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } }
>> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access
>> +   adopts realign_load scheme.  It requires rs6000_builtin_mask_for_load to
>> +   generate mask whose return type is vector char.  */
>> +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */
> 
> Thanks for looking at this.  The patch is OK as-is.  However, since
> vect-over-widen-17.c is a negative test for generic code, there probably
> isn't much need for the new scan-tree-dump-times line, and it could start
> failing if we make different optimisation decisions in future.  So the
> patch is also OK with just the change to the scan-tree-dump-not line,
> if you prefer that.  (Please keep the comment either way though --
> it's really helpful.)
> 

Thanks for your suggestion!  The new patch is updated as below.  I removed
the scan-tree-dump-times, as well as powerpc specific requirement.  
Does it look good to you especially the later?  Thanks in advance!

BR,
Kewen

gcc/testsuite/ChangeLog

2020-03-05  Kewen Lin  <linkw@gcc.gnu.org>

	PR testsuite/94019
	* gcc.dg/vect/vect-over-widen-17.c: Don't expect vector char if it's
	without misaligned vector access support.

------

diff --git a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
index 0448260..333d74a 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
@@ -41,6 +41,9 @@ main (void)
 }

 /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */
-/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */
+/* On Power, if there is no vect_hw_misalign support, unaligned vector access
+   adopts realign_load scheme.  It requires rs6000_builtin_mask_for_load to
+   generate mask whose return type is vector char.  */
+/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target vect_hw_misalign } } } */
 /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */
 /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */

"Kewen.Lin" <linkw@linux.ibm.com> writes:
> on 2020/3/5 上午3:09, Richard Sandiford wrote:
>> "Kewen.Lin" <linkw@linux.ibm.com> writes:
>>> Hi,
>>>
>>>
>>> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
>>> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
>>> @@ -41,6 +41,10 @@ main (void)
>>>  }
>>>
>>>  /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */
>>> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */
>>> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } }
>>> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access
>>> +   adopts realign_load scheme.  It requires rs6000_builtin_mask_for_load to
>>> +   generate mask whose return type is vector char.  */
>>> +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */
>> 
>> Thanks for looking at this.  The patch is OK as-is.  However, since
>> vect-over-widen-17.c is a negative test for generic code, there probably
>> isn't much need for the new scan-tree-dump-times line, and it could start
>> failing if we make different optimisation decisions in future.  So the
>> patch is also OK with just the change to the scan-tree-dump-not line,
>> if you prefer that.  (Please keep the comment either way though --
>> it's really helpful.)
>> 
>
> Thanks for your suggestion!  The new patch is updated as below.  I removed
> the scan-tree-dump-times, as well as powerpc specific requirement.  
> Does it look good to you especially the later?  Thanks in advance!
>
> BR,
> Kewen
>
> gcc/testsuite/ChangeLog
>
> 2020-03-05  Kewen Lin  <linkw@gcc.gnu.org>
>
> 	PR testsuite/94019
> 	* gcc.dg/vect/vect-over-widen-17.c: Don't expect vector char if it's
> 	without misaligned vector access support.

OK, thanks.

Richard

>
> ------
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
> index 0448260..333d74a 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c
> @@ -41,6 +41,9 @@ main (void)
>  }
>
>  /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */
> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */
> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access
> +   adopts realign_load scheme.  It requires rs6000_builtin_mask_for_load to
> +   generate mask whose return type is vector char.  */
> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target vect_hw_misalign } } } */
>  /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */
>  /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */