public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
@ 2015-04-28 16:01 Renlin Li
  2015-04-29 12:10 ` Marcus Shawcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Renlin Li @ 2015-04-28 16:01 UTC (permalink / raw)
  To: gcc-patches
  Cc: Alan Lawrence, Marcus Shawcroft, James.Greenhalgh, Ramana Radhakrishnan

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

Hi all,

unsigned shift left dosen't support immediate shift. Previouly, gcc will 
generate asm instruction like this: "ushl d1, d0, 32", which is not a 
legal insn and will be rejected by assembler. This patch change the use 
of ushl in vec_shr_<mode> into shl.

A test case is added, and it passes on both aarch64-none-elf and 
aarch64_be-none-elf tool-chain.

Okay to commit?

Regards,
Renlin Li

gcc/ChangeLog:

2015-04-28  Renlin Li  <renlin.li@arm.com>

     * config/aarch64/aarch64-simd.md (vec_shr_<mode>): Use shl.

gcc/testsuite/ChangeLog:

2015-04-28  Renlin Li  <renlin.li@arm.com>
                     Alan Lawrence  <alan.lawrence@arm.com>

     * gcc.target/aarch64/vect-reduc-or_1.c: New.

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

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 0557570..41706fb 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -788,7 +788,7 @@
   "TARGET_SIMD"
   {
     if (BYTES_BIG_ENDIAN)
-      return "ushl %d0, %d1, %2";
+      return "shl %d0, %d1, %2";
     else
       return "ushr %d0, %d1, %2";
   }
diff --git a/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c b/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c
new file mode 100644
index 0000000..f5d9460
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-all" } */
+/* Write a reduction loop to be reduced using whole vector right shift.  */
+
+extern void abort(void);
+
+unsigned char in[8] __attribute__((__aligned__(16)));
+
+int
+main (unsigned char argc, char **argv)
+{
+  unsigned char i = 0;
+  unsigned char sum = 1;
+
+  for (i = 0; i < 8; i++)
+    in[i] = (i + i + 1) & 0xfd;
+
+  /* Prevent constant propagation of the entire loop below.  */
+  asm volatile ("" : : : "memory");
+
+  for (i = 0; i < 8; i++)
+    sum |= in[i];
+
+  if (sum != 13)
+    {
+      __builtin_printf("Failed %d\n", sum);
+      abort();
+    }
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Reduce using vector shifts" "vect" } } */
+/* { dg-final { cleanup-tree-dump "vect" } } */

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

* Re: [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
  2015-04-28 16:01 [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern Renlin Li
@ 2015-04-29 12:10 ` Marcus Shawcroft
  2015-04-30 11:58   ` Renlin Li
  0 siblings, 1 reply; 6+ messages in thread
From: Marcus Shawcroft @ 2015-04-29 12:10 UTC (permalink / raw)
  To: Renlin Li; +Cc: gcc-patches, Alan Lawrence

On 28 April 2015 at 16:41, Renlin Li <renlin.li@arm.com> wrote:
> Hi all,
>
> unsigned shift left dosen't support immediate shift. Previouly, gcc will
> generate asm instruction like this: "ushl d1, d0, 32", which is not a legal
> insn and will be rejected by assembler. This patch change the use of ushl in
> vec_shr_<mode> into shl.
>
> A test case is added, and it passes on both aarch64-none-elf and
> aarch64_be-none-elf tool-chain.
>
> Okay to commit?
>
> Regards,
> Renlin Li
>
> gcc/ChangeLog:
>
> 2015-04-28  Renlin Li  <renlin.li@arm.com>
>
>     * config/aarch64/aarch64-simd.md (vec_shr_<mode>): Use shl.
>
> gcc/testsuite/ChangeLog:
>
> 2015-04-28  Renlin Li  <renlin.li@arm.com>
>                     Alan Lawrence  <alan.lawrence@arm.com>
>
>     * gcc.target/aarch64/vect-reduc-or_1.c: New.


I think there is another issue here, this change:

     if (BYTES_BIG_ENDIAN)
-      return "ushl %d0, %d1, %2";
+      return "shl %d0, %d1, %2";
     else
       return "ushr %d0, %d1, %2";

is in the context of:

(define_insn "vec_shr_<mode>"
  [(set (match_operand:VD 0 "register_operand" "=w")
        (lshiftrt:VD (match_operand:VD 1 "register_operand" "w")
                     (match_operand:SI 2 "immediate_operand" "i")))]

The RTL describes a right shift of the bits within each element in the
vector while the optab expects  a right shift of the elements within
the vector?

/Marcus

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

* Re: [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
  2015-04-29 12:10 ` Marcus Shawcroft
