Patchwork [ARM] Fix PR45094

login
register
mail settings
Submitter Yao Qi
Date Aug. 4, 2010, 4:18 a.m.
Message ID <20100804041811.GA18664@qiyaows>
Download mbox | patch
Permalink /patch/60819/
State New
Headers show

Comments

Yao Qi - Aug. 4, 2010, 4:18 a.m.
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?
Ramana Radhakrishnan - Aug. 4, 2010, 12:52 p.m.
> > 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
Yao Qi - Aug. 10, 2010, 1:13 p.m.
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

Patch

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;
+}