[PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[Juzhe-Zhong]

RVV (RISC-V Vector) doesn't enable vect_unpack, but we still vectorize this case well.
So, adjust dump check for RVV.

gcc/testsuite/ChangeLog:

	* gcc.dg/vect/vect-multitypes-16.c: Fix dump FAIL of RVV.

---
 gcc/testsuite/gcc.dg/vect/vect-multitypes-16.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/vect/vect-multitypes-16.c b/gcc/testsuite/gcc.dg/vect/vect-multitypes-16.c
index a61f1a9a221..829a4d41601 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-multitypes-16.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-multitypes-16.c
@@ -35,6 +35,6 @@ 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 "vectorized 1 loops" 0 "vect" { target { ! vect_unpack } } } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_unpack || riscv_v } } } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 0 "vect" { target { { ! vect_unpack } && { ! riscv_v } } } } } */
 
-- 
2.36.3

Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[Jeff Law]

On 10/8/23 05:35, Juzhe-Zhong wrote:
> RVV (RISC-V Vector) doesn't enable vect_unpack, but we still vectorize this case well.
> So, adjust dump check for RVV.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/vect/vect-multitypes-16.c: Fix dump FAIL of RVV.
I'd hoped to avoid a bunch of risc-v special casing in the generic part 
of the testsuite.  Basically the more we have target specific 
conditionals rather than conditionals using properties, the more likely 
we are to keep revisiting this stuff over time and possibly for other 
architectures as well.

What is it about risc-v's vector support that allows it to optimize this 
case?  Is it the same property that allows us to handle the outer loop 
vectorization tests that you changed in another patch?

Neither an ACK nor NAK right now.

Jeff

Re: Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[钟居哲]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

No. They are not the same property.

Maybe I should pretend RVV support vect_pack/vect_unpack and enable all the tests in target-supports.exp?




juzhe.zhong@rivai.ai
 
From: Jeff Law
Date: 2023-10-08 23:09
To: Juzhe-Zhong; gcc-patches
CC: rguenther
Subject: Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV
 
 
On 10/8/23 05:35, Juzhe-Zhong wrote:
> RVV (RISC-V Vector) doesn't enable vect_unpack, but we still vectorize this case well.
> So, adjust dump check for RVV.
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.dg/vect/vect-multitypes-16.c: Fix dump FAIL of RVV.
I'd hoped to avoid a bunch of risc-v special casing in the generic part 
of the testsuite.  Basically the more we have target specific 
conditionals rather than conditionals using properties, the more likely 
we are to keep revisiting this stuff over time and possibly for other 
architectures as well.
 
What is it about risc-v's vector support that allows it to optimize this 
case?  Is it the same property that allows us to handle the outer loop 
vectorization tests that you changed in another patch?
 
Neither an ACK nor NAK right now.
 
Jeff
 

Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[Robin Dapp]

> Maybe I should pretend RVV support vect_pack/vect_unpack and enable
> all the tests in target-supports.exp?

The problem is that vect_pack/unpack is an overloaded term in the
moment meaning "vector conversion" (promotion/demotion) or so.  This
test does not require pack/unpack for successful vectorization but our
method of keeping the number of elements the same works as well.  The
naming probably precedes vectorizer support for that.
I can't imagine cases where vectorization would fail because of this
as we can always work around it some way.  So from that point of view
"pretending" to support it would work.  However in case somebody wants
to really write a specific test cases that relies on pack/unpack
(maybe there are already some?) "pretending" would fail.

I lean towards "pretending" at the moment ;)  The other option would be
to rename that and audit all test cases.

Note there are also vect_intfloat_cvt as well as others that don't have
pack/unpack in the name (that we also probably still need to enable).

Regards
 Robin

Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[Richard Biener]

On Sun, 8 Oct 2023, Jeff Law wrote:

> 
> 
> On 10/8/23 05:35, Juzhe-Zhong wrote:
> > RVV (RISC-V Vector) doesn't enable vect_unpack, but we still vectorize this
> > case well.
> > So, adjust dump check for RVV.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >  * gcc.dg/vect/vect-multitypes-16.c: Fix dump FAIL of RVV.
> I'd hoped to avoid a bunch of risc-v special casing in the generic part of the
> testsuite.  Basically the more we have target specific conditionals rather
> than conditionals using properties, the more likely we are to keep revisiting
> this stuff over time and possibly for other architectures as well.
> 
> What is it about risc-v's vector support that allows it to optimize this case?
> Is it the same property that allows us to handle the outer loop vectorization
> tests that you changed in another patch?

