Patchwork [AArch64] Implement Vector Permute Support

login
register
mail settings
Submitter Alex Velenko
Date Jan. 16, 2014, 2:43 p.m.
Message ID <52D7F00D.1030206@arm.com>
Download mbox | patch
Permalink /patch/311739/
State New
Headers show

Comments

Alex Velenko - Jan. 16, 2014, 2:43 p.m.
On 14/01/14 15:51, pinskia@gmail.com wrote:
>
>
>> On Jan 14, 2014, at 7:19 AM, Alex Velenko <Alex.Velenko@arm.com> wrote:
>>
>> Hi,
>>
>> This patch turns off the vec_perm patterns for aarch64_be, this should resolve
>> the issue  highlighted here http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00321.html
>> With this patch applied, the test case provided in that link compiles without an ICE.
>>
>> However, the Big-Endian port is still in development. This patch exposes
>> another known but unrelated issue with Big-Endian Large-Int modes.
>>
>> The patch has been tested on aarch64-none-elf and aarch64_be-none-elf resulting in five
>> further regression due to the broken implementation of Big-Endian Large-Int modes.
>>
>> Kind regards,
>> Alex Velenko
>>
>> gcc/
>>
>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>
>>     * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>     * config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>
>> gcc/testsuite/
>>
>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>
>>     * lib/target-supports.exp
>>     (check_effective_target_vect_perm): Exclude aarch64_be.
>>     (check_effective_target_vect_perm_byte): Likewise.
>>     (check_effective_target_vect_perm_short): Likewise.
>
> I think you want to use a function to check if the target is effectively big-endian instead.  Internally at Cavium, our elf compiler has big-endian multi-lib.
>
> Thanks,
> Andrew
>
>>
>> <vec-perm.patch>
>

Hi,
Here is a vec-perm patch with changes proposed previously.
Little and Big-Endian tested with no additional issues appearing.

Kind regards,
Alex

gcc/

2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>

	* config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
	* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.

gcc/testsuite/

2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>

	* lib/target-supports.exp
	(check_effective_target_vect_perm): Exclude aarch64_be.
	(check_effective_target_vect_perm_byte): Likewise.
	(check_effective_target_vect_perm_short): Likewise.
Richard Earnshaw - Jan. 17, 2014, 3:55 p.m.
On 16/01/14 14:43, Alex Velenko wrote:
> On 14/01/14 15:51, pinskia@gmail.com wrote:
>>
>>
>>> On Jan 14, 2014, at 7:19 AM, Alex Velenko <Alex.Velenko@arm.com> wrote:
>>>
>>> Hi,
>>>
>>> This patch turns off the vec_perm patterns for aarch64_be, this should resolve
>>> the issue  highlighted here http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00321.html
>>> With this patch applied, the test case provided in that link compiles without an ICE.
>>>
>>> However, the Big-Endian port is still in development. This patch exposes
>>> another known but unrelated issue with Big-Endian Large-Int modes.
>>>
>>> The patch has been tested on aarch64-none-elf and aarch64_be-none-elf resulting in five
>>> further regression due to the broken implementation of Big-Endian Large-Int modes.
>>>
>>> Kind regards,
>>> Alex Velenko
>>>
>>> gcc/
>>>
>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>
>>>     * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>>     * config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>
>>> gcc/testsuite/
>>>
>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>
>>>     * lib/target-supports.exp
>>>     (check_effective_target_vect_perm): Exclude aarch64_be.
>>>     (check_effective_target_vect_perm_byte): Likewise.
>>>     (check_effective_target_vect_perm_short): Likewise.
>>
>> I think you want to use a function to check if the target is effectively big-endian instead.  Internally at Cavium, our elf compiler has big-endian multi-lib.
>>
>> Thanks,
>> Andrew
>>
>>>
>>> <vec-perm.patch>
>>
> 
> Hi,
> Here is a vec-perm patch with changes proposed previously.
> Little and Big-Endian tested with no additional issues appearing.
> 
> Kind regards,
> Alex
> 
> gcc/
> 
> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
> 
> 	* config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
> 	* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
> 
> gcc/testsuite/
> 
> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
> 
> 	* lib/target-supports.exp
> 	(check_effective_target_vect_perm): Exclude aarch64_be.
> 	(check_effective_target_vect_perm_byte): Likewise.
> 	(check_effective_target_vect_perm_short): Likewise.
> 

