testsuite: arm: Use effective-target for pr68620 and pr78041 tests

Message ID 20241031182605.3072159-1-torbjorn.svensson@foss.st.com
State Superseded
Headers
Series testsuite: arm: Use effective-target for pr68620 and pr78041 tests |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Torbjörn SVENSSON Oct. 31, 2024, 6:26 p.m. UTC
  Ok for trunk and releases/gcc-14?

--

Tests uses neon, so add effective-target arm_neon.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/pr68620.c: Use effective-target arm_neon.
	* gcc.target/arm/pr78041.c: Likewise.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
 gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)
  

Comments

Richard Earnshaw (lists) Nov. 4, 2024, 4:03 p.m. UTC | #1
On 31/10/2024 18:26, Torbjörn SVENSSON wrote:
> Ok for trunk and releases/gcc-14?
> 
> --
> 
> Tests uses neon, so add effective-target arm_neon.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/arm/pr68620.c: Use effective-target arm_neon.
> 	* gcc.target/arm/pr78041.c: Likewise.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>  gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
>  gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
> index 91878432b00..b4a44dab6ba 100644
> --- a/gcc/testsuite/gcc.target/arm/pr68620.c
> +++ b/gcc/testsuite/gcc.target/arm/pr68620.c
> @@ -1,8 +1,8 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
> -/* { dg-require-effective-target arm_fp_ok } */
> +/* { dg-require-effective-target arm_neon_ok } */

This seems reasonable, but ...

>  /* { dg-options "-mfp16-format=ieee" } */
> -/* { dg-add-options arm_fp } */
> +/* { dg-add-options arm_neon } */
>  
>  #include "arm_neon.h"
>  

... I don't think this is right.  It looks like the point of this test is to check that adding the #pragma to select a neon-based FPU enables a specific intrinsic.  That ought to work with the existing checks (at least, modulo changing the effective-target at the start).  But adding neon options on the command line shouldn't be needed.  What's the option combination that leads to a failure?

> diff --git a/gcc/testsuite/gcc.target/arm/pr78041.c b/gcc/testsuite/gcc.target/arm/pr78041.c
> index 340ab5cb433..418b7e09fc4 100644
> --- a/gcc/testsuite/gcc.target/arm/pr78041.c
> +++ b/gcc/testsuite/gcc.target/arm/pr78041.c
> @@ -1,6 +1,7 @@
>  /* { dg-require-effective-target arm_thumb2_ok } */
>  /* { dg-require-effective-target arm_neon_ok } */
> -/* { dg-options "-fno-inline -mthumb -O1 -mfpu=neon -w" } */
> +/* { dg-options "-fno-inline -mthumb -O1 -w" } */
> +/* { dg-add-options arm_neon } */
>  
>  extern void abort (void);

This bit is OK, though.

>
  
Torbjörn SVENSSON Nov. 4, 2024, 8:34 p.m. UTC | #2
On 2024-11-04 17:03, Richard Earnshaw (lists) wrote:
> On 31/10/2024 18:26, Torbjörn SVENSSON wrote:
>> Ok for trunk and releases/gcc-14?
>>
>> --
>>
>> Tests uses neon, so add effective-target arm_neon.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/arm/pr68620.c: Use effective-target arm_neon.
>> 	* gcc.target/arm/pr78041.c: Likewise.
>>
>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>> ---
>>   gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
>>   gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
>> index 91878432b00..b4a44dab6ba 100644
>> --- a/gcc/testsuite/gcc.target/arm/pr68620.c
>> +++ b/gcc/testsuite/gcc.target/arm/pr68620.c
>> @@ -1,8 +1,8 @@
>>   /* { dg-do compile } */
>>   /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
>> -/* { dg-require-effective-target arm_fp_ok } */
>> +/* { dg-require-effective-target arm_neon_ok } */
> 
> This seems reasonable, but ...
> 
>>   /* { dg-options "-mfp16-format=ieee" } */
>> -/* { dg-add-options arm_fp } */
>> +/* { dg-add-options arm_neon } */
>>   
>>   #include "arm_neon.h"
>>   
> 
> ... I don't think this is right.  It looks like the point of this test is to check that adding the #pragma to select a neon-based FPU enables a specific intrinsic.  That ought to work with the existing checks (at least, modulo changing the effective-target at the start).  But adding neon options on the command line shouldn't be needed.  What's the option combination that leads to a failure?