I suspect for VLA vectorization we can use direct conversion from
char to long long here?  I also notice the testcase uses 'char',
not specifying its sign.  So either of [sz]extVxyzDIVxyzQI is possibly
provided by RISCV?  (or possibly via some intermediate types in a
multi-step conversion)

For non-VLA and with the single vector size restriction we'd need
unpacking.

So it might be better

 { target { vect_unpack || { vect_vla && vect_sext_char_longlong } } }

where I think neither vect_vla nor vect_sext_char_longlong exists.

Richard - didn't you run into similar things with SVE?

Richard.


> Neither an ACK nor NAK right now.
> 
> Jeff
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[juzhe.zhong]

[-- Attachment #1: Type: text/plain, Size: 2095 bytes --]

Yes. We do have && enable char -> long conversion (vsext.vf8/vzext.vf8)

Thanks for the comment, I will adapt test as you suggested.



juzhe.zhong@rivai.ai
 
From: Richard Biener
Date: 2023-10-09 15:31
To: Jeff Law
CC: Juzhe-Zhong; gcc-patches; richard.sandiford
Subject: Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV
On Sun, 8 Oct 2023, Jeff Law wrote:
 
> 
> 
> On 10/8/23 05:35, Juzhe-Zhong wrote:
> > RVV (RISC-V Vector) doesn't enable vect_unpack, but we still vectorize this
> > case well.
> > So, adjust dump check for RVV.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >  * gcc.dg/vect/vect-multitypes-16.c: Fix dump FAIL of RVV.
> I'd hoped to avoid a bunch of risc-v special casing in the generic part of the
> testsuite.  Basically the more we have target specific conditionals rather
> than conditionals using properties, the more likely we are to keep revisiting
> this stuff over time and possibly for other architectures as well.
> 
> What is it about risc-v's vector support that allows it to optimize this case?
> Is it the same property that allows us to handle the outer loop vectorization
> tests that you changed in another patch?
 
I suspect for VLA vectorization we can use direct conversion from
char to long long here?  I also notice the testcase uses 'char',
not specifying its sign.  So either of [sz]extVxyzDIVxyzQI is possibly
provided by RISCV?  (or possibly via some intermediate types in a
multi-step conversion)
 
For non-VLA and with the single vector size restriction we'd need
unpacking.
 
So it might be better
 
{ target { vect_unpack || { vect_vla && vect_sext_char_longlong } } }
 
where I think neither vect_vla nor vect_sext_char_longlong exists.
 
Richard - didn't you run into similar things with SVE?
 
Richard.
 
 
> Neither an ACK nor NAK right now.
> 
> Jeff
> 
> 
 
-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
 

Re: [PATCH] TEST: Fix dump FAIL of vect-multitypes-16.c for RVV

[Richard Biener]

On Mon, 9 Oct 2023, Robin Dapp wrote:

> > Maybe I should pretend RVV support vect_pack/vect_unpack and enable
> > all the tests in target-supports.exp?
> 
> The problem is that vect_pack/unpack is an overloaded term in the
> moment meaning "vector conversion" (promotion/demotion) or so.  This
> test does not require pack/unpack for successful vectorization but our
> method of keeping the number of elements the same works as well.  The
> naming probably precedes vectorizer support for that.
> I can't imagine cases where vectorization would fail because of this
> as we can always work around it some way.  So from that point of view
> "pretending" to support it would work.  However in case somebody wants
> to really write a specific test cases that relies on pack/unpack
> (maybe there are already some?) "pretending" would fail.

I suspect that for VLS you need to provide the respective patterns
because of the single vector-size restriction.

> I lean towards "pretending" at the moment ;)  The other option would be
> to rename that and audit all test cases.
> 
> Note there are also vect_intfloat_cvt as well as others that don't have
> pack/unpack in the name (that we also probably still need to enable).

Yeah well - the dejagnu "targets" are mostly too broad, but as usual
time is spent elsewhere instead of at cleaning up the mess ;)

It might be more useful to provide vect_<optab><mode>.. dg targets
because then it's at least obvious what is meant.  Or group
things as vect_<optab>float vect_<optab>int.

Richard.

> Regards
>  Robin
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)