public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian
@ 2015-04-16 17:27 Alan Lawrence
  2015-04-29 13:34 ` Alan Lawrence
  2015-04-29 13:45 ` Marcus Shawcroft
  0 siblings, 2 replies; 3+ messages in thread
From: Alan Lawrence @ 2015-04-16 17:27 UTC (permalink / raw)
  To: gcc-patches; +Cc: Marcus Shawcroft, Charles Baylis

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

As per bugzilla entry, indices in the generated assembly for bigendian are 
flipped when they should not be (and, flipped always relative to a Q-register!).

This flips the lane indices back again at assembly time, fixing PR. The 
"indices" contained in the RTL are still wrong for D registers, but these are 
only parameters to an UNSPEC and so never acted upon. (Nonetheless I intend to 
fix this anomaly in later patches).

Tested check-gcc on aarch64-none-elf and aarch64_be-none-elf.
New test (initially failing on bigendian) now passing on both.

gcc/ChangeLog:

	PR target/65770
	config/aarch64/aarch64-simd.md (vec_store_lanesoi_lane<mode>,
	vec_store_lanesci_lane<mode>, vec_store_lanesxi_lane<mode>):
	Flip lane index back at assembly time for bigendian.

gcc/testsuite/ChangeLog:

	PR target/65770
	gcc.target/aarch64/vstN_lane_1.c: New file.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vstn_lane.patch --]
[-- Type: text/x-patch; name=vstn_lane.patch, Size: 5374 bytes --]

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 055757036d54d0d5cf5df4bd05419e39ea119f46..b84374443a08a89a7b7c372b1585e128ac8b7fdd 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3954,6 +3954,7 @@
   [(set_attr "type" "neon_store2_2reg<q>")]
 )
 
+;; RTL uses GCC vector extension indices, so flip only for assembly.
 (define_insn "vec_store_lanesoi_lane<mode>"
   [(set (match_operand:<V_TWO_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
 	(unspec:<V_TWO_ELEM> [(match_operand:OI 1 "register_operand" "w")
@@ -3961,7 +3962,10 @@
 		    (match_operand:SI 2 "immediate_operand" "i")]
                    UNSPEC_ST2_LANE))]
   "TARGET_SIMD"
-  "st2\\t{%S1.<Vetype> - %T1.<Vetype>}[%2], %0"
+  {
+    operands[2] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[2])));
+    return "st2\\t{%S1.<Vetype> - %T1.<Vetype>}[%2], %0";
+  }
   [(set_attr "type" "neon_store3_one_lane<q>")]
 )
 
@@ -4045,6 +4049,7 @@
   [(set_attr "type" "neon_store3_3reg<q>")]
 )
 
+;; RTL uses GCC vector extension indices, so flip only for assembly.
 (define_insn "vec_store_lanesci_lane<mode>"
   [(set (match_operand:<V_THREE_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
 	(unspec:<V_THREE_ELEM> [(match_operand:CI 1 "register_operand" "w")
@@ -4052,7 +4057,10 @@
 		    (match_operand:SI 2 "immediate_operand" "i")]
                    UNSPEC_ST3_LANE))]
   "TARGET_SIMD"
-  "st3\\t{%S1.<Vetype> - %U1.<Vetype>}[%2], %0"
+  {
+    operands[2] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[2])));
+    return "st3\\t{%S1.<Vetype> - %U1.<Vetype>}[%2], %0";
+  }
   [(set_attr "type" "neon_store3_one_lane<q>")]
 )
 
@@ -4136,6 +4144,7 @@
   [(set_attr "type" "neon_store4_4reg<q>")]
 )
 