The arm_fp is not enough to ensure a valid architecture is in use.

If I do not switch from arm_fp to arm_neon, I get the test executed like 
this for m85hard:

.../bin/arm-none-eabi-gcc  .../gcc/testsuite/gcc.target/arm/pr68620.c 
-mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard 
-mfpu=fpv5-d16   -fdiagnostics-plain-output   -mfp16-format=ieee  -S 
-o pr68620.s

Obvious, -mfp16-format=ieee is valid for Cortex-M85, but it's not the 
same thing as that it supports neon/nenon-fp16. The check for arm_neon 
passes as there are flags that could be added that override and makes 
the check pass, i.e.:

.../bin/arm-none-eabi-gcc   -mthumb -march=armv8.1-m.main+mve 
-mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16 
-fdiagnostics-plain-output  -mfpu=neon -mfloat-abi=softfp -mcpu=unset 
-march=armv7-a -Wno-complain-wrong-lang -c     -o arm_neon_ok16723.o 
arm_neon_ok16723.c


Note: I get this when I am adding -mcpu=unset to the arm_neon_ok check. 
If I do not add the -mcpu=unset, the test is marked as unsupported due 
to a conflicting -march/-mcpu combination (this is what I'm trying to 
fix in the patchset that I will share in a few days, but without a 
dedicated fix, these tests will be listed as regressions).


So, in order for the test to pass, a compatible architecture must be 
selected and if we are not going to use the arm_neon check, then what 
should we us to get as wide coverage as possible?

Kind regards,
Torbjörn
  
