[Bug tree-optimization/57233] New: Vector lowering of LROTATE_EXPR pessimizes code

[Bug tree-optimization/57233] New: Vector lowering of LROTATE_EXPR pessimizes code

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57233

            Bug ID: 57233
           Summary: Vector lowering of LROTATE_EXPR pessimizes code
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: glisse at gcc dot gnu.org

Hello,

the vector lowering pass, when it sees a rotate on a vector that is not a
supported operation, lowers it to scalar rotates. However, from a quick look at
the RTL expanders (untested), they know how to handle a vector rotate as long
as shifts and ior are supported, and that would yield better code than the
scalar ops. So I think the vector lowering pass should not just check if rotate
is supported, but also if shift and ior are, before splitting the operation.

typedef unsigned vec __attribute__((vector_size(4*sizeof(int))));
vec f(vec a){
  return (a<<2)|(a>>30);
}

without rotate:
    vpsrld    $30, %xmm0, %xmm1
    vpslld    $2, %xmm0, %xmm0
    vpor    %xmm0, %xmm1, %xmm0

with a patch that recognizes rotate for vectors:
    vpextrd    $2, %xmm0, %edx
    vmovd    %xmm0, %eax
    rorx    $30, %eax, %eax
    movl    %eax, -16(%rsp)
    rorx    $30, %edx, %ecx
    vpextrd    $1, %xmm0, %eax
    movl    %ecx, -12(%rsp)
    vmovd    -16(%rsp), %xmm3
    vpextrd    $3, %xmm0, %edx
    vmovd    -12(%rsp), %xmm2
    rorx    $30, %eax, %eax
    rorx    $30, %edx, %edx
    vpinsrd    $1, %eax, %xmm3, %xmm1
    vpinsrd    $1, %edx, %xmm2, %xmm0
    vpunpcklqdq    %xmm0, %xmm1, %xmm0

(I am not sure all those ext/ins are optimal, I would have expected one mov
from xmm0 to memory, then the scalar rotates are done and write to memory
again, and one final mov back to the FPU, but my intuition may be wrong)

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57233

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-10
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
The vector lowering pass should lower to (X << Y) | (X >> ((-Y) & 31))
instead if vector shifts are available.  Or just rely on the generic
RTL expansion support (via a new predicate, can_expand_vector_rotate_p or
similar).

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |jakub at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 33007
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33007&action=edit
gcc410-pr57233.patch

Untested fix.

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Fri Jun 27 07:03:50 2014
New Revision: 212063

URL: https://gcc.gnu.org/viewcvs?rev=212063&root=gcc&view=rev
Log:
    PR tree-optimization/57233
    PR tree-optimization/61299
    * tree-vect-generic.c (get_compute_type, count_type_subparts): New
    functions.
    (expand_vector_operations_1): Use them.  If {L,R}ROTATE_EXPR
    would be lowered to scalar shifts, check if corresponding
    shifts and vector BIT_IOR_EXPR are supported and don't lower
    or lower just to narrower vector type in that case.
    * expmed.c (expand_shift_1): Fix up handling of vector
    shifts and rotates.

    * gcc.dg/pr57233.c: New test.
    * gcc.target/i386/pr57233.c: New test.
    * gcc.target/i386/sse2-pr57233.c: New test.
    * gcc.target/i386/avx-pr57233.c: New test.
    * gcc.target/i386/avx2-pr57233.c: New test.
    * gcc.target/i386/avx512f-pr57233.c: New test.
    * gcc.target/i386/xop-pr57233.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/avx-pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/avx2-pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/avx512f-pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/sse2-pr57233.c
    trunk/gcc/testsuite/gcc.target/i386/xop-pr57233.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expmed.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vect-generic.c

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Jun 30 16:56:01 2014
New Revision: 212158

URL: https://gcc.gnu.org/viewcvs?rev=212158&root=gcc&view=rev
Log:
2014-06-30  Jakub Jelinek  <jakub@redhat.com>

    Backported from mainline
    2014-06-27  Jakub Jelinek  <jakub@redhat.com>

    PR tree-optimization/57233
    PR tree-optimization/61299
    * tree-vect-generic.c (get_compute_type, count_type_subparts): New
    functions.
    (expand_vector_operations_1): Use them.  If {L,R}ROTATE_EXPR
    would be lowered to scalar shifts, check if corresponding
    shifts and vector BIT_IOR_EXPR are supported and don't lower
    or lower just to narrower vector type in that case.
    * expmed.c (expand_shift_1): Fix up handling of vector
    shifts and rotates.

    * gcc.dg/pr57233.c: New test.
    * gcc.target/i386/pr57233.c: New test.
    * gcc.target/i386/sse2-pr57233.c: New test.
    * gcc.target/i386/avx-pr57233.c: New test.
    * gcc.target/i386/avx2-pr57233.c: New test.
    * gcc.target/i386/avx512f-pr57233.c: New test.
    * gcc.target/i386/xop-pr57233.c: New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.dg/pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/avx-pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/avx2-pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/avx512f-pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/sse2-pr57233.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/i386/xop-pr57233.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/expmed.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/tree-vect-generic.c

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.9.1

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed for 4.9.1+.

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[christophe.lyon at st dot com]

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

christophe.lyon at st dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |christophe.lyon at st dot com

--- Comment #6 from christophe.lyon at st dot com ---
The new gcc.dg/pr57233.c test fails at execution on target aarch64_be-none-elf
(using the Foundation model as execution engine)

Executing on host:
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc3/gcc/xgcc
-B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc3/gcc/
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pr57233.c gcc_tg.o 
-fno-diagnostics-show-caret -fdiagnostics-color=never   -O2 -DSTACK_SIZE=16384
-specs=rdimon.specs    
-Wl,/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64_be-none-elf/gcc3/gcc/testsuite/../../../newlib/aarch64_be-none-elf/libgloss/aarch64/cpu-init/rdimon-aem-el3.o
-Wl,-Ttext-segment=0x80000000  -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main
-Wl,-wrap,abort -lm   -o ./pr57233.exe    (timeout = 800)
PASS: gcc.dg/pr57233.c (test for excess errors)

*** EXIT code 4242
emu: host signal 6
FAIL: gcc.dg/pr57233.c execution test

[Bug tree-optimization/57233] Vector lowering of LROTATE_EXPR pessimizes code

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

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

--- Comment #7 from ktkachov at gcc dot gnu.org ---
Created attachment 33070
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33070&action=edit
Testcase failing on aarch64_be-none-elf with 4.9.1

Noticed this failure as well. Attaching reduced testcase with a printf at the
abort.
On 4.9 this aborts with:
p[4]= 45300095,  (q[4] << ss) | (q[4] >> (32 - ss))= 342c17f9

Doesn't happen on trunk or aarch64 little-endian though.