* [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior
@ 2023-11-21 23:27 Palmer Dabbelt
2023-11-21 23:27 ` [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets Palmer Dabbelt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Palmer Dabbelt @ 2023-11-21 23:27 UTC (permalink / raw)
To: gcc-patches; +Cc: Palmer Dabbelt
I was poking around with this test failure and noticed it was exercising
undefined behavior. The return type doesn't matter for what's being
tested, so just mark it as void.
gcc/testsuite/ChangeLog:
* gcc.dg/unroll-8.c: Remove UB.
---
I didn't tes this, but it seems trivial enough that I'm just going to
throw it at the bots and hope I'm right.
---
gcc/testsuite/gcc.dg/unroll-8.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.dg/unroll-8.c b/gcc/testsuite/gcc.dg/unroll-8.c
index 4388f47d4c7..06d32e56893 100644
--- a/gcc/testsuite/gcc.dg/unroll-8.c
+++ b/gcc/testsuite/gcc.dg/unroll-8.c
@@ -3,7 +3,7 @@
/* { dg-additional-options "-fno-tree-vectorize" { target amdgcn-*-* } } */
struct a {int a[7];};
-int t(struct a *a, int n)
+void t(struct a *a, int n)
{
int i;
for (i=0;i<n;i++)
--
2.42.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets
2023-11-21 23:27 [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Palmer Dabbelt
@ 2023-11-21 23:27 ` Palmer Dabbelt
2023-11-22 22:38 ` Jeff Law
2023-11-22 22:32 ` [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Jeff Law
2023-11-23 0:08 ` Andrew Pinski
2 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2023-11-21 23:27 UTC (permalink / raw)
To: gcc-patches; +Cc: Palmer Dabbelt
The vectorizer picks up these loops and disables unrolling on targets
with variable vector factors. That result in better code here, but it
trips up the unrolling tests. So just disable vectorization for these.
gcc/testsuite/ChangeLog:
PR target/112531
* gcc.dg/unroll-8.c: Disable vectorization on arm64 and riscv.
---
This also isn't tested.
---
gcc/testsuite/gcc.dg/unroll-8.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gcc/testsuite/gcc.dg/unroll-8.c b/gcc/testsuite/gcc.dg/unroll-8.c
index 06d32e56893..4465c620800 100644
--- a/gcc/testsuite/gcc.dg/unroll-8.c
+++ b/gcc/testsuite/gcc.dg/unroll-8.c
@@ -1,6 +1,15 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-rtl-loop2_unroll-details-blocks -funroll-loops" } */
+
+/*
+ * Targets that support variable-length vectorization don't unroll loops (see
+ * the "Disabling unrolling due to variable-length vectorization factor" out in
+ * tree-vect-loop. So disable tree vectorization for these targets, as it will
+ * interfere with the unrolling we're looking for below.
+ */
+/* { dg-additional-options "-fno-tree-vectorize" { target aarch64-*-* } } */
/* { dg-additional-options "-fno-tree-vectorize" { target amdgcn-*-* } } */
+/* { dg-additional-options "-fno-tree-vectorize" { target riscv*-*-* } } */
struct a {int a[7];};
void t(struct a *a, int n)
--
2.42.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets
2023-11-21 23:27 ` [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets Palmer Dabbelt
@ 2023-11-22 22:38 ` Jeff Law
2023-11-26 4:36 ` Andrew Pinski
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2023-11-22 22:38 UTC (permalink / raw)
To: Palmer Dabbelt, gcc-patches
On 11/21/23 16:27, Palmer Dabbelt wrote:
> The vectorizer picks up these loops and disables unrolling on targets
> with variable vector factors. That result in better code here, but it
> trips up the unrolling tests. So just disable vectorization for these.
>
> gcc/testsuite/ChangeLog:
>
> PR target/112531
> * gcc.dg/unroll-8.c: Disable vectorization on arm64 and riscv.
So probably the right check is to test for vector and
!vect_variable_length rather than doing something target specific for
aarch64/riscv
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets
2023-11-22 22:38 ` Jeff Law
@ 2023-11-26 4:36 ` Andrew Pinski
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2023-11-26 4:36 UTC (permalink / raw)
To: Jeff Law; +Cc: Palmer Dabbelt, gcc-patches
On Wed, Nov 22, 2023 at 4:18 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 11/21/23 16:27, Palmer Dabbelt wrote:
> > The vectorizer picks up these loops and disables unrolling on targets
> > with variable vector factors. That result in better code here, but it
> > trips up the unrolling tests. So just disable vectorization for these.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR target/112531
> > * gcc.dg/unroll-8.c: Disable vectorization on arm64 and riscv.
> So probably the right check is to test for vector and
> !vect_variable_length rather than doing something target specific for
> aarch64/riscv
Yes I think that would definitely be better.
>
> Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior
2023-11-21 23:27 [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Palmer Dabbelt
2023-11-21 23:27 ` [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets Palmer Dabbelt
@ 2023-11-22 22:32 ` Jeff Law
2023-11-23 0:08 ` Andrew Pinski
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2023-11-22 22:32 UTC (permalink / raw)
To: Palmer Dabbelt, gcc-patches
On 11/21/23 16:27, Palmer Dabbelt wrote:
> I was poking around with this test failure and noticed it was exercising
> undefined behavior. The return type doesn't matter for what's being
> tested, so just mark it as void.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/unroll-8.c: Remove UB.
I just reviewed the history of unroll-8, I don't think this compromises
the test's original intent. OK for the trunk.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior
2023-11-21 23:27 [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Palmer Dabbelt
2023-11-21 23:27 ` [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets Palmer Dabbelt
2023-11-22 22:32 ` [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Jeff Law
@ 2023-11-23 0:08 ` Andrew Pinski
2 siblings, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2023-11-23 0:08 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: gcc-patches
On Tue, Nov 21, 2023 at 3:29 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> I was poking around with this test failure and noticed it was exercising
> undefined behavior. The return type doesn't matter for what's being
> tested, so just mark it as void.
Just a quick note, this is NOT undefined behavior in C to return
without a value from a function which has a non-void return type. It
is only undefined if the value was used.
It is undefined behavior in C++ though for a fallthrough.
Yes there is a difference in the language. As Jeff said it does not
change what the testcase was/is testing but we should be clear in the
changelog that this is NOT undefined behavior.
Thanks,
Andrew Pinski
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/unroll-8.c: Remove UB.
> ---
> I didn't tes this, but it seems trivial enough that I'm just going to
> throw it at the bots and hope I'm right.
> ---
> gcc/testsuite/gcc.dg/unroll-8.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.dg/unroll-8.c b/gcc/testsuite/gcc.dg/unroll-8.c
> index 4388f47d4c7..06d32e56893 100644
> --- a/gcc/testsuite/gcc.dg/unroll-8.c
> +++ b/gcc/testsuite/gcc.dg/unroll-8.c
> @@ -3,7 +3,7 @@
> /* { dg-additional-options "-fno-tree-vectorize" { target amdgcn-*-* } } */
>
> struct a {int a[7];};
> -int t(struct a *a, int n)
> +void t(struct a *a, int n)
> {
> int i;
> for (i=0;i<n;i++)
> --
> 2.42.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-26 4:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 23:27 [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Palmer Dabbelt
2023-11-21 23:27 ` [PATCH 2/2] testsuite/unroll-8: Disable vectorization for varibale-factor targets Palmer Dabbelt
2023-11-22 22:38 ` Jeff Law
2023-11-26 4:36 ` Andrew Pinski
2023-11-22 22:32 ` [PATCH 1/2] testsuite/unroll-8: Avoid triggering undefined behavior Jeff Law
2023-11-23 0:08 ` Andrew Pinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).