* [PATCH, ARM] Fix PR45094
@ 2010-08-01 8:36 Yao Qi
2010-08-01 12:38 ` John Tytgat
0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2010-08-01 8:36 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 434 bytes --]
This patch fixes generated wrong instructions reported in PR45094,
which is caused by typo.
Tested on arm-unknown-linux-gnueabi, fixes the new test.
OK for mainline?
ChangeLog:
gcc/
PR target/45094
* config/arm/arm.c (output_move_double): Fix typo generating
instructions ('ldr'->'str').
gcc/testsuite/
PR target/45094
* gcc.target/arm/pr45094.c: New test.
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
[-- Attachment #2: pr45094.patch --]
[-- Type: text/x-diff, Size: 1598 bytes --]
gcc/
PR target/45094
* config/arm/arm.c (output_move_double): Fix typo generating
instructions ('ldr'->'str').
gcc/testsuite/
PR target/45094
* gcc.target/arm/pr45094.c: New test.
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c (revision 162792)
+++ config/arm/arm.c (working copy)
@@ -12570,13 +12570,13 @@
{
if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
{
- output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
+ output_asm_insn ("strr%?\t%H0, [%1, #4]", otherops);
}
else
{
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
- output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1], %2", otherops);
}
}
else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
Index: testsuite/gcc.target/arm/pr45094.c
===================================================================
--- testsuite/gcc.target/arm/pr45094.c (revision 0)
+++ testsuite/gcc.target/arm/pr45094.c (revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=softfp" } */
+
+long long buffer[32];
+
+void __attribute__((noinline)) f(long long *p, int n)
+{
+ while (--n >= 0) {
+ *p = 1;
+ p += 32;
+ }
+}
+
+int main(void)
+{
+ f(buffer, 1);
+ return !buffer[0];
+}
+
+/* { dg-final { scan-assembler "str" } } */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-01 8:36 [PATCH, ARM] Fix PR45094 Yao Qi
@ 2010-08-01 12:38 ` John Tytgat
2010-08-01 14:58 ` Yao Qi
0 siblings, 1 reply; 9+ messages in thread
From: John Tytgat @ 2010-08-01 12:38 UTC (permalink / raw)
To: gcc-patches
In message <20100801083614.GA2569@qiyaows>
Yao Qi <yao@codesourcery.com> wrote:
> This patch fixes generated wrong instructions reported in PR45094,
> which is caused by typo.
>
> Tested on arm-unknown-linux-gnueabi, fixes the new test.
> [...]
> Index: config/arm/arm.c
> ===================================================================
> --- config/arm/arm.c (revision 162792)
> +++ config/arm/arm.c (working copy)
> @@ -12570,13 +12570,13 @@
> {
> if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
> {
> - output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
> - output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
> + output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
> + output_asm_insn ("strr%?\t%H0, [%1, #4]", otherops);
^^^
Typo (one 'r') ?
> }
> else
> {
> - output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
> - output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
> + output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
> + output_asm_insn ("str%?\t%0, [%1], %2", otherops);
> }
> }
> else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
John.
--
John Tytgat, in his comfy chair at home
John.Tytgat@aaug.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-01 12:38 ` John Tytgat
@ 2010-08-01 14:58 ` Yao Qi
2010-08-02 0:38 ` Daniel Jacobowitz
0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2010-08-01 14:58 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]
On Sun, Aug 01, 2010 at 01:43:09PM +0100, John Tytgat wrote:
> In message <20100801083614.GA2569@qiyaows>
> Yao Qi <yao@codesourcery.com> wrote:
>
> > This patch fixes generated wrong instructions reported in PR45094,
> > which is caused by typo.
> >
> > Tested on arm-unknown-linux-gnueabi, fixes the new test.
> > [...]
>
> > Index: config/arm/arm.c
> > ===================================================================
> > --- config/arm/arm.c (revision 162792)
> > +++ config/arm/arm.c (working copy)
> > @@ -12570,13 +12570,13 @@
> > {
> > if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
> > {
> > - output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
> > - output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
> > + output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
> > + output_asm_insn ("strr%?\t%H0, [%1, #4]", otherops);
> ^^^
>
> Typo (one 'r') ?
>
Oops... Correct typo in new patch.
> > }
> > else
> > {
> > - output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
> > - output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
> > + output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
> > + output_asm_insn ("str%?\t%0, [%1], %2", otherops);
> > }
> > }
> > else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
[-- Attachment #2: pr45094.patch --]
[-- Type: text/x-diff, Size: 1597 bytes --]
gcc/
PR target/45094
* config/arm/arm.c (output_move_double): Fix typo generating
instructions ('ldr'->'str').
gcc/testsuite/
PR target/45094
* gcc.target/arm/pr45094.c: New test.
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c (revision 162792)
+++ config/arm/arm.c (working copy)
@@ -12570,13 +12570,13 @@
{
if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
{
- output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
}
else
{
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
- output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1], %2", otherops);
}
}
else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
Index: testsuite/gcc.target/arm/pr45094.c
===================================================================
--- testsuite/gcc.target/arm/pr45094.c (revision 0)
+++ testsuite/gcc.target/arm/pr45094.c (revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=softfp" } */
+
+long long buffer[32];
+
+void __attribute__((noinline)) f(long long *p, int n)
+{
+ while (--n >= 0) {
+ *p = 1;
+ p += 32;
+ }
+}
+
+int main(void)
+{
+ f(buffer, 1);
+ return !buffer[0];
+}
+
+/* { dg-final { scan-assembler "str" } } */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-01 14:58 ` Yao Qi
@ 2010-08-02 0:38 ` Daniel Jacobowitz
2010-08-03 13:29 ` Yao Qi
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Jacobowitz @ 2010-08-02 0:38 UTC (permalink / raw)
To: Yao Qi; +Cc: gcc-patches
On Sun, Aug 01, 2010 at 10:58:29PM +0800, Yao Qi wrote:
> Oops... Correct typo in new patch.
This suggests there should be an execute test. When possible, those
are better.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-02 0:38 ` Daniel Jacobowitz
@ 2010-08-03 13:29 ` Yao Qi
2010-08-03 23:58 ` Ramana Radhakrishnan
0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2010-08-03 13:29 UTC (permalink / raw)
To: gcc-patches; +Cc: Yao Qi
[-- Attachment #1: Type: text/plain, Size: 485 bytes --]
On Sun, Aug 01, 2010 at 08:38:18PM -0400, Daniel Jacobowitz wrote:
> On Sun, Aug 01, 2010 at 10:58:29PM +0800, Yao Qi wrote:
> > Oops... Correct typo in new patch.
>
> This suggests there should be an execute test. When possible, those
> are better.
Revise test case to an 'execute test', rather than a 'compile test'.
Bootstrapped and regression tested on arm-unknown-linux-gnueabi. No
regression. OK to commit?
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
[-- Attachment #2: pr45094.patch --]
[-- Type: text/x-diff, Size: 1619 bytes --]
gcc/
PR target/45094
* config/arm/arm.c (output_move_double): Fix typo generating
instructions ('ldr'->'str').
gcc/testsuite/
PR target/45094
* gcc.target/arm/pr45094.c: New test.
Index: gcc/testsuite/gcc.target/arm/pr45094.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr45094.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/pr45094.c (revision 0)
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=softfp" } */
+
+#include <stdlib.h>
+
+long long buffer[32];
+
+void __attribute__((noinline)) f(long long *p, int n)
+{
+ while (--n >= 0) {
+ *p = 1;
+ p += 32;
+ }
+}
+
+int main(void)
+{
+ f(buffer, 1);
+
+ if (!buffer[0])
+ abort();
+
+ return 0;
+}
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 162792)
+++ gcc/config/arm/arm.c (working copy)
@@ -12570,13 +12570,13 @@
{
if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
{
- output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
}
else
{
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
- output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1], %2", otherops);
}
}
else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-03 13:29 ` Yao Qi
@ 2010-08-03 23:58 ` Ramana Radhakrishnan
2010-08-04 4:18 ` Yao Qi
0 siblings, 1 reply; 9+ messages in thread
From: Ramana Radhakrishnan @ 2010-08-03 23:58 UTC (permalink / raw)
To: Yao Qi; +Cc: gcc-patches
On Tue, Aug 3, 2010 at 2:28 PM, Yao Qi <yao@codesourcery.com> wrote:
> On Sun, Aug 01, 2010 at 08:38:18PM -0400, Daniel Jacobowitz wrote:
>> On Sun, Aug 01, 2010 at 10:58:29PM +0800, Yao Qi wrote:
>> > Oops... Correct typo in new patch.
>>
>> This suggests there should be an execute test. When possible, those
>> are better.
>
> Revise test case to an 'execute test', rather than a 'compile test'.
> Bootstrapped and regression tested on arm-unknown-linux-gnueabi. No
> regression. OK to commit?
No - this test is still not ok. It is not necessary that all
arm-linux-gnueabi targets have Neon. Think about the Tegra2 where
there is no Neon unit where this test will fail. Thus if you really
need the neon options (out of curiosity why?) please add the following
to the test.
/* { dg-require-effective-target arm_neon_hw } */
before the dg-do run. IIRC there might be examples in other tests in
gcc.target/arm/neon*.c
cheers
Ramana
cheers
Ramana
>
> --
> Yao Qi
> CodeSourcery
> yao@codesourcery.com
> (650) 331-3385 x739
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-03 23:58 ` Ramana Radhakrishnan
@ 2010-08-04 4:18 ` Yao Qi
2010-08-04 12:52 ` Ramana Radhakrishnan
0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2010-08-04 4:18 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]
On Wed, Aug 04, 2010 at 12:58:47AM +0100, Ramana Radhakrishnan wrote:
> On Tue, Aug 3, 2010 at 2:28 PM, Yao Qi <yao@codesourcery.com> wrote:
> > On Sun, Aug 01, 2010 at 08:38:18PM -0400, Daniel Jacobowitz wrote:
> >> On Sun, Aug 01, 2010 at 10:58:29PM +0800, Yao Qi wrote:
> >> > Oops... Â Correct typo in new patch.
> >>
> >> This suggests there should be an execute test. Â When possible, those
> >> are better.
> >
> > Revise test case to an 'execute test', rather than a 'compile test'.
> > Bootstrapped and regression tested on arm-unknown-linux-gnueabi. Â No
> > regression. Â OK to commit?
>
> No - this test is still not ok. It is not necessary that all
> arm-linux-gnueabi targets have Neon. Think about the Tegra2 where
> there is no Neon unit where this test will fail. Thus if you really
> need the neon options (out of curiosity why?) please add the following
> to the test.
>
> /* { dg-require-effective-target arm_neon_hw } */
Add /* { dg-require-effective-target arm_neon_hw } */ into the test
as you suggested here. Re-create patch against mainline trunk, and
tested. OK to apply?
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
[-- Attachment #2: pr45094.patch --]
[-- Type: text/x-diff, Size: 1680 bytes --]
gcc/
PR target/45094
* config/arm/arm.c (output_move_double): Fix typo generating
instructions ('ldr'->'str').
gcc/testsuite/
PR target/45094
* gcc.target/arm/pr45094.c: New test.
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 162853)
+++ gcc/config/arm/arm.c (working copy)
@@ -12836,13 +12836,13 @@
{
if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
{
- output_asm_insn ("ldr%?\t%0, [%1, %2]!", otherops);
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1, %2]!", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
}
else
{
- output_asm_insn ("ldr%?\t%H0, [%1, #4]", otherops);
- output_asm_insn ("ldr%?\t%0, [%1], %2", otherops);
+ output_asm_insn ("str%?\t%H0, [%1, #4]", otherops);
+ output_asm_insn ("str%?\t%0, [%1], %2", otherops);
}
}
else if (GET_CODE (XEXP (operands[0], 0)) == PRE_MODIFY)
Index: gcc/testsuite/gcc.target/arm/pr45094.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr45094.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/pr45094.c (revision 0)
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_neon_hw } */
+/* { dg-options "-O2 -mcpu=cortex-a8" } */
+/* { dg-add-options arm_neon } */
+
+#include <stdlib.h>
+
+long long buffer[32];
+
+void __attribute__((noinline)) f(long long *p, int n)
+{
+ while (--n >= 0)
+ {
+ *p = 1;
+ p += 32;
+ }
+}
+
+int main(void)
+{
+ f(buffer, 1);
+
+ if (!buffer[0])
+ abort();
+
+ return 0;
+}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, ARM] Fix PR45094
2010-08-04 4:18 ` Yao Qi
@ 2010-08-04 12:52 ` Ramana Radhakrishnan
2010-08-10 13:16 ` [PING] " Yao Qi
0 siblings, 1 reply; 9+ messages in thread
From: Ramana Radhakrishnan @ 2010-08-04 12:52 UTC (permalink / raw)
To: Yao Qi; +Cc: gcc-patches
> > No - this test is still not ok. It is not necessary that all
> > arm-linux-gnueabi targets have Neon. Think about the Tegra2 where
> > there is no Neon unit where this test will fail. Thus if you really
> > need the neon options (out of curiosity why?) please add the following
> > to the test.
> >
> > /* { dg-require-effective-target arm_neon_hw } */
>
> Add /* { dg-require-effective-target arm_neon_hw } */ into the test
> as you suggested here. Re-create patch against mainline trunk, and
> tested. OK to apply?
Yes this looks much better - thanks. This should go in under the
"obvious" rule but please wait for a backend maintainer to comment.
cheers
Ramana
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PING] [PATCH, ARM] Fix PR45094
2010-08-04 12:52 ` Ramana Radhakrishnan
@ 2010-08-10 13:16 ` Yao Qi
0 siblings, 0 replies; 9+ messages in thread
From: Yao Qi @ 2010-08-10 13:16 UTC (permalink / raw)
To: gcc-patches
Ramana Radhakrishnan wrote:
>>> No - this test is still not ok. It is not necessary that all
>>> arm-linux-gnueabi targets have Neon. Think about the Tegra2 where
>>> there is no Neon unit where this test will fail. Thus if you really
>>> need the neon options (out of curiosity why?) please add the following
>>> to the test.
>>>
>>> /* { dg-require-effective-target arm_neon_hw } */
>> Add /* { dg-require-effective-target arm_neon_hw } */ into the test
>> as you suggested here. Re-create patch against mainline trunk, and
>> tested. OK to apply?
>
> Yes this looks much better - thanks. This should go in under the
> "obvious" rule but please wait for a backend maintainer to comment.
>
Here is the latest patch to fix GCC PR45094, after Ramana
Radhakrishnan's review.
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00249.html
--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-08-10 13:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-01 8:36 [PATCH, ARM] Fix PR45094 Yao Qi
2010-08-01 12:38 ` John Tytgat
2010-08-01 14:58 ` Yao Qi
2010-08-02 0:38 ` Daniel Jacobowitz
2010-08-03 13:29 ` Yao Qi
2010-08-03 23:58 ` Ramana Radhakrishnan
2010-08-04 4:18 ` Yao Qi
2010-08-04 12:52 ` Ramana Radhakrishnan
2010-08-10 13:16 ` [PING] " Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).