diff mbox series

testsuite: Allow vst1 instruction

Message ID 20240719130709.3943936-1-torbjorn.svensson@foss.st.com
State Rejected
Headers
Series testsuite: Allow vst1 instruction |

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
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

Torbjorn SVENSSON July 19, 2024, 1:07 p.m. UTC 
  Ok for trunk and releases/gcc-14?

--

On Cortex-A7 with -mfloat-abi=hard and -mfpu=neon, the following
instructions are generated for the test case:

        vldr    d16, .L3
        vst1.32 {d16}, [r0]

For Cortex-A7 with -mfloat-abi=soft, it's instead:

        movs    r2, #1
        movs    r3, #0
        strd    r2, r3, [r0]

Both these are valid and should not cause the test to fail.
For Cortex-M0/3/4/7/33, they all generate the same instructions as
Cortex-A7 with -mfloat-abi=soft.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/pr40457-2.c: Add vst1 to allowed instructions.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 gcc/testsuite/gcc.target/arm/pr40457-2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Earnshaw (lists) July 19, 2024, 2:36 p.m. UTC | #1 
On 19/07/2024 14:07, Torbjörn SVENSSON wrote:
> Ok for trunk and releases/gcc-14?
> 
> --
> 
> On Cortex-A7 with -mfloat-abi=hard and -mfpu=neon, the following
> instructions are generated for the test case:
> 
>         vldr    d16, .L3
>         vst1.32 {d16}, [r0]
> 
> For Cortex-A7 with -mfloat-abi=soft, it's instead:
> 
>         movs    r2, #1
>         movs    r3, #0
>         strd    r2, r3, [r0]
> 
> Both these are valid and should not cause the test to fail.
> For Cortex-M0/3/4/7/33, they all generate the same instructions as
> Cortex-A7 with -mfloat-abi=soft.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/arm/pr40457-2.c: Add vst1 to allowed instructions.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>  gcc/testsuite/gcc.target/arm/pr40457-2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.target/arm/pr40457-2.c b/gcc/testsuite/gcc.target/arm/pr40457-2.c
> index 31624d35127..6aed42a4fbc 100644
> --- a/gcc/testsuite/gcc.target/arm/pr40457-2.c
> +++ b/gcc/testsuite/gcc.target/arm/pr40457-2.c
> @@ -7,4 +7,4 @@ void foo(int* p)
>    p[1] = 0;
>  }
>  
> -/* { dg-final { scan-assembler "strd|stm" } } */
> +/* { dg-final { scan-assembler "strd|stm|vst1" } } */

Whilst the code generated is functionally correct, I'm not sure the compiler should be generating that.  This new code needs more code space (the constant isn't shown, but it needs 8 bytes in the literal pool).  I suspect this means the SLP cost model for arm is in need of attention.

R.
Torbjorn SVENSSON July 19, 2024, 3:10 p.m. UTC | #2 
On 2024-07-19 16:36, Richard Earnshaw (lists) wrote:
> On 19/07/2024 14:07, Torbjörn SVENSSON wrote:
>> Ok for trunk and releases/gcc-14?
>>
>> --
>>
>> On Cortex-A7 with -mfloat-abi=hard and -mfpu=neon, the following
>> instructions are generated for the test case:
>>
>>          vldr    d16, .L3
>>          vst1.32 {d16}, [r0]
>>
>> For Cortex-A7 with -mfloat-abi=soft, it's instead:
>>
>>          movs    r2, #1
>>          movs    r3, #0
>>          strd    r2, r3, [r0]
>>
>> Both these are valid and should not cause the test to fail.
>> For Cortex-M0/3/4/7/33, they all generate the same instructions as
>> Cortex-A7 with -mfloat-abi=soft.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/arm/pr40457-2.c: Add vst1 to allowed instructions.
>>
>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>> ---
>>   gcc/testsuite/gcc.target/arm/pr40457-2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/pr40457-2.c b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>> index 31624d35127..6aed42a4fbc 100644
>> --- a/gcc/testsuite/gcc.target/arm/pr40457-2.c
>> +++ b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>> @@ -7,4 +7,4 @@ void foo(int* p)
>>     p[1] = 0;
>>   }
>>   
>> -/* { dg-final { scan-assembler "strd|stm" } } */
>> +/* { dg-final { scan-assembler "strd|stm|vst1" } } */
> 
> Whilst the code generated is functionally correct, I'm not sure the compiler should be generating that.  This new code needs more code space (the constant isn't shown, but it needs 8 bytes in the literal pool).  I suspect this means the SLP cost model for arm is in need of attention.

Do you want me to create a ticket for the SLP cost model review or will 
you handle it?

It looks like arm-gnu-toolchain-11.3.rel1-x86_64-arm-none-eabi does 
produce the old output, while 
arm-gnu-toolchain-12.2.rel1-x86_64-arm-none-eabi changed it.

