[1/2] testsuite/unroll-8: Avoid triggering undefined behavior

Message ID 20231121232704.12336-3-palmer@rivosinc.com
State New
Headers
Series [1/2] testsuite/unroll-8: Avoid triggering undefined behavior |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed

Commit Message

Palmer Dabbelt Nov. 21, 2023, 11:27 p.m. UTC
  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(-)
  

Comments

Jeff Law Nov. 22, 2023, 10:32 p.m. UTC | #1
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
  
Andrew Pinski Nov. 23, 2023, 12:08 a.m. UTC | #2
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
>
  

Patch

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++)