[Bug tree-optimization/53773] New: Vectorizer generates non-canonical multiplies

[Bug tree-optimization/53773] New: Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

             Bug #: 53773
           Summary: Vectorizer generates non-canonical multiplies
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: wschmidt@gcc.gnu.org
                CC: bergner@vnet.ibm.com, rguenther@suse.de


There are two tests in the regression testsuite for which the vectorizer
generates GIMPLE multiplies that are not canonical.  Specifically, they
generate multiplies where rhs1 is a constant and rhs2 is an SSA name.  The two
tests are:

(1) gcc.dg/vect/pr50451.c
(2) gfortran.dg/pr17143.f90

For (1), the generated GIMPLE includes:

  integral_7 = 10 * integral_17;
  ...
  decimal_8 = 10 * decimal_18;

For (2):

  i.3_3 = 2 * prephitmp.9_23;

Probably best just to run these tests from the test suite to reproduce, but
here are the commands I used.

Example invocation for (1):

/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/gcc/xgcc
-B/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/gcc/
/home/wschmidt/gcc/gcc-mainline-slsrpt1a/gcc/testsuite/gcc.dg/vect/pr50451.c
-fno-diagnostics-show-caret -flto -maltivec -mvsx -mno-allow-movmisalign
-ftree-vectorize -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details
-S -m64 -o pr50451.s

Example invocation for (2):

/home/wschmidt/gcc/install/gcc-mainline-slsrpt1a-debug/bin/gfortran
-B/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/gcc/testsuite/gfortran3/../../
-B/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/powerpc64-linux/32/libgfortran/
/home/wschmidt/gcc/gcc-mainline-slsrpt1a/gcc/testsuite/gfortran.dg/pr17143.f90
-fno-diagnostics-show-caret -O3 -fomit-frame-pointer -std=legacy
-B/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/powerpc64-linux/32/libgfortran/.libs
-L/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/powerpc64-linux/32/libgfortran/.libs
-L/home/wschmidt/gcc/build/gcc-mainline-slsrpt1a-debug/powerpc64-linux/32/libgfortran/.libs
-lm -m32 -o ./pr17143.exe

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-06-26 08:31:36 UTC ---
Seems this is vect_is_simple_reduction_1's:
      if (check_reduction)
        {
          /* Swap operands (just for simplicity - so that the rest of the code
             can assume that the reduction variable is always the last (second)
             argument).  */
          if (vect_print_dump_info (REPORT_DETAILS))
            report_vect_op (def_stmt,
                            "detected reduction: need to swap operands: ");

          swap_tree_operands (def_stmt, gimple_assign_rhs1_ptr (def_stmt),
                              gimple_assign_rhs2_ptr (def_stmt));
        }

at least on the first testcase.  Now, the question is how hard would it be to
adjust the vectorizer so that it would work even without doing this swapping,
or
alternatively if we shouldn't canonicalize the operand order afterwards.

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[rguenther at suse dot de]

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

--- Comment #2 from rguenther at suse dot de <rguenther at suse dot de> 2012-06-26 08:35:59 UTC ---
On Tue, 26 Jun 2012, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53773
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-06-26 08:31:36 UTC ---
> Seems this is vect_is_simple_reduction_1's:
>       if (check_reduction)
>         {
>           /* Swap operands (just for simplicity - so that the rest of the code
>              can assume that the reduction variable is always the last (second)
>              argument).  */
>           if (vect_print_dump_info (REPORT_DETAILS))
>             report_vect_op (def_stmt,
>                             "detected reduction: need to swap operands: ");
> 
>           swap_tree_operands (def_stmt, gimple_assign_rhs1_ptr (def_stmt),
>                               gimple_assign_rhs2_ptr (def_stmt));
>         }
> 
> at least on the first testcase.  Now, the question is how hard would it be to
> adjust the vectorizer so that it would work even without doing this swapping,
> or
> alternatively if we shouldn't canonicalize the operand order afterwards.

Or change it to "just for simplicity - ... that the reduction variable is 
always the first argument.".  Eventually that will wreck with reduction
of MINUS_EXPR though, not sure.

Richard.

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-06-26
     Ever Confirmed|0                           |1

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-26 08:52:21 UTC ---
Confirmed.

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

--- Comment #4 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-07-27 16:26:47 UTC ---
Operand swapping is also done in vect_is_slp_reduction.

Places that rely on the reduction operand being the last operand:
 * vect_model_reduction_cost
 * vectorizable_reduction
 * vect_recog_dot_prod_pattern
 * vect_recog_widen_sum_pattern

Some commentary in vectorizable_reduction indicates that the last-operand rule
is not used for nested cycles, so there are a few places where a reduc_index
parameter is passed around instead.

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot       |wschmidt at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.8.0

--- Comment #5 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-07-29 16:54:45 UTC ---
I'll take this one.

I think the assumption of operand placement is too embedded to tease out
easily, so I'm going to approach this by re-canonicalizing PLUS_EXPR,
POINTER_PLUS_EXPR, and MULT_EXPR when operand swapping has occurred.  Are there
other tree codes that could be broken?

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[rguenther at suse dot de]

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

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> 2012-07-30 08:47:35 UTC ---
On Sun, 29 Jul 2012, wschmidt at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53773
> 
> William J. Schmidt <wschmidt at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>          AssignedTo|unassigned at gcc dot       |wschmidt at gcc dot gnu.org
>                    |gnu.org                     |
>    Target Milestone|---                         |4.8.0
> 
> --- Comment #5 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-07-29 16:54:45 UTC ---
> I'll take this one.
> 
> I think the assumption of operand placement is too embedded to tease out
> easily, so I'm going to approach this by re-canonicalizing PLUS_EXPR,
> POINTER_PLUS_EXPR, and MULT_EXPR when operand swapping has occurred.  Are there
> other tree codes that could be broken?

I don't think so

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

--- Comment #7 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-07-31 12:25:10 UTC ---
Author: wschmidt
Date: Tue Jul 31 12:25:04 2012
New Revision: 190007

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190007
Log:
gcc:

2012-07-31  Bill Schmidt  <wschmidt@linux.ibm.com>

    PR tree-optimization/53773
    * tree-vectorizer.h (struct _loop_vec_info): Add operands_swapped.
    (LOOP_VINFO_OPERANDS_SWAPPED): New macro.
    * tree-vect-loop.c (new_loop_vec_info): Initialize
    LOOP_VINFO_OPERANDS_SWAPPED field.
    (destroy_loop_vec_info): Restore canonical form.
    (vect_is_slp_reduction): Set LOOP_VINFO_OPERANDS_SWAPPED field.
    (vect_is_simple_reduction_1): Likewise.

gcc/testsuite:

2012-07-31  Bill Schmidt  <wschmidt@linux.ibm.com>

    PR tree-optimization/53773
    * testsuite/gcc.dg/vect/pr53773.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/vect/pr53773.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vect-loop.c
    trunk/gcc/tree-vectorizer.h

[Bug tree-optimization/53773] Vectorizer generates non-canonical multiplies

[wschmidt at gcc dot gnu.org]

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

William J. Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #8 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-07-31 12:26:37 UTC ---
Fixed.