testsuite: arm: Use effective-target for memset-inline* tests

Message ID 20241024085037.4064056-1-torbjorn.svensson@foss.st.com
State Changes Requested
Headers
Series testsuite: arm: Use effective-target for memset-inline* tests |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test 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

Torbjorn SVENSSON Oct. 24, 2024, 8:50 a.m. UTC
  Ok for trunk and releases/gcc-14?

--

As these tests are set to execute and require neon hardware to do so,
add the missing dg-require-effective-target arm_neon_hw.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/memset-inline-4.c: Use effective-target
	arm_neon_hw.
	* gcc.target/arm/memset-inline-5.c: Likewise.
	* gcc.target/arm/memset-inline-6.c: Likewise.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 gcc/testsuite/gcc.target/arm/memset-inline-4.c | 1 +
 gcc/testsuite/gcc.target/arm/memset-inline-5.c | 1 +
 gcc/testsuite/gcc.target/arm/memset-inline-6.c | 1 +
 3 files changed, 3 insertions(+)
  

Comments

Richard Earnshaw (lists) Nov. 1, 2024, 6:40 p.m. UTC | #1
On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
> Ok for trunk and releases/gcc-14?
> 
> --
> 
> As these tests are set to execute and require neon hardware to do so,
> add the missing dg-require-effective-target arm_neon_hw.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/arm/memset-inline-4.c: Use effective-target
> 	arm_neon_hw.
> 	* gcc.target/arm/memset-inline-5.c: Likewise.
> 	* gcc.target/arm/memset-inline-6.c: Likewise.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>

These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write

/* { dg-do run { arm_neon_hw } } */

instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.

Would you mind testing that out, please?

R.
  
Torbjorn SVENSSON Nov. 1, 2024, 7:18 p.m. UTC | #2
On 2024-11-01 19:40, Richard Earnshaw (lists) wrote:
> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>> Ok for trunk and releases/gcc-14?
>>
>> --
>>
>> As these tests are set to execute and require neon hardware to do so,
>> add the missing dg-require-effective-target arm_neon_hw.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/arm/memset-inline-4.c: Use effective-target
>> 	arm_neon_hw.
>> 	* gcc.target/arm/memset-inline-5.c: Likewise.
>> 	* gcc.target/arm/memset-inline-6.c: Likewise.
>>
>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> 
> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
> 
> /* { dg-do run { arm_neon_hw } } */
> 
> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
> 
> Would you mind testing that out, please?

Using

/* { dg-do run { target arm_neon_hw } } */

works, but I'm not entirely sure sure what the difference is.
With both solutions (dg-run and dg-require-effective-target), Cortex-A7 
tests with -mfloat-abi=soft marks the tests as unsupported. All other 
targets that I test is unchanged.

I suppose that the reason why there is no difference between the two 
suggested solutions is that the tests are skipped if 
arm_tune_string_ops_prefer_neon is not true (the line above my addition 
in the patch).

Let me know if you prefer the dg-require-effective-target or your 
solution. If we go for your solution, should I send a v2 with it?

Kind regards,
Torbjörn
  
Richard Earnshaw (lists) Nov. 4, 2024, 11:44 a.m. UTC | #3
On 01/11/2024 19:18, Torbjorn SVENSSON wrote:
> 
> 
> On 2024-11-01 19:40, Richard Earnshaw (lists) wrote:
>> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>>> Ok for trunk and releases/gcc-14?
>>>
>>> -- 
>>>
>>> As these tests are set to execute and require neon hardware to do so,
>>> add the missing dg-require-effective-target arm_neon_hw.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/arm/memset-inline-4.c: Use effective-target
>>>     arm_neon_hw.
>>>     * gcc.target/arm/memset-inline-5.c: Likewise.
>>>     * gcc.target/arm/memset-inline-6.c: Likewise.
>>>
>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>
>> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
>>
>> /* { dg-do run { arm_neon_hw } } */
>>
>> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
>>
>> Would you mind testing that out, please?
> 
> Using
> 
> /* { dg-do run { target arm_neon_hw } } */
> 
> works, but I'm not entirely sure sure what the difference is.
> With both solutions (dg-run and dg-require-effective-target), Cortex-A7 tests with -mfloat-abi=soft marks the tests as unsupported. All other targets that I test is unchanged.
> 
> I suppose that the reason why there is no difference between the two suggested solutions is that the tests are skipped if arm_tune_string_ops_prefer_neon is not true (the line above my addition in the patch).
> 
> Let me know if you prefer the dg-require-effective-target or your solution. If we go for your solution, should I send a v2 with it?
> 
> Kind regards,
> Torbjörn
> 

