| Message ID | 20240719130709.3943936-1-torbjorn.svensson@foss.st.com |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: Allow vst1 instruction | expand |
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.
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
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.
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 --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" } } */
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(-)