diff mbox series

testsuite: Make pr104992.c irrelated to target vector feature [PR113418]

Message ID 20240123120956.45968-1-xry111@xry111.site
State New
Headers show
Series testsuite: Make pr104992.c irrelated to target vector feature [PR113418] | expand

Commit Message

Xi Ruoyao Jan. 23, 2024, 12:09 p.m. UTC 
The vect_int_mod target selector is evaluated with the options in
DEFAULT_VECTCFLAGS in effect, but these options are not automatically
passed to tests out of the vect directories.  So this test fails on
targets where integer vector modulo operation is supported but requiring
an option to enable, for example LoongArch.

In this test case, the only expected optimization not happened in
original is in corge because it needs forward propogation.  So we can
scan the forwprop2 dump (where the vector operation is not expanded to
scalars yet) instead of optimized, then we don't need to consider
vect_int_mod or not.

gcc/testsuite/ChangeLog:

	PR testsuite/113418
	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
	instead of -fdump-tree-optimized.
	(dg-final): Scan forwprop2 dump instead of optimized, and remove
	the use of vect_int_mod.
---

This fixes the test failure on loongarch64-linux-gnu, and I've also
tested it on x86_64-linux-gnu.  Ok for trunk?

 gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

chenxiaolong Jan. 24, 2024, 10:32 a.m. UTC | #1 
On 20:09 +0800 on Tuesday, 2024-01-23, Xi Ruoyao wrote：
> The vect_int_mod target selector is evaluated with the options in
> DEFAULT_VECTCFLAGS in effect, but these options are not automatically
> passed to tests out of the vect directories.  So this test fails on
> targets where integer vector modulo operation is supported but
> requiring
> an option to enable, for example LoongArch.
> 
> In this test case, the only expected optimization not happened in
> original is in corge because it needs forward propogation.  So we can
> scan the forwprop2 dump (where the vector operation is not expanded
> to
> scalars yet) instead of optimized, then we don't need to consider
> vect_int_mod or not.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR testsuite/113418
> 	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
> 	instead of -fdump-tree-optimized.
> 	(dg-final): Scan forwprop2 dump instead of optimized, and
> remove
> 	the use of vect_int_mod.
> ---
> 
> This fixes the test failure on loongarch64-linux-gnu, and I've also
> tested it on x86_64-linux-gnu.  Ok for trunk?
> 
>  gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/pr104992.c
> b/gcc/testsuite/gcc.dg/pr104992.c
> index 82f8c75559c..6fd513d34b2 100644
> --- a/gcc/testsuite/gcc.dg/pr104992.c
> +++ b/gcc/testsuite/gcc.dg/pr104992.c
> @@ -1,6 +1,6 @@
>  /* PR tree-optimization/104992 */
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
> +/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
>  
>  #define vector __attribute__((vector_size(4*sizeof(int))))
>  
> @@ -54,5 +54,4 @@ __attribute__((noipa)) unsigned waldo (unsigned x,
> unsigned y, unsigned z) {
>      return x / y * z == x;
>  }
>  
> -/* { dg-final { scan-tree-dump-times " % " 9 "optimized" { target {
> ! vect_int_mod } } } } */
> -/* { dg-final { scan-tree-dump-times " % " 6 "optimized" { target
> vect_int_mod } } } */
> +/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */

Hello, currently vect_int_mod vectorization operation detection only
ppc,amd,riscv,LoongArch architecture support. When -fdump-tree-
forwprop2 is used instead of -fdump-tree-optimized, The
check_effective_target_vect_int_mod procedure defined in the target-
supports.exp file will never be called. It will only be called on
pr104992.c, should we consider supporting other architectures?
Xi Ruoyao Jan. 24, 2024, 11 a.m. UTC | #2 
On Wed, 2024-01-24 at 18:32 +0800, chenxiaolong wrote:
> On 20:09 +0800 on Tuesday, 2024-01-23, Xi Ruoyao wrote：
> > The vect_int_mod target selector is evaluated with the options in
> > DEFAULT_VECTCFLAGS in effect, but these options are not automatically
> > passed to tests out of the vect directories.  So this test fails on
> > targets where integer vector modulo operation is supported but
> > requiring
> > an option to enable, for example LoongArch.
> > 
> > In this test case, the only expected optimization not happened in
> > original is in corge because it needs forward propogation.  So we can
> > scan the forwprop2 dump (where the vector operation is not expanded
> > to
> > scalars yet) instead of optimized, then we don't need to consider
> > vect_int_mod or not.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR testsuite/113418
> > 	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
> > 	instead of -fdump-tree-optimized.
> > 	(dg-final): Scan forwprop2 dump instead of optimized, and
> > remove
> > 	the use of vect_int_mod.
> > ---
> > 
> > This fixes the test failure on loongarch64-linux-gnu, and I've also
> > tested it on x86_64-linux-gnu.  Ok for trunk?
> > 
> >  gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.dg/pr104992.c
> > b/gcc/testsuite/gcc.dg/pr104992.c
> > index 82f8c75559c..6fd513d34b2 100644
> > --- a/gcc/testsuite/gcc.dg/pr104992.c
> > +++ b/gcc/testsuite/gcc.dg/pr104992.c
> > @@ -1,6 +1,6 @@
> >  /* PR tree-optimization/104992 */
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
> > +/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
> >  
> >  #define vector __attribute__((vector_size(4*sizeof(int))))
> >  
> > @@ -54,5 +54,4 @@ __attribute__((noipa)) unsigned waldo (unsigned x,
> > unsigned y, unsigned z) {
> >      return x / y * z == x;
> >  }
> >  
> > -/* { dg-final { scan-tree-dump-times " % " 9 "optimized" { target {
> > ! vect_int_mod } } } } */
> > -/* { dg-final { scan-tree-dump-times " % " 6 "optimized" { target
> > vect_int_mod } } } */
> > +/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */
> 
> Hello, currently vect_int_mod vectorization operation detection only
> ppc,amd,riscv,LoongArch architecture support. When -fdump-tree-
> forwprop2 is used instead of -fdump-tree-optimized, The
> check_effective_target_vect_int_mod procedure defined in the target-
> supports.exp file will never be called. It will only be called on
> pr104992.c, should we consider supporting other architectures?

Hmm, then we should remove check_effective_target_vect_int_mod.

If we want to keep -fdump-tree-optimized for this test case and also
make it correct, we'll at least have to move it into vect/, and write
something like

{ dg-final { scan-tree-dump-times " % " 9 "optimized" { target { ! vect_int_mod } } } }
{ dg-final { scan-tree-dump-times " % " 6 "optimized" { target { vect_int_mod && vect128 } } } }
{ dg-final { scan-tree-dump-times " % " 7 "optimized" { target { vect_int_mod && vect64 && !vect128 } } } }

