diff mbox

[trivial,AArch64] Fix mode iterator for *aarch64_simd_ld1r<mode> pattern

Message ID DA41BE1DDCA941489001C7FBD7A8820E55556F8D@szxema507-mbx.china.huawei.com
State New
Headers show

Commit Message

Yangfei (Felix) Nov. 13, 2014, 6:14 a.m. UTC 
Hi,

  We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode> pattern is not appropriate. 
  The reason is that it's impossible to get a new operand of DImode by vec_duplicating an operand of the same mode. 
  So this patch just excludes the DImode and uses VALL instead. 
  Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?

Comments

Joey Ye Nov. 14, 2014, 1:27 a.m. UTC | #1 
Is there a case or PR to demonstrate the issue? If yes, better to
include it as a test case.

Thanks,
Joey

On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> Hi,
>
>   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode> pattern is not appropriate.
>   The reason is that it's impossible to get a new operand of DImode by vec_duplicating an operand of the same mode.
>   So this patch just excludes the DImode and uses VALL instead.
>   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?
>
>
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog       (revision 217394)
> +++ gcc/ChangeLog       (working copy)
> @@ -1,3 +1,9 @@
> +2014-11-13  Felix Yang  <felix.yang@huawei.com>
> +       Jiji Jiang  <jiangjiji@huawei.com>
> +
> +       * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r<mode>): Use
> +       VALL mode iterator instead of VALLDI.
> +
>  2014-11-11  Andrew Pinski  <apinski@cavium.com>
>
>         Bug target/61997
> Index: gcc/config/aarch64/aarch64-simd.md
> ===================================================================
> --- gcc/config/aarch64/aarch64-simd.md  (revision 217394)
> +++ gcc/config/aarch64/aarch64-simd.md  (working copy)
> @@ -4936,8 +4936,8 @@
>  })
>
>  (define_insn "*aarch64_simd_ld1r<mode>"
> -  [(set (match_operand:VALLDI 0 "register_operand" "=w")
> -       (vec_duplicate:VALLDI
> +  [(set (match_operand:VALL 0 "register_operand" "=w")
> +       (vec_duplicate:VALL
>           (match_operand:<VEL> 1 "aarch64_simd_struct_operand" "Utv")))]
>    "TARGET_SIMD"
>    "ld1r\\t{%0.<Vtype>}, %1"
Yangfei (Felix) Nov. 14, 2014, 1:32 a.m. UTC | #2 
No, we noticed this issue when improving the vld1(q?)_dup intrinsics.  Thanks. 


> Is there a case or PR to demonstrate the issue? If yes, better to include it as a test

> case.

> 

> Thanks,

> Joey

> 

> On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix) <felix.yang@huawei.com>

> wrote:

> > Hi,

> >

> >   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode>

> pattern is not appropriate.

> >   The reason is that it's impossible to get a new operand of DImode by

> vec_duplicating an operand of the same mode.

> >   So this patch just excludes the DImode and uses VALL instead.

> >   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?

> >

> >

> > Index: gcc/ChangeLog

> >

> =============================================================

> ======

> > --- gcc/ChangeLog       (revision 217394)

> > +++ gcc/ChangeLog       (working copy)

> > @@ -1,3 +1,9 @@

> > +2014-11-13  Felix Yang  <felix.yang@huawei.com>

> > +       Jiji Jiang  <jiangjiji@huawei.com>

> > +

> > +       * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r<mode>):

> Use

> > +       VALL mode iterator instead of VALLDI.

> > +

> >  2014-11-11  Andrew Pinski  <apinski@cavium.com>

> >

> >         Bug target/61997

> > Index: gcc/config/aarch64/aarch64-simd.md

> >

> =============================================================

> ======

> > --- gcc/config/aarch64/aarch64-simd.md  (revision 217394)

> > +++ gcc/config/aarch64/aarch64-simd.md  (working copy)

> > @@ -4936,8 +4936,8 @@

> >  })

> >