Kind regards,
Torbjörn
Richard Earnshaw (lists) July 19, 2024, 3:22 p.m. UTC | #3 
On 19/07/2024 16:10, Torbjorn SVENSSON wrote:
> 
> 
> On 2024-07-19 16:36, Richard Earnshaw (lists) wrote:
>> On 19/07/2024 14:07, Torbjörn SVENSSON wrote:
>>> Ok for trunk and releases/gcc-14?
>>>
>>> -- 
>>>
>>> On Cortex-A7 with -mfloat-abi=hard and -mfpu=neon, the following
>>> instructions are generated for the test case:
>>>
>>>          vldr    d16, .L3
>>>          vst1.32 {d16}, [r0]
>>>
>>> For Cortex-A7 with -mfloat-abi=soft, it's instead:
>>>
>>>          movs    r2, #1
>>>          movs    r3, #0
>>>          strd    r2, r3, [r0]
>>>
>>> Both these are valid and should not cause the test to fail.
>>> For Cortex-M0/3/4/7/33, they all generate the same instructions as
>>> Cortex-A7 with -mfloat-abi=soft.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/arm/pr40457-2.c: Add vst1 to allowed instructions.
>>>
>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>> ---
>>>   gcc/testsuite/gcc.target/arm/pr40457-2.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/pr40457-2.c b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>> index 31624d35127..6aed42a4fbc 100644
>>> --- a/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>> +++ b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>> @@ -7,4 +7,4 @@ void foo(int* p)
>>>     p[1] = 0;
>>>   }
>>>   -/* { dg-final { scan-assembler "strd|stm" } } */
>>> +/* { dg-final { scan-assembler "strd|stm|vst1" } } */
>>
>> Whilst the code generated is functionally correct, I'm not sure the compiler should be generating that.  This new code needs more code space (the constant isn't shown, but it needs 8 bytes in the literal pool).  I suspect this means the SLP cost model for arm is in need of attention.
> 
> Do you want me to create a ticket for the SLP cost model review or will you handle it?
> 
> It looks like arm-gnu-toolchain-11.3.rel1-x86_64-arm-none-eabi does produce the old output, while arm-gnu-toolchain-12.2.rel1-x86_64-arm-none-eabi changed it.
> 
> Kind regards,
> Torbjörn
> 


A ticket would be good please.  I don't think I'm going to have time to look at this soon, so it will likely get forgotten without one.

It would be interesting to know which change led to this regression as well.

R.
Torbjorn SVENSSON July 22, 2024, 5 p.m. UTC | #4 
On 2024-07-19 17:22, Richard Earnshaw (lists) wrote:
> On 19/07/2024 16:10, Torbjorn SVENSSON wrote:
>>
>>
>> On 2024-07-19 16:36, Richard Earnshaw (lists) wrote:
>>> On 19/07/2024 14:07, Torbjörn SVENSSON wrote:
>>>> Ok for trunk and releases/gcc-14?
>>>>
>>>> -- 
>>>>
>>>> On Cortex-A7 with -mfloat-abi=hard and -mfpu=neon, the following
>>>> instructions are generated for the test case:
>>>>
>>>>           vldr    d16, .L3
>>>>           vst1.32 {d16}, [r0]
>>>>
>>>> For Cortex-A7 with -mfloat-abi=soft, it's instead:
>>>>
>>>>           movs    r2, #1
>>>>           movs    r3, #0
>>>>           strd    r2, r3, [r0]
>>>>
>>>> Both these are valid and should not cause the test to fail.
>>>> For Cortex-M0/3/4/7/33, they all generate the same instructions as
>>>> Cortex-A7 with -mfloat-abi=soft.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>      * gcc.target/arm/pr40457-2.c: Add vst1 to allowed instructions.
>>>>
>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>>> ---
>>>>    gcc/testsuite/gcc.target/arm/pr40457-2.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/gcc/testsuite/gcc.target/arm/pr40457-2.c b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>>> index 31624d35127..6aed42a4fbc 100644
>>>> --- a/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>>> +++ b/gcc/testsuite/gcc.target/arm/pr40457-2.c
>>>> @@ -7,4 +7,4 @@ void foo(int* p)
>>>>      p[1] = 0;
>>>>    }
>>>>    -/* { dg-final { scan-assembler "strd|stm" } } */
>>>> +/* { dg-final { scan-assembler "strd|stm|vst1" } } */
>>>
>>> Whilst the code generated is functionally correct, I'm not sure the compiler should be generating that.  This new code needs more code space (the constant isn't shown, but it needs 8 bytes in the literal pool).  I suspect this means the SLP cost model for arm is in need of attention.
>>
>> Do you want me to create a ticket for the SLP cost model review or will you handle it?
>>
>> It looks like arm-gnu-toolchain-11.3.rel1-x86_64-arm-none-eabi does produce the old output, while arm-gnu-toolchain-12.2.rel1-x86_64-arm-none-eabi changed it.
>>
>> Kind regards,
>> Torbjörn
>>
> 
> 
> A ticket would be good please.  I don't think I'm going to have time to look at this soon, so it will likely get forgotten without one.
> 
> It would be interesting to know which change led to this regression as well.
> 
> R.

Created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116032 for the SLP 
cost model review.

Kind regards,
Torbjörn
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/arm/pr40457-2.c b/gcc/testsuite/gcc.target/arm/pr40457-2.c
index 31624d35127..6aed42a4fbc 100644
--- a/gcc/testsuite/gcc.target/arm/pr40457-2.c
+++ b/gcc/testsuite/gcc.target/arm/pr40457-2.c
@@ -7,4 +7,4 @@  void foo(int* p)
   p[1] = 0;
 }
 
-/* { dg-final { scan-assembler "strd|stm" } } */
+/* { dg-final { scan-assembler "strd|stm|vst1" } } */