and how about vect256 etc?  This would be very nasty and deviating from
the original purpose of this test case (against PR104992, which is a
missed-optimization issue unrelated to vectors).
chenxiaolong Jan. 24, 2024, 11:08 a.m. UTC | #3 
At 19:00 +0800 on Wednesday, 2024-01-24, Xi Ruoyao wrote：
> On Wed, 2024-01-24 at 18:32 +0800, chenxiaolong wrote:
> > On 20:09 +0800 on Tuesday, 2024-01-23, Xi Ruoyao wrote：
> > > The vect_int_mod target selector is evaluated with the options in
> > > DEFAULT_VECTCFLAGS in effect, but these options are not
> > > automatically
> > > passed to tests out of the vect directories.  So this test fails
> > > on
> > > targets where integer vector modulo operation is supported but
> > > requiring
> > > an option to enable, for example LoongArch.
> > > 
> > > In this test case, the only expected optimization not happened in
> > > original is in corge because it needs forward propogation.  So we
> > > can
> > > scan the forwprop2 dump (where the vector operation is not
> > > expanded
> > > to
> > > scalars yet) instead of optimized, then we don't need to consider
> > > vect_int_mod or not.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	PR testsuite/113418
> > > 	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
> > > 	instead of -fdump-tree-optimized.
> > > 	(dg-final): Scan forwprop2 dump instead of optimized, and
> > > remove
> > > 	the use of vect_int_mod.
> > > ---
> > > 
> > > This fixes the test failure on loongarch64-linux-gnu, and I've
> > > also
> > > tested it on x86_64-linux-gnu.  Ok for trunk?
> > > 
> > >  gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/gcc/testsuite/gcc.dg/pr104992.c
> > > b/gcc/testsuite/gcc.dg/pr104992.c
> > > index 82f8c75559c..6fd513d34b2 100644
> > > --- a/gcc/testsuite/gcc.dg/pr104992.c
> > > +++ b/gcc/testsuite/gcc.dg/pr104992.c
> > > @@ -1,6 +1,6 @@
> > >  /* PR tree-optimization/104992 */
> > >  /* { dg-do compile } */
> > > -/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
> > > +/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
> > >  
> > >  #define vector __attribute__((vector_size(4*sizeof(int))))
> > >  
> > > @@ -54,5 +54,4 @@ __attribute__((noipa)) unsigned waldo (unsigned
> > > x,
> > > unsigned y, unsigned z) {
> > >      return x / y * z == x;
> > >  }
> > >  
> > > -/* { dg-final { scan-tree-dump-times " % " 9 "optimized" {
> > > target {
> > > ! vect_int_mod } } } } */
> > > -/* { dg-final { scan-tree-dump-times " % " 6 "optimized" {
> > > target
> > > vect_int_mod } } } */
> > > +/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */
> > 
> > Hello, currently vect_int_mod vectorization operation detection
> > only
> > ppc,amd,riscv,LoongArch architecture support. When -fdump-tree-
> > forwprop2 is used instead of -fdump-tree-optimized, The
> > check_effective_target_vect_int_mod procedure defined in the
> > target-
> > supports.exp file will never be called. It will only be called on
> > pr104992.c, should we consider supporting other architectures?
> 
> Hmm, then we should remove check_effective_target_vect_int_mod.
> 
> If we want to keep -fdump-tree-optimized for this test case and also
> make it correct, we'll at least have to move it into vect/, and write
> something like
> 
> { dg-final { scan-tree-dump-times " % " 9 "optimized" { target { !
> vect_int_mod } } } }
> { dg-final { scan-tree-dump-times " % " 6 "optimized" { target {
> vect_int_mod && vect128 } } } }
> { dg-final { scan-tree-dump-times " % " 7 "optimized" { target {
> vect_int_mod && vect64 && !vect128 } } } }
> 
> and how about vect256 etc?  This would be very nasty and deviating
> from
> the original purpose of this test case (against PR104992, which is a
> missed-optimization issue unrelated to vectors).
> 
Ok, let me think about how to make the pr104992.c test case more
reasonable.
Xi Ruoyao Jan. 24, 2024, 11:42 a.m. UTC | #4 
On Wed, 2024-01-24 at 19:08 +0800, chenxiaolong wrote:
> At 19:00 +0800 on Wednesday, 2024-01-24, Xi Ruoyao wrote：
> > On Wed, 2024-01-24 at 18:32 +0800, chenxiaolong wrote:
> > > On 20:09 +0800 on Tuesday, 2024-01-23, Xi Ruoyao wrote：
> > > > The vect_int_mod target selector is evaluated with the options in
> > > > DEFAULT_VECTCFLAGS in effect, but these options are not
> > > > automatically
> > > > passed to tests out of the vect directories.  So this test fails
> > > > on
> > > > targets where integer vector modulo operation is supported but
> > > > requiring
> > > > an option to enable, for example LoongArch.
> > > > 
> > > > In this test case, the only expected optimization not happened in
> > > > original is in corge because it needs forward propogation.  So we
> > > > can
> > > > scan the forwprop2 dump (where the vector operation is not
> > > > expanded
> > > > to
> > > > scalars yet) instead of optimized, then we don't need to consider
> > > > vect_int_mod or not.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	PR testsuite/113418
> > > > 	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
> > > > 	instead of -fdump-tree-optimized.
> > > > 	(dg-final): Scan forwprop2 dump instead of optimized, and
> > > > remove
> > > > 	the use of vect_int_mod.
> > > > ---
> > > > 
> > > > This fixes the test failure on loongarch64-linux-gnu, and I've
> > > > also
> > > > tested it on x86_64-linux-gnu.  Ok for trunk?
> > > > 
> > > >  gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
> > > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/gcc/testsuite/gcc.dg/pr104992.c
> > > > b/gcc/testsuite/gcc.dg/pr104992.c
> > > > index 82f8c75559c..6fd513d34b2 100644
> > > > --- a/gcc/testsuite/gcc.dg/pr104992.c
> > > > +++ b/gcc/testsuite/gcc.dg/pr104992.c
> > > > @@ -1,6 +1,6 @@
> > > >  /* PR tree-optimization/104992 */
> > > >  /* { dg-do compile } */
> > > > -/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
> > > > +/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
> > > >  
> > > >  #define vector __attribute__((vector_size(4*sizeof(int))))
> > > >  
> > > > @@ -54,5 +54,4 @@ __attribute__((noipa)) unsigned waldo (unsigned
> > > > x,
> > > > unsigned y, unsigned z) {
> > > >      return x / y * z == x;
> > > >  }
> > > >  
> > > > -/* { dg-final { scan-tree-dump-times " % " 9 "optimized" {
> > > > target {
> > > > ! vect_int_mod } } } } */
> > > > -/* { dg-final { scan-tree-dump-times " % " 6 "optimized" {
> > > > target
> > > > vect_int_mod } } } */
> > > > +/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */
> > > 
> > > Hello, currently vect_int_mod vectorization operation detection
> > > only
> > > ppc,amd,riscv,LoongArch architecture support. When -fdump-tree-
> > > forwprop2 is used instead of -fdump-tree-optimized, The
> > > check_effective_target_vect_int_mod procedure defined in the
> > > target-
> > > supports.exp file will never be called. It will only be called on
> > > pr104992.c, should we consider supporting other architectures?
> > 
> > Hmm, then we should remove check_effective_target_vect_int_mod.
> > 
> > If we want to keep -fdump-tree-optimized for this test case and also
> > make it correct, we'll at least have to move it into vect/, and write
> > something like
> > 
> > { dg-final { scan-tree-dump-times " % " 9 "optimized" { target { !
> > vect_int_mod } } } }
> > { dg-final { scan-tree-dump-times " % " 6 "optimized" { target {
> > vect_int_mod && vect128 } } } }
> > { dg-final { scan-tree-dump-times " % " 7 "optimized" { target {
> > vect_int_mod && vect64 && !vect128 } } } }
> > 
> > and how about vect256 etc?  This would be very nasty and deviating
> > from
> > the original purpose of this test case (against PR104992, which is a
> > missed-optimization issue unrelated to vectors).
> > 
> Ok, let me think about how to make the pr104992.c test case more
> reasonable.