Yeah, this is going to be a bit more invasive than I anticipated.  But I do think we can make this test more widely applicable.  It's going to require a few steps though.

1:  I think we need to fix target-supports.exp(check_effective_target_arm_neon_ok_noncache) to add -mcpu=unset whenever there's a -march flag in a variant.
2:  We need to change the test to not use the skip-if, but to require an effective target of arm_neon (hence the need for the above)
3:  We need to add a -mtune option as well to ensure that the test does use neon instructions for memcpy operations: adding -mtune=cortex-a7 via additional-options should achieve that.

With those changes the compile tests should kick in whenever we can successfully override the default options with something appropriate and the execution tests should kick in whenever we have neon hardware (the test is not about whether the performance it great, just whether or not it works).

R.
  
Torbjorn SVENSSON Nov. 4, 2024, 12:28 p.m. UTC | #4
On 2024-11-04 12:44, Richard Earnshaw (lists) wrote:
> On 01/11/2024 19:18, Torbjorn SVENSSON wrote:
>>
>>
>> On 2024-11-01 19:40, Richard Earnshaw (lists) wrote:
>>> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>>>> Ok for trunk and releases/gcc-14?
>>>>
>>>> -- 
>>>>
>>>> As these tests are set to execute and require neon hardware to do so,
>>>> add the missing dg-require-effective-target arm_neon_hw.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>      * gcc.target/arm/memset-inline-4.c: Use effective-target
>>>>      arm_neon_hw.
>>>>      * gcc.target/arm/memset-inline-5.c: Likewise.
>>>>      * gcc.target/arm/memset-inline-6.c: Likewise.
>>>>
>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>
>>> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
>>>
>>> /* { dg-do run { arm_neon_hw } } */
>>>
>>> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
>>>
>>> Would you mind testing that out, please?
>>
>> Using
>>
>> /* { dg-do run { target arm_neon_hw } } */
>>
>> works, but I'm not entirely sure sure what the difference is.
>> With both solutions (dg-run and dg-require-effective-target), Cortex-A7 tests with -mfloat-abi=soft marks the tests as unsupported. All other targets that I test is unchanged.
>>
>> I suppose that the reason why there is no difference between the two suggested solutions is that the tests are skipped if arm_tune_string_ops_prefer_neon is not true (the line above my addition in the patch).
>>
>> Let me know if you prefer the dg-require-effective-target or your solution. If we go for your solution, should I send a v2 with it?
>>
>> Kind regards,
>> Torbjörn
>>
> 
> Yeah, this is going to be a bit more invasive than I anticipated.  But I do think we can make this test more widely applicable.  It's going to require a few steps though.
> 
> 1:  I think we need to fix target-supports.exp(check_effective_target_arm_neon_ok_noncache) to add -mcpu=unset whenever there's a -march flag in a variant.
> 2:  We need to change the test to not use the skip-if, but to require an effective target of arm_neon (hence the need for the above)
> 3:  We need to add a -mtune option as well to ensure that the test does use neon instructions for memcpy operations: adding -mtune=cortex-a7 via additional-options should achieve that.
> 
> With those changes the compile tests should kick in whenever we can successfully override the default options with something appropriate and the execution tests should kick in whenever we have neon hardware (the test is not about whether the performance it great, just whether or not it works).

Can we do this as a 2 parted process?
With the first part, I would like to avoid running it in configuration 
where it's known to fail and this part should ideally be retrofired to 
gcc14.
For the 2nd part, I agree with the steps above, but that will only be 
applicable for gcc15, unless you also want to backport the 
-mcpu=unset/-march=unset feature to gcc14 too.

Can we do the first part in this patch and then come back to part 2 
afterwards?

Kind regards,
Torbjörn
  