+;; RTL uses GCC vector extension indices, so flip only for assembly.
 (define_insn "vec_store_lanesxi_lane<mode>"
   [(set (match_operand:<V_FOUR_ELEM> 0 "aarch64_simd_struct_operand" "=Utv")
 	(unspec:<V_FOUR_ELEM> [(match_operand:XI 1 "register_operand" "w")
@@ -4143,7 +4152,10 @@
 		    (match_operand:SI 2 "immediate_operand" "i")]
                    UNSPEC_ST4_LANE))]
   "TARGET_SIMD"
-  "st4\\t{%S1.<Vetype> - %V1.<Vetype>}[%2], %0"
+  {
+    operands[2] = GEN_INT (ENDIAN_LANE_N (<MODE>mode, INTVAL (operands[2])));
+    return "st4\\t{%S1.<Vetype> - %V1.<Vetype>}[%2], %0";
+  }
   [(set_attr "type" "neon_store4_one_lane<q>")]
 )
 
diff --git a/gcc/testsuite/gcc.target/aarch64/vstN_lane_1.c b/gcc/testsuite/gcc.target/aarch64/vstN_lane_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..a695aa1954036ef1c1782b14ddb3c46ec78b5f0b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/vstN_lane_1.c
@@ -0,0 +1,75 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline" } */
+
+#include <arm_neon.h>
+
+extern void abort (void);
+
+#define VARIANTS(VARIANT, STRUCT)	\
+VARIANT (uint8, , 8, _u8, 6, STRUCT)	\
+VARIANT (uint16, , 4, _u16, 3, STRUCT)	\
+VARIANT (uint32, , 2, _u32, 1, STRUCT)	\
+VARIANT (uint64, , 1, _u64, 0, STRUCT)	\
+VARIANT (int8, , 8, _s8, 5, STRUCT)	\
+VARIANT (int16, , 4, _s16, 2, STRUCT)	\
+VARIANT (int32, , 2, _s32, 0, STRUCT)	\
+VARIANT (int64, , 1, _s64, 0, STRUCT)	\
+VARIANT (poly8, , 8, _p8, 7, STRUCT)	\
+VARIANT (poly16, , 4, _p16, 1, STRUCT)	\
+VARIANT (float32, , 2, _f32, 1, STRUCT)	\
+VARIANT (float64, , 1, _f64, 0, STRUCT)	\
+VARIANT (uint8, q, 16, _u8, 14, STRUCT)	\
+VARIANT (uint16, q, 8, _u16, 4, STRUCT)	\
+VARIANT (uint32, q, 4, _u32, 3, STRUCT)	\
+VARIANT (uint64, q, 2, _u64, 0, STRUCT)	\
+VARIANT (int8, q, 16, _s8, 13, STRUCT)	\
+VARIANT (int16, q, 8, _s16, 6, STRUCT)	\
+VARIANT (int32, q, 4, _s32, 2, STRUCT)	\
+VARIANT (int64, q, 2, _s64, 1, STRUCT)	\
+VARIANT (poly8, q, 16, _p8, 12, STRUCT)	\
+VARIANT (poly16, q, 8, _p16, 5, STRUCT)	\
+VARIANT (float32, q, 4, _f32, 1, STRUCT)\
+VARIANT (float64, q, 2, _f64, 0, STRUCT)
+
+#define TESTMETH(BASE, Q, ELTS, SUFFIX, LANE, STRUCT)			\
+int									\
+test_vst##STRUCT##Q##_lane##SUFFIX (const BASE##_t *data)		\
+{									\
+  BASE##x##ELTS##x##STRUCT##_t vectors;					\
+  for (int i = 0; i < STRUCT; i++, data += ELTS)			\
+    vectors.val[i] = vld1##Q##SUFFIX (data);				\
+  BASE##_t temp[STRUCT];						\
+  vst##STRUCT##Q##_lane##SUFFIX (temp, vectors, LANE);			\
+  for (int i = 0; i < STRUCT; i++)					\
+    {									\
+      if (temp[i] != vget##Q##_lane##SUFFIX (vectors.val[i], LANE))	\
+	return 1;							\
+    }									\
+  return 0;								\
+}
+
+/* Tests of vst2_lane and vst2q_lane.  */
+VARIANTS (TESTMETH, 2)
+/* Tests of vst3_lane and vst3q_lane.  */
+VARIANTS (TESTMETH, 3)
+/* Tests of vst4_lane and vst4q_lane.  */
+VARIANTS (TESTMETH, 4)
+
+#define CHECK(BASE, Q, ELTS, SUFFIX, LANE, STRUCT)			\
+  if (test_vst##STRUCT##Q##_lane##SUFFIX ((const BASE##_t *)orig_data))	\
+    abort ();
+
+int
+main (int argc, char **argv)
+{
+  /* Original data for all vector formats.  */
+  uint64_t orig_data[8] = {0x1234567890abcdefULL, 0x13579bdf02468aceULL,
+			   0x012389ab4567cdefULL, 0xfeeddadacafe0431ULL,
+			   0x1032547698badcfeULL, 0xbadbadbadbad0badULL,
+			   0x0102030405060708ULL, 0x0f0e0d0c0b0a0908ULL};
+
+  VARIANTS (CHECK, 2);
+  VARIANTS (CHECK, 3);
+  VARIANTS (CHECK, 4);
+  return 0;
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian
  2015-04-16 17:27 [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian Alan Lawrence
@ 2015-04-29 13:34 ` Alan Lawrence
  2015-04-29 13:45 ` Marcus Shawcroft
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Lawrence @ 2015-04-29 13:34 UTC (permalink / raw)
  To: gcc-patches; +Cc: Marcus Shawcroft

Alan Lawrence wrote:
> As per bugzilla entry, indices in the generated assembly for bigendian are 
> flipped when they should not be (and, flipped always relative to a Q-register!).
> 
> This flips the lane indices back again at assembly time, fixing PR. The 
> "indices" contained in the RTL are still wrong for D registers, but these are 
> only parameters to an UNSPEC and so never acted upon. (Nonetheless I intend to 
> fix this anomaly in later patches).
> 
> Tested check-gcc on aarch64-none-elf and aarch64_be-none-elf.
> New test (initially failing on bigendian) now passing on both.
> 
> gcc/ChangeLog:
> 
> 	PR target/65770
> 	config/aarch64/aarch64-simd.md (vec_store_lanesoi_lane<mode>,
> 	vec_store_lanesci_lane<mode>, vec_store_lanesxi_lane<mode>):
> 	Flip lane index back at assembly time for bigendian.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR target/65770
> 	gcc.target/aarch64/vstN_lane_1.c: New file.

Ping.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian
  2015-04-16 17:27 [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian Alan Lawrence
  2015-04-29 13:34 ` Alan Lawrence
@ 2015-04-29 13:45 ` Marcus Shawcroft
  1 sibling, 0 replies; 3+ messages in thread
From: Marcus Shawcroft @ 2015-04-29 13:45 UTC (permalink / raw)
  To: Alan Lawrence; +Cc: gcc-patches

On 16 April 2015 at 18:27, Alan Lawrence <alan.lawrence@arm.com> wrote:
> As per bugzilla entry, indices in the generated assembly for bigendian are
> flipped when they should not be (and, flipped always relative to a
> Q-register!).
>
> This flips the lane indices back again at assembly time, fixing PR. The
> "indices" contained in the RTL are still wrong for D registers, but these
> are only parameters to an UNSPEC and so never acted upon. (Nonetheless I
> intend to fix this anomaly in later patches).
>
> Tested check-gcc on aarch64-none-elf and aarch64_be-none-elf.
> New test (initially failing on bigendian) now passing on both.
>
> gcc/ChangeLog:
>
>         PR target/65770
>         config/aarch64/aarch64-simd.md (vec_store_lanesoi_lane<mode>,
>         vec_store_lanesci_lane<mode>, vec_store_lanesxi_lane<mode>):
>         Flip lane index back at assembly time for bigendian.
>
> gcc/testsuite/ChangeLog:
>
>         PR target/65770
>         gcc.target/aarch64/vstN_lane_1.c: New file.

OK and backport to 5 please. /Marcus

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-04-29 13:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-16 17:27 [PATCH][AArch64] Fix PR/65770 vstN_lane on bigendian Alan Lawrence
2015-04-29 13:34 ` Alan Lawrence
2015-04-29 13:45 ` Marcus Shawcroft

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).