[Bug target/63173] New: performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[Bug target/63173] New: performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[chenshanyaoboy at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

            Bug ID: 63173
           Summary: performance problem with simd intrinsics vld2_dup_* on
                    aarch64-none-elf
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chenshanyaoboy at gmail dot com

Hi,
I found there is a performance problem with some simd intrinsics
(vld2_dup_*) on aarch64-none-elf. Now the vld2_dup_* are defined as
follows:

#define __LD2R_FUNC(rettype, structtype, ptrtype, \
    regsuffix, funcsuffix, Q) \
  __extension__ static __inline rettype \
  __attribute__ ((__always_inline__))  \
  vld2 ## Q ## _dup_ ## funcsuffix (const ptrtype *ptr) \
  { \
    rettype result; \
    __asm__ ("ld2r {v16." #regsuffix ", v17." #regsuffix "}, %1\n\t" \
     "st1 {v16." #regsuffix ", v17." #regsuffix "}, %0\n\t" \
     : "=Q"(result) \
     : "Q"(*(const structtype *)ptr) \
     : "memory", "v16", "v17"); \
    return result; \
  }

It loads from memory to registers, and then store the value of
registers to memory as a result. Such code is terribly low in
performance because of redundant memory visit and limited registers
allocation.

cat test1.c

#include <arm_neon.h>
int 16x4x2_t foo(int16_t __restrict pDataA,
                 int16_t __restrict pDataB,)
{
    int 16x4x2_t DataA, DataB, DataC;

    DataA = vld2_dup_s16(pDataA);
    DataB = vld2_dup_s16(pDataB);

    DataC.val[0] = vqadd_s16( DataA.val[0], DataB.val[0] ); 
    DataC.val[1] = vqadd_s16( DataA.val[1], DataB.val[1] ); 

    return DataC;
}

aarch64-none-elf-gcc -S -O2 test1.c
cat test1.s

foo:
    sub  sp, sp, #16
    //start of user assembly
    ld2r {v16.4h, v17.4h}, [x0]
    st1  {v16.4h, v17.4h}, [sp]
    //end of user assembly
    ldr  d0, [sp]
    ldr  d1, [sp,8]
    //start of user assembly
    ld2r {v16.4h, v17.4h}, [x1]
    st1  {v16.4h, v17.4h}, [sp]
    //end of user assembly
    ldr  d2, [sp]
    sqadd v0.4h, v0.4h, v2.4h
    ldr  d2, [sp,8]
    add sp, sp, 16
    sqadd v1.4h, v1.4h, v2,4h
    ret


Some intinsics like vld2_* were similar to vld2_dup_*, but now they
are realized by builtin functions.

__extension__ static __inline int16x4x2_t __attribute__ ((__always_inline__))
vld2_s16 (const int16_t * __a)
{
  int16x4x2_t ret;
  __builtin_aarch64_simd_oi __o;
  __o = __builtin_aarch64_ld2v4hi ((const __builtin_aarch64_simd_hi *) __a);
  ret.val[0] = (int16x4_t) __builtin_aarch64_get_dregoiv4hi (__o, 0);
  ret.val[1] = (int16x4_t) __builtin_aarch64_get_dregoiv4hi (__o, 1);
  return ret;
}

test2.c is similar to test1.c ,only vld2_dup_s16 is instead of vld2_s16
cat test2.c

#include <arm_neon.h>
int 16x4x2_t foo (int16_t __restrict pDataA,
                 int16_t __restrict pDataB,)
{
    int 16x4x2_t DataA, DataB, DataC;

    DataA = vld2_s16(pDataA);
    DataB = vld2_s16(pDataB);

    DataC.val[0] = vqadd_s16( DataA.val[0], DataB.val[0] ); 
    DataC.val[1] = vqadd_s16( DataA.val[1], DataB.val[1] ); 

    return DataC;
}
aarch64-none-elf-gcc -S -O2 test2.c
cat test2.s

foo:
    ld2 {v2.4h-v3.4h}, [x0]
    ld2 {v4.4h-v5.4h}, [x1]
    sqadd v1.4h, v5.4h, v3.4h
    sqadd v0.4h, v4.4h, v2.4h
    ret