@ 2015-04-30 11:58   ` Renlin Li
  2015-04-30 15:23     ` Marcus Shawcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Renlin Li @ 2015-04-30 11:58 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: gcc-patches, Alan Lawrence

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

Hi Marcus,

On 29/04/15 13:06, Marcus Shawcroft wrote:
> I think there is another issue here, this change:
>
>       if (BYTES_BIG_ENDIAN)
> -      return "ushl %d0, %d1, %2";
> +      return "shl %d0, %d1, %2";
>       else
>         return "ushr %d0, %d1, %2";
>
> is in the context of:
>
> (define_insn "vec_shr_<mode>"
>    [(set (match_operand:VD 0 "register_operand" "=w")
>          (lshiftrt:VD (match_operand:VD 1 "register_operand" "w")
>                       (match_operand:SI 2 "immediate_operand" "i")))]

You are right. This pattern has ambiguity. I have updated the patch, and 
represent vec_shr as an upspec. This will prevent other rtx patterns 
implicitly matching this one.

The new patch is attached, is it Okay to commit?

Regards,
Renlin Li

gcc/ChangeLog:

2015-04-30  Renlin Li  <renlin.li@arm.com>

     * config/aarch64/aarch64-simd.md (vec_shr): Defined as an unspec.
     * config/aarch64/iterators.md (unspec): Add UNSPEC_VEC_SHR.

gcc/testsuite/ChangeLog:

2015-04-30  Renlin Li  <renlin.li@arm.com>

     * gcc.target/aarch64/vect-reduc-or_1.c: New.

> The RTL describes a right shift of the bits within each element in the
> vector while the optab expects  a right shift of the elements within
> the vector?
>
> /Marcus
>

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

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 0557570..6304eae6 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -783,12 +783,13 @@
 ;; For 64-bit modes we use ushl/r, as this does not require a SIMD zero.
 (define_insn "vec_shr_<mode>"
   [(set (match_operand:VD 0 "register_operand" "=w")
-        (lshiftrt:VD (match_operand:VD 1 "register_operand" "w")
-		     (match_operand:SI 2 "immediate_operand" "i")))]
+        (unspec:VD [(match_operand:VD 1 "register_operand" "w")
+		    (match_operand:SI 2 "immediate_operand" "i")]
+		   UNSPEC_VEC_SHR))]
   "TARGET_SIMD"
   {
     if (BYTES_BIG_ENDIAN)
-      return "ushl %d0, %d1, %2";
+      return "shl %d0, %d1, %2";
     else
       return "ushr %d0, %d1, %2";
   }
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 1fdff04..498358a 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -278,6 +278,7 @@
     UNSPEC_PMULL        ; Used in aarch64-simd.md.
     UNSPEC_PMULL2       ; Used in aarch64-simd.md.
     UNSPEC_REV_REGLIST  ; Used in aarch64-simd.md.
+    UNSPEC_VEC_SHR      ; Used in aarch64-simd.md.
 ])
 
 ;; -------------------------------------------------------------------