> >  (define_insn "*aarch64_simd_ld1r<mode>"

> > -  [(set (match_operand:VALLDI 0 "register_operand" "=w")

> > -       (vec_duplicate:VALLDI

> > +  [(set (match_operand:VALL 0 "register_operand" "=w")

> > +       (vec_duplicate:VALL

> >           (match_operand:<VEL> 1 "aarch64_simd_struct_operand"

> "Utv")))]

> >    "TARGET_SIMD"

> >    "ld1r\\t{%0.<Vtype>}, %1"
Joey Ye Nov. 14, 2014, 2:16 a.m. UTC | #3 
Can a new case be rewritten then?

- Joey

On Fri, Nov 14, 2014 at 9:32 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> No, we noticed this issue when improving the vld1(q?)_dup intrinsics.  Thanks.
>
>
>> Is there a case or PR to demonstrate the issue? If yes, better to include it as a test
>> case.
>>
>> Thanks,
>> Joey
>>
>> On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix) <felix.yang@huawei.com>
>> wrote:
>> > Hi,
>> >
>> >   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode>
>> pattern is not appropriate.
>> >   The reason is that it's impossible to get a new operand of DImode by
>> vec_duplicating an operand of the same mode.
>> >   So this patch just excludes the DImode and uses VALL instead.
>> >   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?
>> >
>> >
>> > Index: gcc/ChangeLog
>> >
>> =============================================================
>> ======
>> > --- gcc/ChangeLog       (revision 217394)
>> > +++ gcc/ChangeLog       (working copy)
>> > @@ -1,3 +1,9 @@
>> > +2014-11-13  Felix Yang  <felix.yang@huawei.com>
>> > +       Jiji Jiang  <jiangjiji@huawei.com>
>> > +
>> > +       * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r<mode>):
>> Use
>> > +       VALL mode iterator instead of VALLDI.
>> > +
>> >  2014-11-11  Andrew Pinski  <apinski@cavium.com>
>> >
>> >         Bug target/61997
>> > Index: gcc/config/aarch64/aarch64-simd.md
>> >
>> =============================================================
>> ======
>> > --- gcc/config/aarch64/aarch64-simd.md  (revision 217394)
>> > +++ gcc/config/aarch64/aarch64-simd.md  (working copy)
>> > @@ -4936,8 +4936,8 @@
>> >  })
>> >
>> >  (define_insn "*aarch64_simd_ld1r<mode>"
>> > -  [(set (match_operand:VALLDI 0 "register_operand" "=w")
>> > -       (vec_duplicate:VALLDI
>> > +  [(set (match_operand:VALL 0 "register_operand" "=w")
>> > +       (vec_duplicate:VALL
>> >           (match_operand:<VEL> 1 "aarch64_simd_struct_operand"
>> "Utv")))]
>> >    "TARGET_SIMD"
>> >    "ld1r\\t{%0.<Vtype>}, %1"
Yangfei (Felix) Nov. 14, 2014, 2:34 a.m. UTC | #4 
Hmm, I don't think so.

The pattern *aarch64_simd_ld1r<mode> is used for pure matching, and GCC will never generate a pattern as follows: 

  [(set (match_operand:DI 0 "register_operand" "=w")
        (vec_duplicate:DI
          (match_operand:DI 1 "aarch64_simd_struct_operand" "Utv")))]


> 

> Can a new case be rewritten then?

> 

> - Joey

> 

> On Fri, Nov 14, 2014 at 9:32 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:

> > No, we noticed this issue when improving the vld1(q?)_dup intrinsics.  Thanks.

> >

> >

> >> Is there a case or PR to demonstrate the issue? If yes, better to

> >> include it as a test case.

> >>

> >> Thanks,

> >> Joey

> >>

> >> On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix)

> >> <felix.yang@huawei.com>

> >> wrote:

> >> > Hi,

> >> >

> >> >   We find that the VALLDI mode iterator used in

> >> > *aarch64_simd_ld1r<mode>

> >> pattern is not appropriate.

> >> >   The reason is that it's impossible to get a new operand of DImode

> >> > by

> >> vec_duplicating an operand of the same mode.

> >> >   So this patch just excludes the DImode and uses VALL instead.

> >> >   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?

> >> >

> >> >

> >> > Index: gcc/ChangeLog

> >> >

> >>

> =============================================================

> >> ======

> >> > --- gcc/ChangeLog       (revision 217394)

> >> > +++ gcc/ChangeLog       (working copy)

> >> > @@ -1,3 +1,9 @@

> >> > +2014-11-13  Felix Yang  <felix.yang@huawei.com>

> >> > +       Jiji Jiang  <jiangjiji@huawei.com>

> >> > +

> >> > +       * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r<mode>):

> >> Use

> >> > +       VALL mode iterator instead of VALLDI.

> >> > +

> >> >  2014-11-11  Andrew Pinski  <apinski@cavium.com>

> >> >

> >> >         Bug target/61997

> >> > Index: gcc/config/aarch64/aarch64-simd.md

> >> >

> >>

> =============================================================

> >> ======

> >> > --- gcc/config/aarch64/aarch64-simd.md  (revision 217394)

> >> > +++ gcc/config/aarch64/aarch64-simd.md  (working copy)

> >> > @@ -4936,8 +4936,8 @@

> >> >  })