It *is* reasonable with -fdump-tree-forwprop2.  It's purposed to test a
/ b * b -> a - a % b simplification, not vector operations.

If we need a test to test vector int modulo operations we should write a
new test in vect/, like

/* ... */

for (int i = 0; i < 4; i++)
  x[i] %= y[i];

/* ... */

/* { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target { vect_int_mod } } } } */
Kewen.Lin Jan. 25, 2024, 2:37 a.m. UTC | #5 
Hi,

Thanks for adjusting this.

on 2024/1/24 19:42, Xi Ruoyao wrote:
> On Wed, 2024-01-24 at 19:08 +0800, chenxiaolong wrote:
>> At 19:00 +0800 on Wednesday, 2024-01-24, Xi Ruoyao wrote：
>>> On Wed, 2024-01-24 at 18:32 +0800, chenxiaolong wrote:
>>>> On 20:09 +0800 on Tuesday, 2024-01-23, Xi Ruoyao wrote：
>>>>> The vect_int_mod target selector is evaluated with the options in
>>>>> DEFAULT_VECTCFLAGS in effect, but these options are not
>>>>> automatically
>>>>> passed to tests out of the vect directories.  So this test fails
>>>>> on
>>>>> targets where integer vector modulo operation is supported but
>>>>> requiring
>>>>> an option to enable, for example LoongArch.
>>>>>
>>>>> In this test case, the only expected optimization not happened in
>>>>> original is in corge because it needs forward propogation.  So we
>>>>> can
>>>>> scan the forwprop2 dump (where the vector operation is not
>>>>> expanded
>>>>> to
>>>>> scalars yet) instead of optimized, then we don't need to consider
>>>>> vect_int_mod or not.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	PR testsuite/113418
>>>>> 	* gcc.dg/pr104992.c (dg-options): Use -fdump-tree-forwprop2
>>>>> 	instead of -fdump-tree-optimized.
>>>>> 	(dg-final): Scan forwprop2 dump instead of optimized, and
>>>>> remove
>>>>> 	the use of vect_int_mod.
>>>>> ---
>>>>>
>>>>> This fixes the test failure on loongarch64-linux-gnu, and I've
>>>>> also
>>>>> tested it on x86_64-linux-gnu.  Ok for trunk?
>>>>>
>>>>>  gcc/testsuite/gcc.dg/pr104992.c | 5 ++---
>>>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/gcc/testsuite/gcc.dg/pr104992.c
>>>>> b/gcc/testsuite/gcc.dg/pr104992.c
>>>>> index 82f8c75559c..6fd513d34b2 100644
>>>>> --- a/gcc/testsuite/gcc.dg/pr104992.c
>>>>> +++ b/gcc/testsuite/gcc.dg/pr104992.c
>>>>> @@ -1,6 +1,6 @@
>>>>>  /* PR tree-optimization/104992 */
>>>>>  /* { dg-do compile } */
>>>>> -/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
>>>>> +/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
>>>>>  
>>>>>  #define vector __attribute__((vector_size(4*sizeof(int))))
>>>>>  
>>>>> @@ -54,5 +54,4 @@ __attribute__((noipa)) unsigned waldo (unsigned
>>>>> x,
>>>>> unsigned y, unsigned z) {
>>>>>      return x / y * z == x;
>>>>>  }
>>>>>  
>>>>> -/* { dg-final { scan-tree-dump-times " % " 9 "optimized" {
>>>>> target {
>>>>> ! vect_int_mod } } } } */
>>>>> -/* { dg-final { scan-tree-dump-times " % " 6 "optimized" {
>>>>> target
>>>>> vect_int_mod } } } */
>>>>> +/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */
>>>>
>>>> Hello, currently vect_int_mod vectorization operation detection
>>>> only
>>>> ppc,amd,riscv,LoongArch architecture support. When -fdump-tree-
>>>> forwprop2 is used instead of -fdump-tree-optimized, The
>>>> check_effective_target_vect_int_mod procedure defined in the
>>>> target-
>>>> supports.exp file will never be called. It will only be called on
>>>> pr104992.c, should we consider supporting other architectures?
>>>
>>> Hmm, then we should remove check_effective_target_vect_int_mod.
>>>
>>> If we want to keep -fdump-tree-optimized for this test case and also
>>> make it correct, we'll at least have to move it into vect/, and write
>>> something like
>>>
>>> { dg-final { scan-tree-dump-times " % " 9 "optimized" { target { !
>>> vect_int_mod } } } }
>>> { dg-final { scan-tree-dump-times " % " 6 "optimized" { target {
>>> vect_int_mod && vect128 } } } }
>>> { dg-final { scan-tree-dump-times " % " 7 "optimized" { target {
>>> vect_int_mod && vect64 && !vect128 } } } }
>>>
>>> and how about vect256 etc?  This would be very nasty and deviating
>>> from
>>> the original purpose of this test case (against PR104992, which is a
>>> missed-optimization issue unrelated to vectors).
>>>
>> Ok, let me think about how to make the pr104992.c test case more
>> reasonable.
> 
> It *is* reasonable with -fdump-tree-forwprop2.  It's purposed to test a
> / b * b -> a - a % b simplification, not vector operations.

