[PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Saurabh Jha]

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

Hey,

Previously, this test was added to fix this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check the compilation options before using them, leading to errors.

This patch fixes the test by first checking whether it can use the options before using them.

Tested for arm-none-eabi and found no regressions. The output of check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:

Before:
# of expected passes          5963
# of unexpected failures      64

After:
# of expected passes          5964
# of unexpected failures      63

Ok for master?

Regards,
Saurabh

gcc/testsuite/ChangeLog:

        * gcc.target/arm/pr112337.c: Check whether we can use the compilation options before using them.

[-- Attachment #2: rb18229.patch --]
[-- Type: application/octet-stream, Size: 978 bytes --]

From b65084c05c165af6fce22e5de19eb508d9ce0ff3 Mon Sep 17 00:00:00 2001
From: Saurabh Jha <saujha01@e130340.arm.com>
Date: Tue, 30 Jan 2024 15:03:36 +0000
Subject: [PATCH] Fix testcase pr112337.c to check the options first

---
 gcc/testsuite/gcc.target/arm/pr112337.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/pr112337.c b/gcc/testsuite/gcc.target/arm/pr112337.c
index 5dacf0aa4f8..b664229b210 100644
--- a/gcc/testsuite/gcc.target/arm/pr112337.c
+++ b/gcc/testsuite/gcc.target/arm/pr112337.c
@@ -1,5 +1,8 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
+/* { dg-require-effective-target arm_hard_ok } */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-options "-O2 -mfloat-abi=hard" } */
+/* { dg-add-options arm_v8_1m_mve } */
 
 #pragma GCC arm "arm_mve_types.h"
 int32x4_t h(void *p) { return __builtin_mve_vldrwq_sv4si(p); }
-- 
2.43.0

Re: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Saurabh Jha]

Ping. I also don't have commit access so can someone please commit on my behalf.

________________________________________
From: Saurabh Jha
Sent: Tuesday, January 30, 2024 5:07 PM
To: Richard Sandiford via Gcc-patches; Richard Sandiford; Kyrylo Tkachov; Richard Earnshaw
Subject: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

Hey,

Previously, this test was added to fix this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check the compilation options before using them, leading to errors.

This patch fixes the test by first checking whether it can use the options before using them.

Tested for arm-none-eabi and found no regressions. The output of check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:

Before:
# of expected passes          5963
# of unexpected failures      64

After:
# of expected passes          5964
# of unexpected failures      63

Ok for master?

Regards,
Saurabh

gcc/testsuite/ChangeLog:

        * gcc.target/arm/pr112337.c: Check whether we can use the compilation options before using them.

Re: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Richard Earnshaw (lists)]

On 30/01/2024 17:07, Saurabh Jha wrote:
> Hey,
> 
> Previously, this test was added to fix this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check the compilation options before using them, leading to errors.
> 
> This patch fixes the test by first checking whether it can use the options before using them.
> 
> Tested for arm-none-eabi and found no regressions. The output of check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:
> 
> Before:
> # of expected passes          5963
> # of unexpected failures      64
> 
> After:
> # of expected passes          5964
> # of unexpected failures      63
> 
> Ok for master?
> 
> Regards,
> Saurabh
> 
> gcc/testsuite/ChangeLog:
> 
>         * gcc.target/arm/pr112337.c: Check whether we can use the compilation options before using them.

My apologies for missing this earlier.  It didn't show up in patchwork. That's most likely because the attachment is a binary blob instead of text/plain.  That also means that the Linaro CI system hasn't seen this patch either.  Please can you fix your mailer to add plain text patch files.

-/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
+/* { dg-require-effective-target arm_hard_ok } */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-options "-O2 -mfloat-abi=hard" } */
+/* { dg-add-options arm_v8_1m_mve } */

This is moving in the right direction, but it adds more than necessary now: checking for, and adding -mfloat-abi=hard is not necessary any more as arm_v8_1m_mve_ok will work out what float-abi flags are needed to make the options work. (What's more, it will prevent the test from running if the base configuration of the compiler is incompatible with the hard float ABI, which is more than we need.).

So please can you re-spin removing the hard-float check and removing that from dg-options.

Thanks,
R.

Re: [PATCH v2] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Saurabh Jha]

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