Richard Earnshaw (lists) Nov. 4, 2024, 1:37 p.m. UTC | #5
On 04/11/2024 12:28, Torbjorn SVENSSON wrote:
> 
> 
> On 2024-11-04 12:44, Richard Earnshaw (lists) wrote:
>> On 01/11/2024 19:18, Torbjorn SVENSSON wrote:
>>>
>>>
>>> On 2024-11-01 19:40, Richard Earnshaw (lists) wrote:
>>>> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>>>>> Ok for trunk and releases/gcc-14?
>>>>>
>>>>> -- 
>>>>>
>>>>> As these tests are set to execute and require neon hardware to do so,
>>>>> add the missing dg-require-effective-target arm_neon_hw.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>>      * gcc.target/arm/memset-inline-4.c: Use effective-target
>>>>>      arm_neon_hw.
>>>>>      * gcc.target/arm/memset-inline-5.c: Likewise.
>>>>>      * gcc.target/arm/memset-inline-6.c: Likewise.
>>>>>
>>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>>
>>>> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
>>>>
>>>> /* { dg-do run { arm_neon_hw } } */
>>>>
>>>> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
>>>>
>>>> Would you mind testing that out, please?
>>>
>>> Using
>>>
>>> /* { dg-do run { target arm_neon_hw } } */
>>>
>>> works, but I'm not entirely sure sure what the difference is.
>>> With both solutions (dg-run and dg-require-effective-target), Cortex-A7 tests with -mfloat-abi=soft marks the tests as unsupported. All other targets that I test is unchanged.
>>>
>>> I suppose that the reason why there is no difference between the two suggested solutions is that the tests are skipped if arm_tune_string_ops_prefer_neon is not true (the line above my addition in the patch).
>>>
>>> Let me know if you prefer the dg-require-effective-target or your solution. If we go for your solution, should I send a v2 with it?
>>>
>>> Kind regards,
>>> Torbjörn
>>>
>>
>> Yeah, this is going to be a bit more invasive than I anticipated.  But I do think we can make this test more widely applicable.  It's going to require a few steps though.
>>
>> 1:  I think we need to fix target-supports.exp(check_effective_target_arm_neon_ok_noncache) to add -mcpu=unset whenever there's a -march flag in a variant.
>> 2:  We need to change the test to not use the skip-if, but to require an effective target of arm_neon (hence the need for the above)
>> 3:  We need to add a -mtune option as well to ensure that the test does use neon instructions for memcpy operations: adding -mtune=cortex-a7 via additional-options should achieve that.
>>
>> With those changes the compile tests should kick in whenever we can successfully override the default options with something appropriate and the execution tests should kick in whenever we have neon hardware (the test is not about whether the performance it great, just whether or not it works).
> 
> Can we do this as a 2 parted process?
> With the first part, I would like to avoid running it in configuration where it's known to fail and this part should ideally be retrofired to gcc14.

I'd prefer not to do that as it creates a user-visible change in a dot release of the compiler.

> For the 2nd part, I agree with the steps above, but that will only be applicable for gcc15, unless you also want to backport the -mcpu=unset/-march=unset feature to gcc14 too.
> 
> Can we do the first part in this patch and then come back to part 2 afterwards?

I'd be happy for completely separate patches to go in on gcc-14 and on trunk - feel free to apply your original patch to the gcc-14 branch while we work on a full fix for gcc-15.


> 
> Kind regards,
> Torbjörn

R.
  
Richard Earnshaw (lists) Nov. 4, 2024, 2:41 p.m. UTC | #6
On 01/11/2024 18:40, Richard Earnshaw (lists) wrote:
> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>> Ok for trunk and releases/gcc-14?
>>
>> --
>>
>> As these tests are set to execute and require neon hardware to do so,
>> add the missing dg-require-effective-target arm_neon_hw.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/arm/memset-inline-4.c: Use effective-target
>> 	arm_neon_hw.
>> 	* gcc.target/arm/memset-inline-5.c: Likewise.
>> 	* gcc.target/arm/memset-inline-6.c: Likewise.
>>
>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> 
> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
> 
> /* { dg-do run { arm_neon_hw } } */
> 
> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.

I've been doing some more digging into this and it looks as though I was mistaken about this fall-back behaviour.  Firstly, I'd missed out the 'target' keyword, the code would need to be