diff --git a/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c b/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c
new file mode 100644
index 0000000..f5d9460
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/vect-reduc-or_1.c
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -ftree-vectorize -fdump-tree-vect-all" } */
+/* Write a reduction loop to be reduced using whole vector right shift.  */
+
+extern void abort (void);
+
+unsigned char in[8] __attribute__((__aligned__(16)));
+
+int
+main (unsigned char argc, char **argv)
+{
+  unsigned char i = 0;
+  unsigned char sum = 1;
+
+  for (i = 0; i < 8; i++)
+    in[i] = (i + i + 1) & 0xfd;
+
+  /* Prevent constant propagation of the entire loop below.  */
+  asm volatile ("" : : : "memory");
+
+  for (i = 0; i < 8; i++)
+    sum |= in[i];
+
+  if (sum != 13)
+    {
+      __builtin_printf("Failed %d\n", sum);
+      abort();
+    }
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Reduce using vector shifts" "vect" } } */
+/* { dg-final { cleanup-tree-dump "vect" } } */

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

* Re: [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
  2015-04-30 11:58   ` Renlin Li
@ 2015-04-30 15:23     ` Marcus Shawcroft
  2015-06-02  9:30       ` Renlin Li
  0 siblings, 1 reply; 6+ messages in thread
From: Marcus Shawcroft @ 2015-04-30 15:23 UTC (permalink / raw)
  To: Renlin Li; +Cc: gcc-patches, Alan Lawrence

On 30 April 2015 at 12:55, Renlin Li <renlin.li@arm.com> wrote:

> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>
>     * config/aarch64/aarch64-simd.md (vec_shr): Defined as an unspec.
>     * config/aarch64/iterators.md (unspec): Add UNSPEC_VEC_SHR.
>
> gcc/testsuite/ChangeLog:
>
> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>
>     * gcc.target/aarch64/vect-reduc-or_1.c: New.

+      __builtin_printf("Failed %d\n", sum);
+      abort();

Space before (
Otherwise OK /Marcus

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

* Re: [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
  2015-04-30 15:23     ` Marcus Shawcroft
@ 2015-06-02  9:30       ` Renlin Li
  2015-06-02  9:48         ` Marcus Shawcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Renlin Li @ 2015-06-02  9:30 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: gcc-patches, Alan Lawrence

Is it Okay for me to backport it to gcc-5?

Regards,
Renlin Li

On 30/04/15 16:21, Marcus Shawcroft wrote:
> On 30 April 2015 at 12:55, Renlin Li <renlin.li@arm.com> wrote:
>
>> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>>
>>      * config/aarch64/aarch64-simd.md (vec_shr): Defined as an unspec.
>>      * config/aarch64/iterators.md (unspec): Add UNSPEC_VEC_SHR.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>>
>>      * gcc.target/aarch64/vect-reduc-or_1.c: New.
> +      __builtin_printf("Failed %d\n", sum);
> +      abort();
>
> Space before (
> Otherwise OK /Marcus
>

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

* Re: [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern.
  2015-06-02  9:30       ` Renlin Li
@ 2015-06-02  9:48         ` Marcus Shawcroft
  0 siblings, 0 replies; 6+ messages in thread
From: Marcus Shawcroft @ 2015-06-02  9:48 UTC (permalink / raw)
  To: Renlin Li; +Cc: gcc-patches, Alan Lawrence

On 2 June 2015 at 10:30, Renlin Li <renlin.li@arm.com> wrote:
> Is it Okay for me to backport it to gcc-5?

OK provided the patch applies cleanly and there are no regressions. /Marcus

>
> Regards,
> Renlin Li
>
>
> On 30/04/15 16:21, Marcus Shawcroft wrote:
>>
>> On 30 April 2015 at 12:55, Renlin Li <renlin.li@arm.com> wrote:
>>
>>> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>>>
>>>      * config/aarch64/aarch64-simd.md (vec_shr): Defined as an unspec.
>>>      * config/aarch64/iterators.md (unspec): Add UNSPEC_VEC_SHR.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2015-04-30  Renlin Li  <renlin.li@arm.com>
>>>
>>>      * gcc.target/aarch64/vect-reduc-or_1.c: New.
>>
>> +      __builtin_printf("Failed %d\n", sum);
>> +      abort();
>>
>> Space before (
>> Otherwise OK /Marcus
>>
>

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

end of thread, other threads:[~2015-06-02  9:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-28 16:01 [PATCH][AARCH64]Use shl for vec_shr_<mode> rtx pattern Renlin Li
2015-04-29 12:10 ` Marcus Shawcroft
2015-04-30 11:58   ` Renlin Li
2015-04-30 15:23     ` Marcus Shawcroft
2015-06-02  9:30       ` Renlin Li
2015-06-02  9:48         ` 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).