The patch is missing the hunk for aarch64.c.
Alex Velenko - Jan. 20, 2014, 11:15 a.m.
On 17/01/14 15:55, Richard Earnshaw wrote:
> On 16/01/14 14:43, Alex Velenko wrote:
>> On 14/01/14 15:51, pinskia@gmail.com wrote:
>>>
>>>
>>>> On Jan 14, 2014, at 7:19 AM, Alex Velenko <Alex.Velenko@arm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This patch turns off the vec_perm patterns for aarch64_be, this should resolve
>>>> the issue  highlighted here http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00321.html
>>>> With this patch applied, the test case provided in that link compiles without an ICE.
>>>>
>>>> However, the Big-Endian port is still in development. This patch exposes
>>>> another known but unrelated issue with Big-Endian Large-Int modes.
>>>>
>>>> The patch has been tested on aarch64-none-elf and aarch64_be-none-elf resulting in five
>>>> further regression due to the broken implementation of Big-Endian Large-Int modes.
>>>>
>>>> Kind regards,
>>>> Alex Velenko
>>>>
>>>> gcc/
>>>>
>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>
>>>>      * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>>>      * config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>>
>>>> gcc/testsuite/
>>>>
>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>
>>>>      * lib/target-supports.exp
>>>>      (check_effective_target_vect_perm): Exclude aarch64_be.
>>>>      (check_effective_target_vect_perm_byte): Likewise.
>>>>      (check_effective_target_vect_perm_short): Likewise.
>>>
>>> I think you want to use a function to check if the target is effectively big-endian instead.  Internally at Cavium, our elf compiler has big-endian multi-lib.
>>>
>>> Thanks,
>>> Andrew
>>>
>>>>
>>>> <vec-perm.patch>
>>>
>>
>> Hi,
>> Here is a vec-perm patch with changes proposed previously.
>> Little and Big-Endian tested with no additional issues appearing.
>>
>> Kind regards,
>> Alex
>>
>> gcc/
>>
>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>
>> 	* config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>> 	* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>
>> gcc/testsuite/
>>
>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>
>> 	* lib/target-supports.exp
>> 	(check_effective_target_vect_perm): Exclude aarch64_be.
>> 	(check_effective_target_vect_perm_byte): Likewise.
>> 	(check_effective_target_vect_perm_short): Likewise.
>>
>
> The patch is missing the hunk for aarch64.c.
>
>

Hi,
It is a faulty changelog entry, not patch.
Should be:

gcc/

2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>

     * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.

gcc/testsuite/

2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>

     * lib/target-supports.exp
     (check_effective_target_vect_perm): Exclude aarch64_be.
     (check_effective_target_vect_perm_byte): Likewise.
     (check_effective_target_vect_perm_short): Likewise.
