public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch AArch64] Fix for PR62040
@ 2014-08-19 23:43 Carrot Wei
  2014-08-20 11:27 ` Kyrill Tkachov
  2014-09-03 13:05 ` Marcus Shawcroft
  0 siblings, 2 replies; 8+ messages in thread
From: Carrot Wei @ 2014-08-19 23:43 UTC (permalink / raw)
  To: gcc-patches

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

Hi

Current AArch64 backend can generate rtl expressions like
(vec_duplicate:DI (const_int 0 [0])), which causes ICE in
simplify_const_unary_operation because vec_duplicate should generate
vector mode only.

As suggested by Andrew in the bug entry, I split the original insn
patterns to avoid scalar mode vec_duplicate expression.

Passed regression tests on qemu without failure.
OK for trunk and 4.9 branch?

thanks
Guozhi Wei

2014-08-19  Guozhi Wei  <carrot@google.com>

        PR target/62040
        * config/aarch64/iterators.md (VQ_NO2E, VQ_2E): New iterators.
        * config/aarch64/aarch64-simd.md (move_lo_quad_internal_<mode>): Split
        it into two patterns.
        (move_lo_quad_internal_be_<mode>): Likewise.

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 2636 bytes --]

Index: iterators.md
===================================================================
--- iterators.md	(revision 214194)
+++ iterators.md	(working copy)
@@ -66,6 +66,12 @@
 ;; Quad vector modes.
 (define_mode_iterator VQ [V16QI V8HI V4SI V2DI V4SF V2DF])
 
+;; VQ without 2 element modes.
+(define_mode_iterator VQ_NO2E [V16QI V8HI V4SI V4SF])
+
+;; Quad vector with only 2 element modes.
+(define_mode_iterator VQ_2E [V2DI V2DF])
+
 ;; All vector modes, except double.
 (define_mode_iterator VQ_S [V8QI V16QI V4HI V8HI V2SI V4SI])
 