On 2/9/2024 2:57 PM, Richard Earnshaw (lists) wrote:
> On 30/01/2024 17:07, Saurabh Jha wrote:
>> Hey,
>>
>> Previously, this test was added to fix this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check the compilation options before using them, leading to errors.
>>
>> This patch fixes the test by first checking whether it can use the options before using them.
>>
>> Tested for arm-none-eabi and found no regressions. The output of check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:
>>
>> Before:
>> # of expected passes          5963
>> # of unexpected failures      64
>>
>> After:
>> # of expected passes          5964
>> # of unexpected failures      63
>>
>> Ok for master?
>>
>> Regards,
>> Saurabh
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * gcc.target/arm/pr112337.c: Check whether we can use the compilation options before using them.
> My apologies for missing this earlier.  It didn't show up in patchwork. That's most likely because the attachment is a binary blob instead of text/plain.  That also means that the Linaro CI system hasn't seen this patch either.  Please can you fix your mailer to add plain text patch files.
>
> -/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
> +/* { dg-require-effective-target arm_hard_ok } */
> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> +/* { dg-options "-O2 -mfloat-abi=hard" } */
> +/* { dg-add-options arm_v8_1m_mve } */
>
> This is moving in the right direction, but it adds more than necessary now: checking for, and adding -mfloat-abi=hard is not necessary any more as arm_v8_1m_mve_ok will work out what float-abi flags are needed to make the options work. (What's more, it will prevent the test from running if the base configuration of the compiler is incompatible with the hard float ABI, which is more than we need.).
>
> So please can you re-spin removing the hard-float check and removing that from dg-options.
>
> Thanks,
> R.

Hi Richard,

Agreed with your comments. Please find the patch with the suggested 
changes attached.

Regards,

Saurabh


[-- Attachment #2: rb18229.patch --]
[-- Type: text/plain, Size: 909 bytes --]

From 1c92c94074449929f40cea99a6450bcde3aec12f Mon Sep 17 00:00:00 2001
From: Saurabh Jha <saujha01@e130340.arm.com>
Date: Tue, 30 Jan 2024 15:03:36 +0000
Subject: [PATCH] Fix testcase pr112337.c to check the options first

---
 gcc/testsuite/gcc.target/arm/pr112337.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/pr112337.c b/gcc/testsuite/gcc.target/arm/pr112337.c
index 5dacf0aa4f8..10b7881b9f9 100644
--- a/gcc/testsuite/gcc.target/arm/pr112337.c
+++ b/gcc/testsuite/gcc.target/arm/pr112337.c
@@ -1,5 +1,7 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
+/* { dg-options "-O2" } */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
 
 #pragma GCC arm "arm_mve_types.h"
 int32x4_t h(void *p) { return __builtin_mve_vldrwq_sv4si(p); }
-- 
2.43.0

Re: [PATCH v2] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Saurabh Jha]

Ping

On 2/19/2024 10:11 AM, Saurabh Jha wrote:
>
> On 2/9/2024 2:57 PM, Richard Earnshaw (lists) wrote:
>> On 30/01/2024 17:07, Saurabh Jha wrote:
>>> Hey,
>>>
>>> Previously, this test was added to fix this bug: 
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did 
>>> not check the compilation options before using them, leading to errors.
>>>
>>> This patch fixes the test by first checking whether it can use the 
>>> options before using them.
>>>
>>> Tested for arm-none-eabi and found no regressions. The output of 
>>> check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:
>>>
>>> Before:
>>> # of expected passes          5963
>>> # of unexpected failures      64
>>>
>>> After:
>>> # of expected passes          5964
>>> # of unexpected failures      63
>>>
>>> Ok for master?
>>>
>>> Regards,
>>> Saurabh
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * gcc.target/arm/pr112337.c: Check whether we can use the 
>>> compilation options before using them.
>> My apologies for missing this earlier.  It didn't show up in 
>> patchwork. That's most likely because the attachment is a binary blob 
>> instead of text/plain.  That also means that the Linaro CI system 
>> hasn't seen this patch either.  Please can you fix your mailer to add 
>> plain text patch files.
>>
>> -/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp 
>> -mfloat-abi=hard" } */
>> +/* { dg-require-effective-target arm_hard_ok } */
>> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
>> +/* { dg-options "-O2 -mfloat-abi=hard" } */
>> +/* { dg-add-options arm_v8_1m_mve } */
>>
>> This is moving in the right direction, but it adds more than 
>> necessary now: checking for, and adding -mfloat-abi=hard is not 
>> necessary any more as arm_v8_1m_mve_ok will work out what float-abi 
>> flags are needed to make the options work. (What's more, it will 
>> prevent the test from running if the base configuration of the 
>> compiler is incompatible with the hard float ABI, which is more than 
>> we need.).
>>
>> So please can you re-spin removing the hard-float check and removing 
>> that from dg-options.
>>
>> Thanks,
>> R.
>
> Hi Richard,
>
> Agreed with your comments. Please find the patch with the suggested 
> changes attached.
>
> Regards,
>
> Saurabh
>

Re: [PATCH v2] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

[Richard Earnshaw (lists)]

On 19/02/2024 10:11, Saurabh Jha wrote:
> 
> On 2/9/2024 2:57 PM, Richard Earnshaw (lists) wrote:
>> On 30/01/2024 17:07, Saurabh Jha wrote:
>>> Hey,
>>>
>>> Previously, this test was added to fix this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check the compilation options before using them, leading to errors.
>>>
>>> This patch fixes the test by first checking whether it can use the options before using them.
>>>
>>> Tested for arm-none-eabi and found no regressions. The output of check-gcc with RUNTESTFLAGS="arm.exp=*" changed like this:
>>>
>>> Before:
>>> # of expected passes          5963
>>> # of unexpected failures      64
>>>
>>> After:
>>> # of expected passes          5964
>>> # of unexpected failures      63
>>>
>>> Ok for master?
>>>
>>> Regards,
>>> Saurabh
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * gcc.target/arm/pr112337.c: Check whether we can use the compilation options before using them.
>> My apologies for missing this earlier.  It didn't show up in patchwork. That's most likely because the attachment is a binary blob instead of text/plain.  That also means that the Linaro CI system hasn't seen this patch either.  Please can you fix your mailer to add plain text patch files.
>>
>> -/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
>> +/* { dg-require-effective-target arm_hard_ok } */
>> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
>> +/* { dg-options "-O2 -mfloat-abi=hard" } */
>> +/* { dg-add-options arm_v8_1m_mve } */
>>
>> This is moving in the right direction, but it adds more than necessary now: checking for, and adding -mfloat-abi=hard is not necessary any more as arm_v8_1m_mve_ok will work out what float-abi flags are needed to make the options work. (What's more, it will prevent the test from running if the base configuration of the compiler is incompatible with the hard float ABI, which is more than we need.).
>>
>> So please can you re-spin removing the hard-float check and removing that from dg-options.
>>
>> Thanks,
>> R.
> 
> Hi Richard,
> 
> Agreed with your comments. Please find the patch with the suggested changes attached.
> 
> Regards,
> 
> Saurabh
> 


Thanks, I've pushed this.  Next time, please can you put the commit message inside the patch, so that I can apply things automatically.  Eg: 

From 1c92c94074449929f40cea99a6450bcde3aec12f Mon Sep 17 00:00:00 2001
From: Saurabh Jha <saujha01@e130340.arm.com>
Date: Tue, 30 Jan 2024 15:03:36 +0000
Subject: [PATCH] Fix testcase pr112337.c to check the options [PR112337]

gcc.target/arm/pr112337.c was failing to validate that adding MVE options
was compatible with the test environment, so add the missing checks.

gcc/testsuite/ChangeLog:

	PR target/112337
	* gcc.target/arm/pr112337.c: Check for, then use the right MVE
	options.
	
---
 gcc/testsuite/gcc.target/arm/pr112337.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/pr112337.c b/gcc/testsuite/gcc.target/arm/pr112337.c

...