Richard Earnshaw - Jan. 20, 2014, 11:16 a.m.
On 20/01/14 11:15, Alex Velenko wrote:
> On 17/01/14 15:55, Richard Earnshaw wrote:
>> On 16/01/14 14:43, Alex Velenko wrote:
>>> On 14/01/14 15:51, pinskia@gmail.com wrote:
>>>>
>>>>
>>>>> On Jan 14, 2014, at 7:19 AM, Alex Velenko <Alex.Velenko@arm.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> This patch turns off the vec_perm patterns for aarch64_be, this should resolve
>>>>> the issue  highlighted here http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00321.html
>>>>> With this patch applied, the test case provided in that link compiles without an ICE.
>>>>>
>>>>> However, the Big-Endian port is still in development. This patch exposes
>>>>> another known but unrelated issue with Big-Endian Large-Int modes.
>>>>>
>>>>> The patch has been tested on aarch64-none-elf and aarch64_be-none-elf resulting in five
>>>>> further regression due to the broken implementation of Big-Endian Large-Int modes.
>>>>>
>>>>> Kind regards,
>>>>> Alex Velenko
>>>>>
>>>>> gcc/
>>>>>
>>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>>
>>>>>      * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>>>>      * config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>>>
>>>>> gcc/testsuite/
>>>>>
>>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>>
>>>>>      * lib/target-supports.exp
>>>>>      (check_effective_target_vect_perm): Exclude aarch64_be.
>>>>>      (check_effective_target_vect_perm_byte): Likewise.
>>>>>      (check_effective_target_vect_perm_short): Likewise.
>>>>
>>>> I think you want to use a function to check if the target is effectively big-endian instead.  Internally at Cavium, our elf compiler has big-endian multi-lib.
>>>>
>>>> Thanks,
>>>> Andrew
>>>>
>>>>>
>>>>> <vec-perm.patch>
>>>>
>>>
>>> Hi,
>>> Here is a vec-perm patch with changes proposed previously.
>>> Little and Big-Endian tested with no additional issues appearing.
>>>
>>> Kind regards,
>>> Alex
>>>
>>> gcc/
>>>
>>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>>
>>> 	* config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>> 	* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>
>>> gcc/testsuite/
>>>
>>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>>
>>> 	* lib/target-supports.exp
>>> 	(check_effective_target_vect_perm): Exclude aarch64_be.
>>> 	(check_effective_target_vect_perm_byte): Likewise.
>>> 	(check_effective_target_vect_perm_short): Likewise.
>>>
>>
>> The patch is missing the hunk for aarch64.c.
>>
>>
> 
> Hi,
> It is a faulty changelog entry, not patch.
> Should be:
> 
> gcc/
> 
> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
> 
>      * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
> 
> gcc/testsuite/
> 
> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
> 
>      * lib/target-supports.exp
>      (check_effective_target_vect_perm): Exclude aarch64_be.
>      (check_effective_target_vect_perm_byte): Likewise.
>      (check_effective_target_vect_perm_short): Likewise.
> 

On that basis, OK.

R.
Alex Velenko - Jan. 20, 2014, 5:33 p.m.
On 20/01/14 11:16, Richard Earnshaw wrote:
> On 20/01/14 11:15, Alex Velenko wrote:
>> On 17/01/14 15:55, Richard Earnshaw wrote:
>>> On 16/01/14 14:43, Alex Velenko wrote:
>>>> On 14/01/14 15:51, pinskia@gmail.com wrote:
>>>>>
>>>>>
>>>>>> On Jan 14, 2014, at 7:19 AM, Alex Velenko <Alex.Velenko@arm.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> This patch turns off the vec_perm patterns for aarch64_be, this should resolve
>>>>>> the issue  highlighted here http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00321.html
>>>>>> With this patch applied, the test case provided in that link compiles without an ICE.
>>>>>>
>>>>>> However, the Big-Endian port is still in development. This patch exposes
>>>>>> another known but unrelated issue with Big-Endian Large-Int modes.
>>>>>>
>>>>>> The patch has been tested on aarch64-none-elf and aarch64_be-none-elf resulting in five
>>>>>> further regression due to the broken implementation of Big-Endian Large-Int modes.
>>>>>>
>>>>>> Kind regards,
>>>>>> Alex Velenko
>>>>>>
>>>>>> gcc/
>>>>>>
>>>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>>>
>>>>>>       * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>>>>>       * config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>>>>
>>>>>> gcc/testsuite/
>>>>>>
>>>>>> 2014-01-14  Alex Velenko  <Alex.Velenko@arm.com>
>>>>>>
>>>>>>       * lib/target-supports.exp
>>>>>>       (check_effective_target_vect_perm): Exclude aarch64_be.
>>>>>>       (check_effective_target_vect_perm_byte): Likewise.
>>>>>>       (check_effective_target_vect_perm_short): Likewise.
>>>>>
>>>>> I think you want to use a function to check if the target is effectively big-endian instead.  Internally at Cavium, our elf compiler has big-endian multi-lib.
>>>>>
>>>>> Thanks,
>>>>> Andrew
>>>>>
>>>>>>
>>>>>> <vec-perm.patch>
>>>>>
>>>>
>>>> Hi,
>>>> Here is a vec-perm patch with changes proposed previously.
>>>> Little and Big-Endian tested with no additional issues appearing.
>>>>
>>>> Kind regards,
>>>> Alex
>>>>
>>>> gcc/
>>>>
>>>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>>>
>>>> 	* config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>>> 	* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Add comment.
>>>>
>>>> gcc/testsuite/
>>>>
>>>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>>>
>>>> 	* lib/target-supports.exp
>>>> 	(check_effective_target_vect_perm): Exclude aarch64_be.
>>>> 	(check_effective_target_vect_perm_byte): Likewise.
>>>> 	(check_effective_target_vect_perm_short): Likewise.
>>>>
>>>
>>> The patch is missing the hunk for aarch64.c.
>>>
>>>
>>
>> Hi,
>> It is a faulty changelog entry, not patch.
>> Should be:
>>
>> gcc/
>>
>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>
>>       * config/aarch64/aarch64-simd.md (vec_perm<mode>): Add BE check.
>>
>> gcc/testsuite/
>>
>> 2014-01-16  Alex Velenko  <Alex.Velenko@arm.com>
>>
>>       * lib/target-supports.exp
>>       (check_effective_target_vect_perm): Exclude aarch64_be.
>>       (check_effective_target_vect_perm_byte): Likewise.
>>       (check_effective_target_vect_perm_short): Likewise.
>>
>
> On that basis, OK.
>
> R.
>