Richard Earnshaw (lists) Nov. 5, 2024, 3:21 p.m. UTC | #3
On 04/11/2024 20:34, Torbjorn SVENSSON wrote:
> 
> 
> On 2024-11-04 17:03, Richard Earnshaw (lists) wrote:
>> On 31/10/2024 18:26, Torbjörn SVENSSON wrote:
>>> Ok for trunk and releases/gcc-14?
>>>
>>> -- 
>>>
>>> Tests uses neon, so add effective-target arm_neon.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/arm/pr68620.c: Use effective-target arm_neon.
>>>     * gcc.target/arm/pr78041.c: Likewise.
>>>
>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>> ---
>>>   gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
>>>   gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
>>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
>>> index 91878432b00..b4a44dab6ba 100644
>>> --- a/gcc/testsuite/gcc.target/arm/pr68620.c
>>> +++ b/gcc/testsuite/gcc.target/arm/pr68620.c
>>> @@ -1,8 +1,8 @@
>>>   /* { dg-do compile } */
>>>   /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
>>> -/* { dg-require-effective-target arm_fp_ok } */
>>> +/* { dg-require-effective-target arm_neon_ok } */
>>
>> This seems reasonable, but ...
>>
>>>   /* { dg-options "-mfp16-format=ieee" } */
>>> -/* { dg-add-options arm_fp } */
>>> +/* { dg-add-options arm_neon } */
>>>     #include "arm_neon.h"
>>>   
>>
>> ... I don't think this is right.  It looks like the point of this test is to check that adding the #pragma to select a neon-based FPU enables a specific intrinsic.  That ought to work with the existing checks (at least, modulo changing the effective-target at the start).  But adding neon options on the command line shouldn't be needed.  What's the option combination that leads to a failure?
> 
> The arm_fp is not enough to ensure a valid architecture is in use.
> 
> If I do not switch from arm_fp to arm_neon, I get the test executed like this for m85hard:
> 
> .../bin/arm-none-eabi-gcc  .../gcc/testsuite/gcc.target/arm/pr68620.c -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16   -fdiagnostics-plain-output   -mfp16-format=ieee  -S -o pr68620.s
> 
> Obvious, -mfp16-format=ieee is valid for Cortex-M85, but it's not the same thing as that it supports neon/nenon-fp16. The check for arm_neon passes as there are flags that could be added that override and makes the check pass, i.e.:
> 
> .../bin/arm-none-eabi-gcc   -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16 -fdiagnostics-plain-output  -mfpu=neon -mfloat-abi=softfp -mcpu=unset -march=armv7-a -Wno-complain-wrong-lang -c     -o arm_neon_ok16723.o arm_neon_ok16723.c
> 
> 
> Note: I get this when I am adding -mcpu=unset to the arm_neon_ok check. If I do not add the -mcpu=unset, the test is marked as unsupported due to a conflicting -march/-mcpu combination (this is what I'm trying to fix in the patchset that I will share in a few days, but without a dedicated fix, these tests will be listed as regressions).
> 
> 
> So, in order for the test to pass, a compatible architecture must be selected and if we are not going to use the arm_neon check, then what should we us to get as wide coverage as possible?

This is similar to the other neon tests I've just replied about: I'd just skip this (pr68620) test on m-profile for now.

R.

> 
> Kind regards,
> Torbjörn
  
Richard Earnshaw (lists) Nov. 5, 2024, 3:37 p.m. UTC | #4
On 05/11/2024 15:21, Richard Earnshaw (lists) wrote:
> On 04/11/2024 20:34, Torbjorn SVENSSON wrote:
>>
>>
>> On 2024-11-04 17:03, Richard Earnshaw (lists) wrote:
>>> On 31/10/2024 18:26, Torbjörn SVENSSON wrote:
>>>> Ok for trunk and releases/gcc-14?
>>>>
>>>> -- 
>>>>
>>>> Tests uses neon, so add effective-target arm_neon.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.target/arm/pr68620.c: Use effective-target arm_neon.
>>>>     * gcc.target/arm/pr78041.c: Likewise.
>>>>
>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>> ---
>>>>   gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
>>>>   gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
>>>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
>>>> index 91878432b00..b4a44dab6ba 100644
>>>> --- a/gcc/testsuite/gcc.target/arm/pr68620.c
>>>> +++ b/gcc/testsuite/gcc.target/arm/pr68620.c
>>>> @@ -1,8 +1,8 @@
>>>>   /* { dg-do compile } */
>>>>   /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
>>>> -/* { dg-require-effective-target arm_fp_ok } */
>>>> +/* { dg-require-effective-target arm_neon_ok } */
>>>
>>> This seems reasonable, but ...
>>>
>>>>   /* { dg-options "-mfp16-format=ieee" } */
>>>> -/* { dg-add-options arm_fp } */
>>>> +/* { dg-add-options arm_neon } */
>>>>     #include "arm_neon.h"
>>>>   
>>>
>>> ... I don't think this is right.  It looks like the point of this test is to check that adding the #pragma to select a neon-based FPU enables a specific intrinsic.  That ought to work with the existing checks (at least, modulo changing the effective-target at the start).  But adding neon options on the command line shouldn't be needed.  What's the option combination that leads to a failure?
>>
>> The arm_fp is not enough to ensure a valid architecture is in use.
>>
>> If I do not switch from arm_fp to arm_neon, I get the test executed like this for m85hard:
>>
>> .../bin/arm-none-eabi-gcc  .../gcc/testsuite/gcc.target/arm/pr68620.c -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16   -fdiagnostics-plain-output   -mfp16-format=ieee  -S -o pr68620.s
>>
>> Obvious, -mfp16-format=ieee is valid for Cortex-M85, but it's not the same thing as that it supports neon/nenon-fp16. The check for arm_neon passes as there are flags that could be added that override and makes the check pass, i.e.:
>>
>> .../bin/arm-none-eabi-gcc   -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16 -fdiagnostics-plain-output  -mfpu=neon -mfloat-abi=softfp -mcpu=unset -march=armv7-a -Wno-complain-wrong-lang -c     -o arm_neon_ok16723.o arm_neon_ok16723.c
>>
>>
>> Note: I get this when I am adding -mcpu=unset to the arm_neon_ok check. If I do not add the -mcpu=unset, the test is marked as unsupported due to a conflicting -march/-mcpu combination (this is what I'm trying to fix in the patchset that I will share in a few days, but without a dedicated fix, these tests will be listed as regressions).
>>
>>
>> So, in order for the test to pass, a compatible architecture must be selected and if we are not going to use the arm_neon check, then what should we us to get as wide coverage as possible?
> 
> This is similar to the other neon tests I've just replied about: I'd just skip this (pr68620) test on m-profile for now.
> 
> R.
> 
>>
>> Kind regards,
>> Torbjörn
> 


I've just realised there's another way we could change this test which stays within the spirit of compiling a function with additional capabilities.

 /* { dg-do compile } */
 /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
 /* { dg-require-effective-target arm_arch_v7a_ok } */
 /* { dg-options "-mfp16-format=ieee -mfpu=auto -mfloat-abi=softfp" } */
 /* { dg-add-options arm_arch_v7a } */

This will have the effect of forcing the architecture to a known baseline and from there we can then override the FPU with a neon extension.

R.
  
Torbjörn SVENSSON Nov. 5, 2024, 5:55 p.m. UTC | #5
On 2024-11-05 16:37, Richard Earnshaw wrote:
> On 05/11/2024 15:21, Richard Earnshaw (lists) wrote:
>> On 04/11/2024 20:34, Torbjorn SVENSSON wrote:
>>>
>>>
>>> On 2024-11-04 17:03, Richard Earnshaw (lists) wrote:
>>>> On 31/10/2024 18:26, Torbjörn SVENSSON wrote:
>>>>> Ok for trunk and releases/gcc-14?
>>>>>
>>>>> -- 
>>>>>
>>>>> Tests uses neon, so add effective-target arm_neon.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>>      * gcc.target/arm/pr68620.c: Use effective-target arm_neon.
>>>>>      * gcc.target/arm/pr78041.c: Likewise.
>>>>>
>>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>>> ---
>>>>>    gcc/testsuite/gcc.target/arm/pr68620.c | 4 ++--
>>>>>    gcc/testsuite/gcc.target/arm/pr78041.c | 3 ++-
>>>>>    2 files changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
>>>>> index 91878432b00..b4a44dab6ba 100644
>>>>> --- a/gcc/testsuite/gcc.target/arm/pr68620.c
>>>>> +++ b/gcc/testsuite/gcc.target/arm/pr68620.c
>>>>> @@ -1,8 +1,8 @@
>>>>>    /* { dg-do compile } */
>>>>>    /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
>>>>> -/* { dg-require-effective-target arm_fp_ok } */
>>>>> +/* { dg-require-effective-target arm_neon_ok } */
>>>>
>>>> This seems reasonable, but ...
>>>>
>>>>>    /* { dg-options "-mfp16-format=ieee" } */
>>>>> -/* { dg-add-options arm_fp } */
>>>>> +/* { dg-add-options arm_neon } */
>>>>>      #include "arm_neon.h"
>>>>>    
>>>>
>>>> ... I don't think this is right.  It looks like the point of this test is to check that adding the #pragma to select a neon-based FPU enables a specific intrinsic.  That ought to work with the existing checks (at least, modulo changing the effective-target at the start).  But adding neon options on the command line shouldn't be needed.  What's the option combination that leads to a failure?
>>>
>>> The arm_fp is not enough to ensure a valid architecture is in use.
>>>
>>> If I do not switch from arm_fp to arm_neon, I get the test executed like this for m85hard:
>>>
>>> .../bin/arm-none-eabi-gcc  .../gcc/testsuite/gcc.target/arm/pr68620.c -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16   -fdiagnostics-plain-output   -mfp16-format=ieee  -S -o pr68620.s
>>>
>>> Obvious, -mfp16-format=ieee is valid for Cortex-M85, but it's not the same thing as that it supports neon/nenon-fp16. The check for arm_neon passes as there are flags that could be added that override and makes the check pass, i.e.:
>>>
>>> .../bin/arm-none-eabi-gcc   -mthumb -march=armv8.1-m.main+mve -mcpu=cortex-m55 -mfloat-abi=hard -mfpu=fpv5-d16 -fdiagnostics-plain-output  -mfpu=neon -mfloat-abi=softfp -mcpu=unset -march=armv7-a -Wno-complain-wrong-lang -c     -o arm_neon_ok16723.o arm_neon_ok16723.c
>>>
>>>
>>> Note: I get this when I am adding -mcpu=unset to the arm_neon_ok check. If I do not add the -mcpu=unset, the test is marked as unsupported due to a conflicting -march/-mcpu combination (this is what I'm trying to fix in the patchset that I will share in a few days, but without a dedicated fix, these tests will be listed as regressions).
>>>
>>>
>>> So, in order for the test to pass, a compatible architecture must be selected and if we are not going to use the arm_neon check, then what should we us to get as wide coverage as possible?
>>
>> This is similar to the other neon tests I've just replied about: I'd just skip this (pr68620) test on m-profile for now.
>>
>> R.
>>
>>>
>>> Kind regards,
>>> Torbjörn
>>
> 
> 
> I've just realised there's another way we could change this test which stays within the spirit of compiling a function with additional capabilities.
> 
>   /* { dg-do compile } */
>   /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
>   /* { dg-require-effective-target arm_arch_v7a_ok } */
>   /* { dg-options "-mfp16-format=ieee -mfpu=auto -mfloat-abi=softfp" } */
>   /* { dg-add-options arm_arch_v7a } */
> 
> This will have the effect of forcing the architecture to a known baseline and from there we can then override the FPU with a neon extension.

The reason why I did not go this way from the start (yes I was 
considering it) was that I'm not sure if it was enough to check this for 
the v7a architecture or if neon should be tested in some other 
permutation in addition to v7a.

Anyway, I think it's better to do a v7a variant than not testing it at 
all, so I'll prepare an updated patch with this (I'll do the same for 
the other patch that you reviewed earlier today too).

Kind regards,
Torbjörn
  

Patch

diff --git a/gcc/testsuite/gcc.target/arm/pr68620.c b/gcc/testsuite/gcc.target/arm/pr68620.c
index 91878432b00..b4a44dab6ba 100644
--- a/gcc/testsuite/gcc.target/arm/pr68620.c
+++ b/gcc/testsuite/gcc.target/arm/pr68620.c
@@ -1,8 +1,8 @@ 
 /* { dg-do compile } */
 /* { dg-skip-if "-mpure-code supports M-profile without Neon only" { *-*-* } { "-mpure-code" } } */
-/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-require-effective-target arm_neon_ok } */
 /* { dg-options "-mfp16-format=ieee" } */
-/* { dg-add-options arm_fp } */
+/* { dg-add-options arm_neon } */
 
 #include "arm_neon.h"
 
diff --git a/gcc/testsuite/gcc.target/arm/pr78041.c b/gcc/testsuite/gcc.target/arm/pr78041.c
index 340ab5cb433..418b7e09fc4 100644
--- a/gcc/testsuite/gcc.target/arm/pr78041.c
+++ b/gcc/testsuite/gcc.target/arm/pr78041.c
@@ -1,6 +1,7 @@ 
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-fno-inline -mthumb -O1 -mfpu=neon -w" } */
+/* { dg-options "-fno-inline -mthumb -O1 -w" } */
+/* { dg-add-options arm_neon } */
 
 extern void abort (void);