/* { dg-do run { target arm_neon_hw } } */

but this still doesn't work as I expected.  Instead the test is skipped entirely if the selector fails to match.  So we shouldn't combine assembly scan tests and execution tests in a single file, but need to have separate tests: one for execution and one for assembly output.

Sorry for the confusion,

R.
  
Torbjorn SVENSSON Nov. 4, 2024, 8:38 p.m. UTC | #7
On 2024-11-04 14:37, Richard Earnshaw (lists) wrote:
> On 04/11/2024 12:28, Torbjorn SVENSSON wrote:
>>
>>
>> On 2024-11-04 12:44, Richard Earnshaw (lists) wrote:
>>> On 01/11/2024 19:18, Torbjorn SVENSSON wrote:
>>>>
>>>>
>>>> On 2024-11-01 19:40, Richard Earnshaw (lists) wrote:
>>>>> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>>>>>> Ok for trunk and releases/gcc-14?
>>>>>>
>>>>>> -- 
>>>>>>
>>>>>> As these tests are set to execute and require neon hardware to do so,
>>>>>> add the missing dg-require-effective-target arm_neon_hw.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>>       * gcc.target/arm/memset-inline-4.c: Use effective-target
>>>>>>       arm_neon_hw.
>>>>>>       * gcc.target/arm/memset-inline-5.c: Likewise.
>>>>>>       * gcc.target/arm/memset-inline-6.c: Likewise.
>>>>>>
>>>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>>>
>>>>> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
>>>>>
>>>>> /* { dg-do run { arm_neon_hw } } */
>>>>>
>>>>> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
>>>>>
>>>>> Would you mind testing that out, please?
>>>>
>>>> Using
>>>>
>>>> /* { dg-do run { target arm_neon_hw } } */
>>>>
>>>> works, but I'm not entirely sure sure what the difference is.
>>>> With both solutions (dg-run and dg-require-effective-target), Cortex-A7 tests with -mfloat-abi=soft marks the tests as unsupported. All other targets that I test is unchanged.
>>>>
>>>> I suppose that the reason why there is no difference between the two suggested solutions is that the tests are skipped if arm_tune_string_ops_prefer_neon is not true (the line above my addition in the patch).
>>>>
>>>> Let me know if you prefer the dg-require-effective-target or your solution. If we go for your solution, should I send a v2 with it?
>>>>
>>>> Kind regards,
>>>> Torbjörn
>>>>
>>>
>>> Yeah, this is going to be a bit more invasive than I anticipated.  But I do think we can make this test more widely applicable.  It's going to require a few steps though.
>>>
>>> 1:  I think we need to fix target-supports.exp(check_effective_target_arm_neon_ok_noncache) to add -mcpu=unset whenever there's a -march flag in a variant.
>>> 2:  We need to change the test to not use the skip-if, but to require an effective target of arm_neon (hence the need for the above)
>>> 3:  We need to add a -mtune option as well to ensure that the test does use neon instructions for memcpy operations: adding -mtune=cortex-a7 via additional-options should achieve that.
>>>
>>> With those changes the compile tests should kick in whenever we can successfully override the default options with something appropriate and the execution tests should kick in whenever we have neon hardware (the test is not about whether the performance it great, just whether or not it works).
>>
>> Can we do this as a 2 parted process?
>> With the first part, I would like to avoid running it in configuration where it's known to fail and this part should ideally be retrofired to gcc14.
> 
> I'd prefer not to do that as it creates a user-visible change in a dot release of the compiler.
> 
>> For the 2nd part, I agree with the steps above, but that will only be applicable for gcc15, unless you also want to backport the -mcpu=unset/-march=unset feature to gcc14 too.
>>
>> Can we do the first part in this patch and then come back to part 2 afterwards?
> 
> I'd be happy for completely separate patches to go in on gcc-14 and on trunk - feel free to apply your original patch to the gcc-14 branch while we work on a full fix for gcc-15.

Original patch pushed as r14.2.0-357-g66a3827379e.
I'll rework the patch according to your requests for gcc15 and will 
include it in my upcoming patchset for -march/mcpu=unset.

Kind regards,
Torbjörn
  