Can someone, please, commit this patch, as I do not have permissions?
Kind regards,
Alex
Marcus Shawcroft - Jan. 20, 2014, 6:36 p.m.
On 20 January 2014 17:33, Alex Velenko <Alex.Velenko@arm.com> wrote:

> Can someone, please, commit this patch, as I do not have permissions?
> Kind regards,
> Alex

Committed.

/Marcus

Patch

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index bc47a291de4b9b24d829e4dbf060fff7a321558f..43a9c5b27d78a47cf965636a03232005a4c8e7c3 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3840,7 +3840,7 @@ 
    (match_operand:VB 1 "register_operand")
    (match_operand:VB 2 "register_operand")
    (match_operand:VB 3 "register_operand")]
-  "TARGET_SIMD"
+  "TARGET_SIMD && !BYTES_BIG_ENDIAN"
 {
   aarch64_expand_vec_perm (operands[0], operands[1],
 			   operands[2], operands[3]);
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 159f88f28dd838d4aee6d75f8d21897695609c49..b425183c1e893c6511ba575a0cd416563c9510be 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3436,7 +3436,8 @@  proc check_effective_target_vect_perm { } {
     } else {
         set et_vect_perm_saved 0
         if { [is-effective-target arm_neon_ok]
-	     || [istarget aarch64*-*-*]
+	     || ([istarget aarch64*-*-*]
+		 && [is-effective-target aarch64_little_endian])
 	     || [istarget powerpc*-*-*]
              || [istarget spu-*-*]
 	     || [istarget i?86-*-*]
@@ -3464,7 +3465,8 @@  proc check_effective_target_vect_perm_byte { } {
         set et_vect_perm_byte_saved 0
         if { ([is-effective-target arm_neon_ok]
 	      && [is-effective-target arm_little_endian])
-	     || [istarget aarch64*-*-*]
+	     || ([istarget aarch64*-*-*]
+		 && [is-effective-target aarch64_little_endian])
 	     || [istarget powerpc*-*-*]
              || [istarget spu-*-*] } {
             set et_vect_perm_byte_saved 1
@@ -3488,7 +3490,8 @@  proc check_effective_target_vect_perm_short { } {
         set et_vect_perm_short_saved 0
         if { ([is-effective-target arm_neon_ok]
 	      && [is-effective-target arm_little_endian])
-	     || [istarget aarch64*-*-*]
+	     || ([istarget aarch64*-*-*]
+		 && [is-effective-target aarch64_little_endian])
 	     || [istarget powerpc*-*-*]
              || [istarget spu-*-*] } {
             set et_vect_perm_short_saved 1