[PATCH] VEC_COND_EXPR: do not expand comparisons feeding it

[Richard Biener]

On Tue, Jun 30, 2020 at 11:44 AM Martin Liška <mliska@suse.cz> wrote:
>
> Hi.
>
> The patch is about blocking of vector expansion of comparisons
> that are only feeding a VEC_COND_EXPR statements.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> The problematic mips64 test-case looks good now.
>
> Ready to be installed?

So why does

static tree
expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0,
                          tree op1, enum tree_code code)
{
  tree t;
  if (!expand_vec_cmp_expr_p (TREE_TYPE (op0), type, code)
      && !expand_vec_cond_expr_p (type, TREE_TYPE (op0), code))
^^^^

not return true
    {

but

/* Expand a vector condition to scalars, by using many conditions
   on the vector's elements.  */
static void
expand_vector_condition (gimple_stmt_iterator *gsi, auto_bitmap *dce_ssa_names)
{
...
  if (expand_vec_cond_expr_p (type, TREE_TYPE (a1), code))
    {
      gcc_assert (TREE_CODE (a) == SSA_NAME || TREE_CODE (a) == VECTOR_CST);
      return;

does?  What's special about the problematical mips testcase?  Can't you produce
the same "bad" result when the comparison is used in a non-VEC_COND?

There's a PR reference missing in the ChangeLog.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
>         * tree-vect-generic.c (expand_vector_comparison): Do not expand
>         comparison that only feed first argument of a VEC_COND_EXPR statement.
> ---
>   gcc/tree-vect-generic.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
> index a4b56195903..4606decd0f2 100644
> --- a/gcc/tree-vect-generic.c
> +++ b/gcc/tree-vect-generic.c
> @@ -379,6 +379,30 @@ static tree
>   expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0,
>                             tree op1, enum tree_code code)
>   {
> +  tree lhs = gimple_assign_lhs (gsi_stmt (*gsi));
> +  use_operand_p use_p;
> +  imm_use_iterator iterator;
> +  bool vec_cond_expr_only = true;
> +  bool has_use = false;
> +
> +  /* As seen in PR95830, we should not expand comparisons that are only
> +     feeding a VEC_COND_EXPR statement.  */
> +  FOR_EACH_IMM_USE_FAST (use_p, iterator, lhs)
> +    {
> +      has_use = true;
> +      gassign *use = dyn_cast<gassign *> (USE_STMT (use_p));
> +      if (use == NULL
> +         || gimple_assign_rhs_code (use) != VEC_COND_EXPR
> +         || gimple_assign_rhs1 (use) != lhs)
> +       {
> +         vec_cond_expr_only = false;
> +         break;
> +       }
> +    }
> +
> +  if (has_use && vec_cond_expr_only)
> +    return NULL_TREE;
> +
>     tree t;
>     if (!expand_vec_cmp_expr_p (TREE_TYPE (op0), type, code)
>         && !expand_vec_cond_expr_p (type, TREE_TYPE (op0), code))
> --
> 2.27.0
>