I agree.  Dropping check_effective_target_vect_int_mod is fine for Power
as we have separated testing coverage on vector int mod capability in
target specific testsuite.  But I'm not sure if it's similar for the
other ports which enable vect_int_mod.  A conservative approach is to
add one new test case into vect/ as you proposed below.

BR,
Kewen

> 
> If we need a test to test vector int modulo operations we should write a
> new test in vect/, like
> 
> /* ... */
> 
> for (int i = 0; i < 4; i++)
>   x[i] %= y[i];
> 
> /* ... */
> 
> /* { dg-final { scan-tree-dump "vectorized 1 loops" "vect" { target { vect_int_mod } } } } */
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/pr104992.c b/gcc/testsuite/gcc.dg/pr104992.c
index 82f8c75559c..6fd513d34b2 100644
--- a/gcc/testsuite/gcc.dg/pr104992.c
+++ b/gcc/testsuite/gcc.dg/pr104992.c
@@ -1,6 +1,6 @@ 
 /* PR tree-optimization/104992 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wno-psabi -fdump-tree-optimized" } */
+/* { dg-options "-O2 -Wno-psabi -fdump-tree-forwprop2" } */
 
 #define vector __attribute__((vector_size(4*sizeof(int))))
 
@@ -54,5 +54,4 @@  __attribute__((noipa)) unsigned waldo (unsigned x, unsigned y, unsigned z) {
     return x / y * z == x;
 }
 
-/* { dg-final { scan-tree-dump-times " % " 9 "optimized" { target { ! vect_int_mod } } } } */
-/* { dg-final { scan-tree-dump-times " % " 6 "optimized" { target vect_int_mod } } } */
+/* { dg-final { scan-tree-dump-times " % " 6 "forwprop2" } } */