Could vld2_dup_* also be written with builtin as vld2_* ?

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-09-04
                 CC|                            |ktkachov at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from ktkachov at gcc dot gnu.org ---
Confirmed.

Feel free to propose a patch for them on gcc-patches along the lines you
described in:
https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[venkataramanan.kumar at amd dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

--- Comment #2 from Venkataramanan <venkataramanan.kumar at amd dot com> ---
Changed the test case to work with latest GCC trunk 

#include <arm_neon.h>
int16x4x2_t foo(int16_t * __restrict pDataA,
                                 int16_t *  __restrict pDataB)
{
        int16x4x2_t DataA, DataB, DataC;

        DataA = vld2_dup_s16(pDataA);
        DataB = vld2_dup_s16(pDataB);

        DataC.val[0] = vqadd_s16( DataA.val[0], DataB.val[0] );
        DataC.val[1] = vqadd_s16( DataA.val[1], DataB.val[1] );

        return DataC;
}

Still seeing loads and stores via memory.

 foo:
        sub     sp, sp, #16
        // Start of user assembly
// 11788
"/home/venkataramanan-kumar/work/pr62308/builds/destdir/x86_64-unknown-linux-gnu/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h"
1
        ld2r {v16.4h, v17.4h}, [x0]
        st1 {v16.4h, v17.4h}, [sp]

// 0 "" 2
        // End of user assembly
        ldr     d0, [sp]
        ldr     d1, [sp, 8]
        // Start of user assembly
// 11788
"/home/venkataramanan-kumar/work/pr62308/builds/destdir/x86_64-unknown-linux-gnu/lib/gcc/aarch64-none-elf/5.0.0/include/arm_neon.h"
1
        ld2r {v16.4h, v17.4h}, [x1]
        st1 {v16.4h, v17.4h}, [sp]

// 0 "" 2
        // End of user assembly
        ldr     d3, [sp]
        ldr     d2, [sp, 8]
        add     sp, sp, 16
        sqadd   v0.4h, v0.4h, v3.4h
        sqadd   v1.4h, v1.4h, v2.4h
        ret
        .size   foo, .-foo
        .ident  "GCC: (Linaro GCC 2014.10) 5.0.0 20140930 (experimental)"

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[fei.yang0953 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

Fei Yang <fei.yang0953 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fei.yang0953 at gmail dot com

--- Comment #3 from Fei Yang <fei.yang0953 at gmail dot com> ---
(In reply to ktkachov from comment #1)
> Confirmed.

Feel free to propose a patch for them on gcc-patches along the
> lines you described in:
https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html

Hi,
  To let you know, we are currently working on this issue.
  We are implementing these with builtins.
  Hopefully, the patch will be posted this week. Thank you.

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[venkataramanan.kumar at amd dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

--- Comment #4 from Venkataramanan <venkataramanan.kumar at amd dot com> ---
(In reply to Fei Yang from comment #3)
> (In reply to ktkachov from comment #1)
> > Confirmed.
> 
> Feel free to propose a patch for them on gcc-patches along the
> > lines you described in:
> https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> 
> Hi,
>   To let you know, we are currently working on this issue.
>   We are implementing these with builtins.
>   Hopefully, the patch will be posted this week. Thank you.


Hi Fei Yang,

Ok no issues. I will let you do this. But please asign (In reply to Fei Yang
from comment #3)
> (In reply to ktkachov from comment #1)
> > Confirmed.
> 
> Feel free to propose a patch for them on gcc-patches along the
> > lines you described in:
> https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> 
> Hi,
>   To let you know, we are currently working on this issue.
>   We are implementing these with builtins.
>   Hopefully, the patch will be posted this week. Thank you.

Ok. Next time please assign the Bugzilla item to your name, so that we wont be
duplicating the work.

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ramana at gcc dot gnu.org

--- Comment #5 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Venkataramanan from comment #4)
> (In reply to Fei Yang from comment #3)
> > (In reply to ktkachov from comment #1)
> > > Confirmed.
> > 
> > Feel free to propose a patch for them on gcc-patches along the
> > > lines you described in:
> > https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> > 
> > Hi,
> >   To let you know, we are currently working on this issue.
> >   We are implementing these with builtins.
> >   Hopefully, the patch will be posted this week. Thank you.
> 
> 
> Hi Fei Yang,
> 
> Ok no issues. I will let you do this. But please asign (In reply to Fei Yang
> from comment #3)
> > (In reply to ktkachov from comment #1)
> > > Confirmed.
> > 
> > Feel free to propose a patch for them on gcc-patches along the
> > > lines you described in:
> > https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> > 
> > Hi,
> >   To let you know, we are currently working on this issue.
> >   We are implementing these with builtins.
> >   Hopefully, the patch will be posted this week. Thank you.
> 
> Ok. Next time please assign the Bugzilla item to your name, so that we wont
> be duplicating the work.


Linaro / Charles Bayliss was already working on this - he had patches out in
September for this.

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[clyon at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

clyon at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cbaylis at gcc dot gnu.org

--- Comment #6 from clyon at gcc dot gnu.org ---
(In reply to Ramana Radhakrishnan from comment #5)
> (In reply to Venkataramanan from comment #4)
> > (In reply to Fei Yang from comment #3)
> > > (In reply to ktkachov from comment #1)
> > > > Confirmed.
> > > 
> > > Feel free to propose a patch for them on gcc-patches along the
> > > > lines you described in:
> > > https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> > > 
> > > Hi,
> > >   To let you know, we are currently working on this issue.
> > >   We are implementing these with builtins.
> > >   Hopefully, the patch will be posted this week. Thank you.
> > 
> > 
> > Hi Fei Yang,
> > 
> > Ok no issues. I will let you do this. But please asign (In reply to Fei Yang
> > from comment #3)
> > > (In reply to ktkachov from comment #1)
> > > > Confirmed.
> > > 
> > > Feel free to propose a patch for them on gcc-patches along the
> > > > lines you described in:
> > > https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> > > 
> > > Hi,
> > >   To let you know, we are currently working on this issue.
> > >   We are implementing these with builtins.
> > >   Hopefully, the patch will be posted this week. Thank you.
> > 
> > Ok. Next time please assign the Bugzilla item to your name, so that we wont
> > be duplicating the work.
> 
> 
> Linaro / Charles Bayliss was already working on this - he had patches out in
> September for this.

It seems that Charles' patches cover vldX_lane, but not vldX_dup.

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[fei.yang0953 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

--- Comment #7 from Fei Yang <fei.yang0953 at gmail dot com> ---
(In reply to clyon from comment #6)
> (In reply to Ramana Radhakrishnan from comment #5)
> (In reply to
> Venkataramanan from comment #4)
> > (In reply to Fei Yang from comment #3)
>
> > > (In reply to ktkachov from comment #1)
> > > > Confirmed.
> > > 
> > >
> Feel free to propose a patch for them on gcc-patches along the
> > > > lines
> you described in:
> > > https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> >
> > 
> > > Hi,
> > >   To let you know, we are currently working on this
> issue.
> > >   We are implementing these with builtins.
> > >   Hopefully,
> the patch will be posted this week. Thank you.
> > 
> > 
> > Hi Fei Yang,
>
> > 
> > Ok no issues. I will let you do this. But please asign (In reply to
> Fei Yang
> > from comment #3)
> > > (In reply to ktkachov from comment #1)
>
> > > > Confirmed.
> > > 
> > > Feel free to propose a patch for them on
> gcc-patches along the
> > > > lines you described in:
> > >
> https://gcc.gnu.org/ml/gcc/2014-09/msg00046.html
> > > 
> > > Hi,
> > >   To
> let you know, we are currently working on this issue.
> > >   We are
> implementing these with builtins.
> > >   Hopefully, the patch will be
> posted this week. Thank you.
> > 
> > Ok. Next time please assign the
> Bugzilla item to your name, so that we wont
> > be duplicating the work.
> 
> > 
> Linaro / Charles Bayliss was already working on this - he had patches
> out in
> September for this.

It seems that Charles' patches cover
> vldX_lane, but not vldX_dup.

Hi Ramana,
  Do you mean this link:
    https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00678.html

[Bug target/63173] performance problem with simd intrinsics vld2_dup_* on aarch64-none-elf

[yroux at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63173

--- Comment #10 from Yvan Roux <yroux at gcc dot gnu.org> ---
Author: yroux
Date: Thu Dec  4 17:46:32 2014
New Revision: 218385

URL: https://gcc.gnu.org/viewcvs?rev=218385&root=gcc&view=rev
Log:
2014-12-04  Yvan Roux  <yvan.roux@linaro.org>

    Backport from trunk r216630.
    2014-10-24  Felix Yang  <felix.yang@huawei.com>
    Jiji Jiang  <jiangjiji@huawei.com>

    PR target/63173
    * config/aarch64/arm_neon.h (__LD2R_FUNC): Remove macro.
    (__LD3R_FUNC): Ditto.
    (__LD4R_FUNC): Ditto.
    (vld2_dup_s8, vld2_dup_s16, vld2_dup_s32, vld2_dup_f32, vld2_dup_f64,
     vld2_dup_u8, vld2_dup_u16, vld2_dup_u32, vld2_dup_p8, vld2_dup_p16
     vld2_dup_s64, vld2_dup_u64, vld2q_dup_s8, vld2q_dup_p8, 
     vld2q_dup_s16, vld2q_dup_p16, vld2q_dup_s32, vld2q_dup_s64, 
     vld2q_dup_u8, vld2q_dup_u16, vld2q_dup_u32, vld2q_dup_u64 
     vld2q_dup_f32, vld2q_dup_f64): Rewrite using builtin functions.
    (vld3_dup_s64, vld3_dup_u64, vld3_dup_f64, vld3_dup_s8 
     vld3_dup_p8, vld3_dup_s16, vld3_dup_p16, vld3_dup_s32 
     vld3_dup_u8, vld3_dup_u16, vld3_dup_u32, vld3_dup_f32
     vld3q_dup_s8, vld3q_dup_p8, vld3q_dup_s16, vld3q_dup_p16 
     vld3q_dup_s32, vld3q_dup_s64, vld3q_dup_u8, vld3q_dup_u16 
     vld3q_dup_u32, vld3q_dup_u64, vld3q_dup_f32, vld3q_dup_f64): Likewise.
    (vld4_dup_s64, vld4_dup_u64, vld4_dup_f64, vld4_dup_s8 
     vld4_dup_p8, vld4_dup_s16, vld4_dup_p16, vld4_dup_s32 
     vld4_dup_u8, vld4_dup_u16, vld4_dup_u32, vld4_dup_f32 
     vld4q_dup_s8, vld4q_dup_p8, vld4q_dup_s16, vld4q_dup_p16 
     vld4q_dup_s32, vld4q_dup_s64, vld4q_dup_u8, vld4q_dup_u16 
     vld4q_dup_u32, vld4q_dup_u64, vld4q_dup_f32, vld4q_dup_f64): Likewise.
    * config/aarch64/aarch64.md (define_c_enum "unspec"): Add
    UNSPEC_LD2_DUP, UNSPEC_LD3_DUP, UNSPEC_LD4_DUP.
    * config/aarch64/aarch64-simd-builtins.def (ld2r, ld3r, ld4r): New
    builtins.
    * config/aarch64/aarch64-simd.md (aarch64_simd_ld2r<mode>): New pattern.
    (aarch64_simd_ld3r<mode>): Likewise.
    (aarch64_simd_ld4r<mode>): Likewise.
    (aarch64_ld2r<mode>): New expand.
    (aarch64_ld3r<mode>): Likewise.
    (aarch64_ld4r<mode>): Likewise.


Modified:
    branches/linaro/gcc-4_9-branch/gcc/ChangeLog.linaro
    branches/linaro/gcc-4_9-branch/gcc/config/aarch64/aarch64-simd-builtins.def
    branches/linaro/gcc-4_9-branch/gcc/config/aarch64/aarch64-simd.md
    branches/linaro/gcc-4_9-branch/gcc/config/aarch64/aarch64.md
    branches/linaro/gcc-4_9-branch/gcc/config/aarch64/arm_neon.h