> >> >

> >> >  (define_insn "*aarch64_simd_ld1r<mode>"

> >> > -  [(set (match_operand:VALLDI 0 "register_operand" "=w")

> >> > -       (vec_duplicate:VALLDI

> >> > +  [(set (match_operand:VALL 0 "register_operand" "=w")

> >> > +       (vec_duplicate:VALL

> >> >           (match_operand:<VEL> 1 "aarch64_simd_struct_operand"

> >> "Utv")))]

> >> >    "TARGET_SIMD"

> >> >    "ld1r\\t{%0.<Vtype>}, %1"
Marcus Shawcroft Nov. 14, 2014, 10:46 a.m. UTC | #5 
On 13 November 2014 06:14, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> Hi,
>
>   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode> pattern is not appropriate.
>   The reason is that it's impossible to get a new operand of DImode by vec_duplicating an operand of the same mode.
>   So this patch just excludes the DImode and uses VALL instead.
>   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?

OK, can you back port it to 4.9?
Thanks
/Marcus
Yangfei (Felix) Nov. 15, 2014, 8:51 a.m. UTC | #6 
> On 13 November 2014 06:14, Yangfei (Felix) <felix.yang@huawei.com> wrote:

> > Hi,

> >

> >   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r<mode>

> pattern is not appropriate.

> >   The reason is that it's impossible to get a new operand of DImode by

> vec_duplicating an operand of the same mode.

> >   So this patch just excludes the DImode and uses VALL instead.

> >   Reg-tested for aarch64-linux-gnu with QEMU.  OK for the trunk?

> 

> OK, can you back port it to 4.9?

> Thanks

> /Marcus


Yes, committed: 
- trunk: r217573. 
- 4.9 branch: r217574.
diff mbox

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217394)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2014-11-13  Felix Yang  <felix.yang@huawei.com>
+	Jiji Jiang  <jiangjiji@huawei.com>
+
+	* config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r<mode>): Use
+	VALL mode iterator instead of VALLDI.
+
 2014-11-11  Andrew Pinski  <apinski@cavium.com>
 
 	Bug target/61997
Index: gcc/config/aarch64/aarch64-simd.md
===================================================================
--- gcc/config/aarch64/aarch64-simd.md	(revision 217394)
+++ gcc/config/aarch64/aarch64-simd.md	(working copy)
@@ -4936,8 +4936,8 @@ 
 })
 
 (define_insn "*aarch64_simd_ld1r<mode>"
-  [(set (match_operand:VALLDI 0 "register_operand" "=w")
-	(vec_duplicate:VALLDI
+  [(set (match_operand:VALL 0 "register_operand" "=w")
+	(vec_duplicate:VALL
 	  (match_operand:<VEL> 1 "aarch64_simd_struct_operand" "Utv")))]
   "TARGET_SIMD"
   "ld1r\\t{%0.<Vtype>}, %1"