Index: aarch64-simd.md
===================================================================
--- aarch64-simd.md	(revision 214194)
+++ aarch64-simd.md	(working copy)
@@ -953,8 +953,8 @@
 ;; On big-endian this is { zeroes, operand }
 
 (define_insn "move_lo_quad_internal_<mode>"
-  [(set (match_operand:VQ 0 "register_operand" "=w,w,w")
-        (vec_concat:VQ
+  [(set (match_operand:VQ_NO2E 0 "register_operand" "=w,w,w")
+	(vec_concat:VQ_NO2E
 	  (match_operand:<VHALF> 1 "register_operand" "w,r,r")
 	  (vec_duplicate:<VHALF> (const_int 0))))]
   "TARGET_SIMD && !BYTES_BIG_ENDIAN"
@@ -968,9 +968,25 @@
    (set_attr "length" "4")]
 )
 
+(define_insn "move_lo_quad_internal_<mode>"
+  [(set (match_operand:VQ_2E 0 "register_operand" "=w,w,w")
+	(vec_concat:VQ_2E
+	  (match_operand:<VHALF> 1 "register_operand" "w,r,r")
+	  (const_int 0)))]
+  "TARGET_SIMD && !BYTES_BIG_ENDIAN"
+  "@
+   dup\\t%d0, %1.d[0]
+   fmov\\t%d0, %1
+   dup\\t%d0, %1"
+  [(set_attr "type" "neon_dup<q>,f_mcr,neon_dup<q>")
+   (set_attr "simd" "yes,*,yes")
+   (set_attr "fp" "*,yes,*")
+   (set_attr "length" "4")]
+)
+
 (define_insn "move_lo_quad_internal_be_<mode>"
-  [(set (match_operand:VQ 0 "register_operand" "=w,w,w")
-        (vec_concat:VQ
+  [(set (match_operand:VQ_NO2E 0 "register_operand" "=w,w,w")
+	(vec_concat:VQ_NO2E
 	  (vec_duplicate:<VHALF> (const_int 0))
 	  (match_operand:<VHALF> 1 "register_operand" "w,r,r")))]
   "TARGET_SIMD && BYTES_BIG_ENDIAN"
@@ -984,6 +1000,22 @@
    (set_attr "length" "4")]
 )
 
+(define_insn "move_lo_quad_internal_be_<mode>"
+  [(set (match_operand:VQ_2E 0 "register_operand" "=w,w,w")
+	(vec_concat:VQ_2E
+	  (const_int 0)
+	  (match_operand:<VHALF> 1 "register_operand" "w,r,r")))]
+  "TARGET_SIMD && BYTES_BIG_ENDIAN"
+  "@
+   dup\\t%d0, %1.d[0]
+   fmov\\t%d0, %1
+   dup\\t%d0, %1"
+  [(set_attr "type" "neon_dup<q>,f_mcr,neon_dup<q>")
+   (set_attr "simd" "yes,*,yes")
+   (set_attr "fp" "*,yes,*")
+   (set_attr "length" "4")]
+)
+
 (define_expand "move_lo_quad_<mode>"
   [(match_operand:VQ 0 "register_operand")
    (match_operand:VQ 1 "register_operand")]

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

* Re: [Patch AArch64] Fix for PR62040
  2014-08-19 23:43 [Patch AArch64] Fix for PR62040 Carrot Wei
@ 2014-08-20 11:27 ` Kyrill Tkachov
  2014-08-20 19:51   ` Carrot Wei
  2014-09-03 13:05 ` Marcus Shawcroft
  1 sibling, 1 reply; 8+ messages in thread
From: Kyrill Tkachov @ 2014-08-20 11:27 UTC (permalink / raw)
  To: Carrot Wei, gcc-patches; +Cc: Marcus Shawcroft, Richard Earnshaw

Hi Carrot,

cc'ing the aarch64 maintainers...

On 20/08/14 00:43, Carrot Wei wrote:
> Hi
>
> Current AArch64 backend can generate rtl expressions like
> (vec_duplicate:DI (const_int 0 [0])), which causes ICE in
> simplify_const_unary_operation because vec_duplicate should generate
> vector mode only.
>
> As suggested by Andrew in the bug entry, I split the original insn
> patterns to avoid scalar mode vec_duplicate expression.

The documentation does say that vec_concat can work on scalars, so it 
seems ok to me at a glance (but I can't approve it myself).

Would be nice to have an addition to the testsuite though...

Kyrill

> Passed regression tests on qemu without failure.
> OK for trunk and 4.9 branch?
>
> thanks
> Guozhi Wei
>
> 2014-08-19  Guozhi Wei  <carrot@google.com>
>
>          PR target/62040
>          * config/aarch64/iterators.md (VQ_NO2E, VQ_2E): New iterators.
>          * config/aarch64/aarch64-simd.md (move_lo_quad_internal_<mode>): Split
>          it into two patterns.
>          (move_lo_quad_internal_be_<mode>): Likewise.


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

* Re: [Patch AArch64] Fix for PR62040
  2014-08-20 11:27 ` Kyrill Tkachov
@ 2014-08-20 19:51   ` Carrot Wei
  2014-08-28 21:45     ` Carrot Wei
  2014-09-03 13:04     ` Marcus Shawcroft
  0 siblings, 2 replies; 8+ messages in thread
From: Carrot Wei @ 2014-08-20 19:51 UTC (permalink / raw)
  To: Kyrill Tkachov; +Cc: gcc-patches, Marcus Shawcroft, Richard Earnshaw

Good suggestion. Add the testcase.

thanks
Guozhi Wei

2014-08-20  Guozhi Wei  <carrot@google.com>

        PR target/62040
        * gcc.target/aarch64/pr62040.c: New test.

Index: pr62040.c
===================================================================
--- pr62040.c (revision 0)
+++ pr62040.c (revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-g -Os" } */
+
+#include "arm_neon.h"
+
+extern bar(int32x4_t);
+
+void foo() {
+  int32x4x4_t rows;
+  uint64x2x2_t row01;
+
+  row01.val[0] = vreinterpretq_u64_s32(rows.val[0]);
+  row01.val[1] = vreinterpretq_u64_s32(rows.val[1]);
+  uint64x1_t row3l = vget_low_u64(row01.val[0]);
+  row01.val[0] = vcombine_u64(vget_low_u64(row01.val[1]), row3l);
+  int32x4_t xxx = vreinterpretq_s32_u64(row01.val[0]);
+  int32x4_t out = vtrn1q_s32 (xxx, xxx);
+  bar(out);
+}

On Wed, Aug 20, 2014 at 4:26 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi Carrot,
>
> cc'ing the aarch64 maintainers...
>
>
> On 20/08/14 00:43, Carrot Wei wrote:
>>
>> Hi
>>
>> Current AArch64 backend can generate rtl expressions like
>> (vec_duplicate:DI (const_int 0 [0])), which causes ICE in
>> simplify_const_unary_operation because vec_duplicate should generate
>> vector mode only.
>>
>> As suggested by Andrew in the bug entry, I split the original insn
>> patterns to avoid scalar mode vec_duplicate expression.
>
>
> The documentation does say that vec_concat can work on scalars, so it seems
> ok to me at a glance (but I can't approve it myself).
>
> Would be nice to have an addition to the testsuite though...
>
> Kyrill
>
>
>> Passed regression tests on qemu without failure.
>> OK for trunk and 4.9 branch?
>>
>> thanks
>> Guozhi Wei
>>
>> 2014-08-19  Guozhi Wei  <carrot@google.com>
>>
>>          PR target/62040
>>          * config/aarch64/iterators.md (VQ_NO2E, VQ_2E): New iterators.
>>          * config/aarch64/aarch64-simd.md (move_lo_quad_internal_<mode>):
>> Split
>>          it into two patterns.
>>          (move_lo_quad_internal_be_<mode>): Likewise.
>
>
>

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

* Re: [Patch AArch64] Fix for PR62040
  2014-08-20 19:51   ` Carrot Wei
@ 2014-08-28 21:45     ` Carrot Wei
  2014-09-03 13:04     ` Marcus Shawcroft
  1 sibling, 0 replies; 8+ messages in thread
From: Carrot Wei @ 2014-08-28 21:45 UTC (permalink / raw)
  To: Marcus Shawcroft, Richard Earnshaw; +Cc: gcc-patches

AArch64 maintainers, could you help to review following patches?

https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01966.html
https://gcc.gnu.org/ml/gcc-patches/2014-08/msg02060.html

thanks
Guozhi Wei


On Wed, Aug 20, 2014 at 12:51 PM, Carrot Wei <carrot@google.com> wrote:
> Good suggestion. Add the testcase.
>
> thanks
> Guozhi Wei
>
> 2014-08-20  Guozhi Wei  <carrot@google.com>
>
>         PR target/62040
>         * gcc.target/aarch64/pr62040.c: New test.
>
> Index: pr62040.c
> ===================================================================
> --- pr62040.c (revision 0)
> +++ pr62040.c (revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-g -Os" } */
> +
> +#include "arm_neon.h"
> +
> +extern bar(int32x4_t);
> +
> +void foo() {
> +  int32x4x4_t rows;
> +  uint64x2x2_t row01;
> +
> +  row01.val[0] = vreinterpretq_u64_s32(rows.val[0]);
> +  row01.val[1] = vreinterpretq_u64_s32(rows.val[1]);
> +  uint64x1_t row3l = vget_low_u64(row01.val[0]);
> +  row01.val[0] = vcombine_u64(vget_low_u64(row01.val[1]), row3l);
> +  int32x4_t xxx = vreinterpretq_s32_u64(row01.val[0]);
> +  int32x4_t out = vtrn1q_s32 (xxx, xxx);
> +  bar(out);
> +}
>
> On Wed, Aug 20, 2014 at 4:26 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi Carrot,
>>
>> cc'ing the aarch64 maintainers...
>>
>>
>> On 20/08/14 00:43, Carrot Wei wrote:
>>>
>>> Hi
>>>
>>> Current AArch64 backend can generate rtl expressions like
>>> (vec_duplicate:DI (const_int 0 [0])), which causes ICE in
>>> simplify_const_unary_operation because vec_duplicate should generate
>>> vector mode only.
>>>
>>> As suggested by Andrew in the bug entry, I split the original insn
>>> patterns to avoid scalar mode vec_duplicate expression.
>>
>>
>> The documentation does say that vec_concat can work on scalars, so it seems
>> ok to me at a glance (but I can't approve it myself).
>>
>> Would be nice to have an addition to the testsuite though...
>>
>> Kyrill
>>
>>
>>> Passed regression tests on qemu without failure.
>>> OK for trunk and 4.9 branch?
>>>
>>> thanks
>>> Guozhi Wei
>>>
>>> 2014-08-19  Guozhi Wei  <carrot@google.com>
>>>
>>>          PR target/62040
>>>          * config/aarch64/iterators.md (VQ_NO2E, VQ_2E): New iterators.
>>>          * config/aarch64/aarch64-simd.md (move_lo_quad_internal_<mode>):
>>> Split
>>>          it into two patterns.
>>>          (move_lo_quad_internal_be_<mode>): Likewise.
>>
>>
>>

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

* Re: [Patch AArch64] Fix for PR62040
  2014-08-20 19:51   ` Carrot Wei
  2014-08-28 21:45     ` Carrot Wei
@ 2014-09-03 13:04     ` Marcus Shawcroft
  2014-09-04  5:34       ` Carrot Wei
  1 sibling, 1 reply; 8+ messages in thread
From: Marcus Shawcroft @ 2014-09-03 13:04 UTC (permalink / raw)
  To: Carrot Wei; +Cc: gcc-patches

On 20 August 2014 20:51, Carrot Wei <carrot@google.com> wrote:
> Good suggestion. Add the testcase.
>
> thanks
> Guozhi Wei
>
> 2014-08-20  Guozhi Wei  <carrot@google.com>
>
>         PR target/62040
>         * gcc.target/aarch64/pr62040.c: New test.
>
> Index: pr62040.c
> ===================================================================
> --- pr62040.c (revision 0)
> +++ pr62040.c (revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-g -Os" } */
> +
> +#include "arm_neon.h"
> +
> +extern bar(int32x4_t);
> +
> +void foo() {
> +  int32x4x4_t rows;
> +  uint64x2x2_t row01;
> +
> +  row01.val[0] = vreinterpretq_u64_s32(rows.val[0]);
> +  row01.val[1] = vreinterpretq_u64_s32(rows.val[1]);
> +  uint64x1_t row3l = vget_low_u64(row01.val[0]);
> +  row01.val[0] = vcombine_u64(vget_low_u64(row01.val[1]), row3l);
> +  int32x4_t xxx = vreinterpretq_s32_u64(row01.val[0]);
> +  int32x4_t out = vtrn1q_s32 (xxx, xxx);
> +  bar(out);
> +}


GNU coding style please.

/Marcus

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

* Re: [Patch AArch64] Fix for PR62040
  2014-08-19 23:43 [Patch AArch64] Fix for PR62040 Carrot Wei
  2014-08-20 11:27 ` Kyrill Tkachov
@ 2014-09-03 13:05 ` Marcus Shawcroft
  1 sibling, 0 replies; 8+ messages in thread
From: Marcus Shawcroft @ 2014-09-03 13:05 UTC (permalink / raw)
  To: Carrot Wei; +Cc: gcc-patches

On 20 August 2014 00:43, Carrot Wei <carrot@google.com> wrote:
> Hi
>
> Current AArch64 backend can generate rtl expressions like
> (vec_duplicate:DI (const_int 0 [0])), which causes ICE in
> simplify_const_unary_operation because vec_duplicate should generate
> vector mode only.
>
> As suggested by Andrew in the bug entry, I split the original insn
> patterns to avoid scalar mode vec_duplicate expression.
>
> Passed regression tests on qemu without failure.
> OK for trunk and 4.9 branch?
>
> thanks
> Guozhi Wei
>
> 2014-08-19  Guozhi Wei  <carrot@google.com>
>
>         PR target/62040
>         * config/aarch64/iterators.md (VQ_NO2E, VQ_2E): New iterators.
>         * config/aarch64/aarch64-simd.md (move_lo_quad_internal_<mode>): Split
>         it into two patterns.
>         (move_lo_quad_internal_be_<mode>): Likewise.

OK once the test case is approved.
Thanks
/Marcus

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

* Re: [Patch AArch64] Fix for PR62040
  2014-09-03 13:04     ` Marcus Shawcroft
@ 2014-09-04  5:34       ` Carrot Wei
  2014-09-04 13:31         ` Marcus Shawcroft
  0 siblings, 1 reply; 8+ messages in thread
From: Carrot Wei @ 2014-09-04  5:34 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: gcc-patches

Changed the coding style.


2014-09-03  Guozhi Wei  <carrot@google.com>

        PR target/62040
        * gcc.target/aarch64/pr62040.c: New test.


Index: pr62040.c
===================================================================
--- pr62040.c (revision 0)
+++ pr62040.c (revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-g -Os" } */
+
+#include "arm_neon.h"
+
+extern void bar (int32x4_t);
+
+void
+foo ()
+{
+  int32x4x4_t rows;
+  uint64x2x2_t row01;
+
+  row01.val[0] = vreinterpretq_u64_s32 (rows.val[0]);
+  row01.val[1] = vreinterpretq_u64_s32 (rows.val[1]);
+  uint64x1_t row3l = vget_low_u64 (row01.val[0]);
+  row01.val[0] = vcombine_u64 (vget_low_u64 (row01.val[1]), row3l);
+  int32x4_t xxx = vreinterpretq_s32_u64 (row01.val[0]);
+  int32x4_t out = vtrn1q_s32 (xxx, xxx);
+  bar (out);
+}


On Wed, Sep 3, 2014 at 6:04 AM, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
> On 20 August 2014 20:51, Carrot Wei <carrot@google.com> wrote:
>> Good suggestion. Add the testcase.
>>
>> thanks
>> Guozhi Wei
>>
>> 2014-08-20  Guozhi Wei  <carrot@google.com>
>>
>>         PR target/62040
>>         * gcc.target/aarch64/pr62040.c: New test.
>>
>> Index: pr62040.c
>> ===================================================================
>> --- pr62040.c (revision 0)
>> +++ pr62040.c (revision 0)
>> @@ -0,0 +1,19 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-g -Os" } */
>> +
>> +#include "arm_neon.h"
>> +
>> +extern bar(int32x4_t);
>> +
>> +void foo() {
>> +  int32x4x4_t rows;
>> +  uint64x2x2_t row01;
>> +
>> +  row01.val[0] = vreinterpretq_u64_s32(rows.val[0]);
>> +  row01.val[1] = vreinterpretq_u64_s32(rows.val[1]);
>> +  uint64x1_t row3l = vget_low_u64(row01.val[0]);
>> +  row01.val[0] = vcombine_u64(vget_low_u64(row01.val[1]), row3l);
>> +  int32x4_t xxx = vreinterpretq_s32_u64(row01.val[0]);
>> +  int32x4_t out = vtrn1q_s32 (xxx, xxx);
>> +  bar(out);
>> +}
>
>
> GNU coding style please.
>
> /Marcus

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

* Re: [Patch AArch64] Fix for PR62040
  2014-09-04  5:34       ` Carrot Wei
@ 2014-09-04 13:31         ` Marcus Shawcroft
  0 siblings, 0 replies; 8+ messages in thread
From: Marcus Shawcroft @ 2014-09-04 13:31 UTC (permalink / raw)
  To: Carrot Wei; +Cc: gcc-patches

On 4 September 2014 06:34, Carrot Wei <carrot@google.com> wrote:
> Changed the coding style.
>
>
> 2014-09-03  Guozhi Wei  <carrot@google.com>
>
>         PR target/62040
>         * gcc.target/aarch64/pr62040.c: New test.

Thank you. OK /Marcus

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

end of thread, other threads:[~2014-09-04 13:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-19 23:43 [Patch AArch64] Fix for PR62040 Carrot Wei
2014-08-20 11:27 ` Kyrill Tkachov
2014-08-20 19:51   ` Carrot Wei
2014-08-28 21:45     ` Carrot Wei
2014-09-03 13:04     ` Marcus Shawcroft
2014-09-04  5:34       ` Carrot Wei
2014-09-04 13:31         ` Marcus Shawcroft
2014-09-03 13:05 ` 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).