On Tue, Sep 22, 2020 at 1:28 PM Martin Liška <mli...@suse.cz> wrote:
>
> @Richi: May I please ping this?

I have commented in the PR and am testing a patch - other PRs may prevail.

Richard.

> On 9/1/20 4:27 PM, Martin Liška wrote:
> > On 8/31/20 10:01 AM, Richard Biener wrote:
> >> On Fri, Aug 28, 2020 at 4:18 PM Martin Liška <mli...@suse.cz> wrote:
> >>>
> >>> Hey.
> >>>
> >>> The patch is about VEC_COND_EXP comparison of a SSA_NAME with a constant
> >>> that is artifact of -fno-tree-ccp.
> >>>
> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>>
> >>> Ready to be installed?
> >>
> >> Err, no - we shouldn't do this at RTL expansion time.  And piecewise
> >> expansion is gross - in this the case is a constant boolean vector IIRC
> >> (not sure what mode?)
> >
> > Yes, it's:
> >
> > (gdb) p op0.typed.type
> > $11 = <vector_type 0x7ffff743a1f8>
> > (gdb) p mode
> > $13 = E_DImode
> >
> > and TYPE_MODE (TREE_TYPE (op0)) is E_BLKmode
> >
> >> and ISEL only tries matching up with
> >> a comparison operand.  But ISEL should also handle matching up with
> >> a constant "boolean vector" ("boolean vector"s are artifacts of 
> >> VEC_COND_EXPRs,
> >> but some targets allow bitwise arithmetic on them...).  So the compensation
> >> needs to happen in ISEL, recognizing
> >>
> >>   _1 = {0}
> >>   _2 = _1 ? ...;
> >>
> >> as
> >>
> >>   _2 = .VCOND (0 == 0, ...)
> >
> > Do we really want to do a scalar arguments of .VCOND? Note that in PR96453
> > (a similar bug), we end up with:
> >
> >    _5 = { -1, -1 };
> >    _6 = VEC_COND_EXPR <_5, { -1, -1 }, { 0, 0 }>;
> >
> > Thanks for hints,
> > Martin
> >
> >>
> >> or so.
> >>
> >> Richard.
> >>
> >>> Thanks,
> >>> Martin
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>>          PR tree-optimization/96466
> >>>          * gimple-fold.c (expand_cmp_piecewise): New.
> >>>          * gimple-fold.h (nunits_for_known_piecewise_op): New.
> >>>          (expand_cmp_piecewise): Moved from ...
> >>>          * tree-vect-generic.c (expand_vector_comparison): ... here.
> >>>          (nunits_for_known_piecewise_op): Moved to gimple-fold.h.
> >>>          * gimple-isel.cc (gimple_expand_vec_cond_expr): Use
> >>>          expand_cmp_piecewise fallback for constants.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>>          PR tree-optimization/96466
> >>>          * gcc.dg/vect/pr96466.c: New test.
> >>> ---
> >>>    gcc/gimple-fold.c                   | 28 ++++++++++++++++++++
> >>>    gcc/gimple-fold.h                   | 14 ++++++++++
> >>>    gcc/gimple-isel.cc                  | 10 ++++---
> >>>    gcc/testsuite/gcc.dg/vect/pr96466.c | 18 +++++++++++++
> >>>    gcc/tree-vect-generic.c             | 41 ++---------------------------
> >>>    5 files changed, 69 insertions(+), 42 deletions(-)
> >>>    create mode 100644 gcc/testsuite/gcc.dg/vect/pr96466.c
> >>>
> >>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> >>> index dcc1b56a273..86d5d0ed7d8 100644
> >>> --- a/gcc/gimple-fold.c
> >>> +++ b/gcc/gimple-fold.c
> >>> @@ -8056,3 +8056,31 @@ gimple_stmt_integer_valued_real_p (gimple *stmt, 
> >>> int depth)
> >>>          return false;
> >>>        }
> >>>    }
> >>> +
> >>> +tree
> >>> +expand_cmp_piecewise (gimple_stmt_iterator *gsi, tree type, tree op0, 
> >>> tree op1)
> >>> +{
> >>> +  tree inner_type = TREE_TYPE (TREE_TYPE (op0));
> >>> +  tree part_width = vector_element_bits_tree (TREE_TYPE (op0));
> >>> +  tree index = bitsize_int (0);
> >>> +  int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0));
> >>> +  int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type));
> >>> +  tree ret_type = build_nonstandard_integer_type (prec, 1);
> >>> +  tree ret_inner_type = boolean_type_node;
> >>> +  int i;
> >>> +  tree t = build_zero_cst (ret_type);
> >>> +
> >>> +  if (TYPE_PRECISION (ret_inner_type) != 1)
> >>> +    ret_inner_type = build_nonstandard_integer_type (1, 1);
> >>> +  for (i = 0; i < nunits;
> >>> +       i++, index = int_const_binop (PLUS_EXPR, index, part_width))
> >>> +    {
> >>> +      tree a = tree_vec_extract (gsi, inner_type, op0, part_width, 
> >>> index);
> >>> +      tree b = tree_vec_extract (gsi, inner_type, op1, part_width, 
> >>> index);
> >>> +      tree result = gimplify_build2 (gsi, NE_EXPR, ret_inner_type, a, b);
> >>> +      t = gimplify_build3 (gsi, BIT_INSERT_EXPR, ret_type, t, result,
> >>> +                          bitsize_int (i));
> >>> +    }
> >>> +
> >>> +  return gimplify_build1 (gsi, VIEW_CONVERT_EXPR, type, t);
> >>> +}
> >>> diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
> >>> index 0ed1d1ffe83..7e843b34f53 100644
> >>> --- a/gcc/gimple-fold.h
> >>> +++ b/gcc/gimple-fold.h
> >>> @@ -147,6 +147,20 @@ gimple_build_vector (gimple_seq *seq, 
> >>> tree_vector_builder *builder)
> >>>    extern bool gimple_stmt_nonnegative_warnv_p (gimple *, bool *, int = 
> >>> 0);
> >>>    extern bool gimple_stmt_integer_valued_real_p (gimple *, int = 0);
> >>>
> >>> +/* Return the number of elements in a vector type TYPE that we have
> >>> +   already decided needs to be expanded piecewise.  We don't support
> >>> +   this kind of expansion for variable-length vectors, since we should
> >>> +   always check for target support before introducing uses of those.  */
> >>> +
> >>> +static inline unsigned int
> >>> +nunits_for_known_piecewise_op (const_tree type)
> >>> +{
> >>> +  return TYPE_VECTOR_SUBPARTS (type).to_constant ();
> >>> +}
> >>> +
> >>> +extern tree expand_cmp_piecewise (gimple_stmt_iterator *gsi, tree lhs,
> >>> +                                 tree op0, tree op1);
> >>> +
> >>>    /* In gimple-match.c.  */
> >>>    extern tree gimple_simplify (enum tree_code, tree, tree,
> >>>                               gimple_seq *, tree (*)(tree));
> >>> diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> >>> index b330cf4c20e..32e3bc31f7f 100644
> >>> --- a/gcc/gimple-isel.cc
> >>> +++ b/gcc/gimple-isel.cc
> >>> @@ -33,8 +33,8 @@ along with GCC; see the file COPYING3.  If not see
> >>>    #include "gimplify-me.h"
> >>>    #include "gimplify.h"
> >>>    #include "tree-cfg.h"
> >>> -#include "bitmap.h"
> >>>    #include "tree-ssa-dce.h"
> >>> +#include "gimple-fold.h"
> >>>
> >>>    /* Expand all VEC_COND_EXPR gimple assignments into calls to internal
> >>>       function based on type of selected expansion.  */
> >>> @@ -119,8 +119,12 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator 
> >>> *gsi,
> >>>          /* Fake op0 < 0.  */
> >>>          else
> >>>          {
> >>> -         gcc_assert (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (op0)))
> >>> -                     == MODE_VECTOR_INT);
> >>> +         if (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (op0))) != 
> >>> MODE_VECTOR_INT)
> >>> +           {
> >>> +             tree t = expand_cmp_piecewise (gsi, TREE_TYPE (lhs), op0, 
> >>> op1);
> >>> +             return gimple_build_assign (lhs, NOP_EXPR, t);
> >>> +           }
> >>> +
> >>>            op0a = op0;
> >>>            op0b = build_zero_cst (TREE_TYPE (op0));
> >>>            tcode = LT_EXPR;
> >>> diff --git a/gcc/testsuite/gcc.dg/vect/pr96466.c 
> >>> b/gcc/testsuite/gcc.dg/vect/pr96466.c
> >>> new file mode 100644
> >>> index 00000000000..8cca5e12ff2
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.dg/vect/pr96466.c
> >>> @@ -0,0 +1,18 @@
> >>> +/* PR tree-optimization/96466 */
> >>> +/* { dg-do compile } */
> >>> +/* { dg-options "-Og -finline-functions-called-once -fno-tree-ccp" } */
> >>> +
> >>> +typedef unsigned long __attribute__ ((__vector_size__ (8))) V;
> >>> +
> >>> +V
> >>> +bar (unsigned long x, V v)
> >>> +{
> >>> +  v &= x >= v;
> >>> +  return (V) v;
> >>> +}
> >>> +
> >>> +V
> >>> +foo (void)
> >>> +{
> >>> +  return bar (5, (V) 4441221375);
> >>> +}
> >>> diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
> >>> index 6d5d65195ae..b01aa301baa 100644
> >>> --- a/gcc/tree-vect-generic.c
> >>> +++ b/gcc/tree-vect-generic.c
> >>> @@ -42,20 +42,11 @@ along with GCC; see the file COPYING3.  If not see
> >>>    #include "insn-config.h"
> >>>    #include "tree-ssa-dce.h"
> >>>    #include "recog.h"            /* FIXME: for insn_data */
> >>> +#include "gimple-fold.h"
> >>>
> >>>
> >>>    static void expand_vector_operations_1 (gimple_stmt_iterator *, 
> >>> bitmap);
> >>>
> >>> -/* Return the number of elements in a vector type TYPE that we have
> >>> -   already decided needs to be expanded piecewise.  We don't support
> >>> -   this kind of expansion for variable-length vectors, since we should
> >>> -   always check for target support before introducing uses of those.  */
> >>> -static unsigned int
> >>> -nunits_for_known_piecewise_op (const_tree type)
> >>> -{
> >>> -  return TYPE_VECTOR_SUBPARTS (type).to_constant ();
> >>> -}
> >>> -
> >>>    /* Return true if TYPE1 has more elements than TYPE2, where either
> >>>       type may be a vector or a scalar.  */
> >>>
> >>> @@ -427,35 +418,7 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, 
> >>> tree type, tree op0,
> >>>                         TYPE_VECTOR_SUBPARTS (type)
> >>>                         * GET_MODE_BITSIZE (SCALAR_TYPE_MODE
> >>>                                                  (TREE_TYPE (type)))))
> >>> -       {
> >>> -         tree inner_type = TREE_TYPE (TREE_TYPE (op0));
> >>> -         tree part_width = vector_element_bits_tree (TREE_TYPE (op0));
> >>> -         tree index = bitsize_int (0);
> >>> -         int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0));
> >>> -         int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type));
> >>> -         tree ret_type = build_nonstandard_integer_type (prec, 1);
> >>> -         tree ret_inner_type = boolean_type_node;
> >>> -         int i;
> >>> -         location_t loc = gimple_location (gsi_stmt (*gsi));
> >>> -         t = build_zero_cst (ret_type);
> >>> -
> >>> -         if (TYPE_PRECISION (ret_inner_type) != 1)
> >>> -           ret_inner_type = build_nonstandard_integer_type (1, 1);
> >>> -         warning_at (loc, OPT_Wvector_operation_performance,
> >>> -                     "vector operation will be expanded piecewise");
> >>> -         for (i = 0; i < nunits;
> >>> -              i++, index = int_const_binop (PLUS_EXPR, index, 
> >>> part_width))
> >>> -           {
> >>> -             tree a = tree_vec_extract (gsi, inner_type, op0, part_width,
> >>> -                                        index);
> >>> -             tree b = tree_vec_extract (gsi, inner_type, op1, part_width,
> >>> -                                        index);
> >>> -             tree result = gimplify_build2 (gsi, code, ret_inner_type, 
> >>> a, b);
> >>> -             t = gimplify_build3 (gsi, BIT_INSERT_EXPR, ret_type, t, 
> >>> result,
> >>> -                                  bitsize_int (i));
> >>> -           }
> >>> -         t = gimplify_build1 (gsi, VIEW_CONVERT_EXPR, type, t);
> >>> -       }
> >>> +       t = expand_cmp_piecewise (gsi, type, op0, op1);
> >>>          else
> >>>          t = expand_vector_piecewise (gsi, do_compare, type,
> >>>                                       TREE_TYPE (TREE_TYPE (op0)), op0, 
> >>> op1,
> >>> --
> >>> 2.28.0
> >>>
> >
>

Reply via email to