Torbjorn SVENSSON Nov. 5, 2024, 7:55 a.m. UTC | #8
On 2024-11-04 15:41, Richard Earnshaw wrote:
> On 01/11/2024 18:40, Richard Earnshaw (lists) wrote:
>> On 24/10/2024 09:50, Torbjörn SVENSSON wrote:
>>> Ok for trunk and releases/gcc-14?
>>>
>>> --
>>>
>>> As these tests are set to execute and require neon hardware to do so,
>>> add the missing dg-require-effective-target arm_neon_hw.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* gcc.target/arm/memset-inline-4.c: Use effective-target
>>> 	arm_neon_hw.
>>> 	* gcc.target/arm/memset-inline-5.c: Likewise.
>>> 	* gcc.target/arm/memset-inline-6.c: Likewise.
>>>
>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>
>> These tests combine both a scan-assembler and a run.  Unconditionally requiring neon hardware before running the test means we lose the scan-assembler when the hardware is not available.  But I think you can write
>>
>> /* { dg-do run { arm_neon_hw } } */
>>
>> instead and now the framework will only try to run the test if hardware is available, but will fall back to a compile test otherwise.
> 
> I've been doing some more digging into this and it looks as though I was mistaken about this fall-back behaviour.  Firstly, I'd missed out the 'target' keyword, the code would need to be
> 
> /* { dg-do run { target arm_neon_hw } } */
> 
> but this still doesn't work as I expected.  Instead the test is skipped entirely if the selector fails to match.  So we shouldn't combine assembly scan tests and execution tests in a single file, but need to have separate tests: one for execution and one for assembly output.

Do you want me to move the assembler checks to a new file, like 
suffixing with "-asm" (memset-inline-5-asm.c) and then include the base 
file in it or do you have something else in mind?

> Sorry for the confusion,

No worries, but I'm still a bit confused on how to proceed. :)

Kind regards,
Torbjörn
  

Patch

diff --git a/gcc/testsuite/gcc.target/arm/memset-inline-4.c b/gcc/testsuite/gcc.target/arm/memset-inline-4.c
index 5d7223ef2c0..fc5f4aeed85 100644
--- a/gcc/testsuite/gcc.target/arm/memset-inline-4.c
+++ b/gcc/testsuite/gcc.target/arm/memset-inline-4.c
@@ -1,5 +1,6 @@ 
 /* { dg-do run } */
 /* { dg-skip-if "Don't inline memset using neon instructions" { ! arm_tune_string_ops_prefer_neon } } */
+/* { dg-require-effective-target arm_neon_hw } */
 /* { dg-options "-save-temps -O2 -fno-inline" } */
 /* { dg-add-options "arm_neon" } */
 
diff --git a/gcc/testsuite/gcc.target/arm/memset-inline-5.c b/gcc/testsuite/gcc.target/arm/memset-inline-5.c
index 6e7ae65eef4..683290771cf 100644
--- a/gcc/testsuite/gcc.target/arm/memset-inline-5.c
+++ b/gcc/testsuite/gcc.target/arm/memset-inline-5.c
@@ -1,5 +1,6 @@ 
 /* { dg-do run } */
 /* { dg-skip-if "Don't inline memset using neon instructions" { ! arm_tune_string_ops_prefer_neon } } */
+/* { dg-require-effective-target arm_neon_hw } */
 /* { dg-options "-save-temps -O2 -fno-inline" } */
 /* { dg-add-options "arm_neon" } */
 
diff --git a/gcc/testsuite/gcc.target/arm/memset-inline-6.c b/gcc/testsuite/gcc.target/arm/memset-inline-6.c
index ae226346d48..66c242eebbe 100644
--- a/gcc/testsuite/gcc.target/arm/memset-inline-6.c
+++ b/gcc/testsuite/gcc.target/arm/memset-inline-6.c
@@ -1,5 +1,6 @@ 
 /* { dg-do run } */
 /* { dg-skip-if "Don't inline memset using neon instructions" { ! arm_tune_string_ops_prefer_neon } } */
+/* { dg-require-effective-target arm_neon_hw } */
 /* { dg-options "-save-temps -O2 -fno-inline" } */
 /* { dg-add-options "arm_neon" } */