VEC_COND_EXPR: do not expand comparisons feeding it

Message ID 84c85a38-42a0-0b45-bfc5-e368c84a44b0@suse.cz
State New
Headers show
Series
  • VEC_COND_EXPR: do not expand comparisons feeding it
Related show

Commit Message

Martin Liška June 30, 2020, 9:44 a.m.
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?
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(+)

-- 
2.27.0

Comments

Peter Bergner via Gcc-patches June 30, 2020, 10:38 a.m. | #1
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

>
Martin Liška June 30, 2020, 12:16 p.m. | #2
On 6/30/20 12:38 PM, Richard Biener wrote:
> 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


Apparently because it's called for type:
(gdb) p debug_tree(gimple_expr_type(stmt))
  <vector_type 0x7ffff7721348
     type <boolean_type 0x7ffff77212a0 SI
         size <integer_cst 0x7ffff76338e8 constant 32>
         unit-size <integer_cst 0x7ffff7633900 constant 4>
         align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff77212a0 precision:32 min <integer_cst 0x7ffff773e750 -2147483648> max <integer_cst 0x7ffff773e768 2147483647>>
     BLK
     size <integer_cst 0x7ffff76336a8 type <integer_type 0x7ffff76370a8 bitsizetype> constant 64>
     unit-size <integer_cst 0x7ffff76336c0 type <integer_type 0x7ffff7637000 sizetype> constant 8>
     align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff7721348 nunits:2>

while ...

>      {

> 

> 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?


this has

  <vector_type 0x7ffff76fb3f0
     type <real_type 0x7ffff763e2a0 float SF
         size <integer_cst 0x7ffff76338e8 constant 32>
         unit-size <integer_cst 0x7ffff7633900 constant 4>
         align:32 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff763e2a0 precision:32
         pointer_to_this <pointer_type 0x7ffff763e7e0>>
     V2SF
     size <integer_cst 0x7ffff76336a8 type <integer_type 0x7ffff76370a8 bitsizetype> constant 64>
     unit-size <integer_cst 0x7ffff76336c0 type <integer_type 0x7ffff7637000 sizetype> constant 8>
     align:64 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff76fb3f0 nunits:2
     pointer_to_this <pointer_type 0x7ffff7721000>>

> What's special about the problematical mips testcase?


That we end up with something like:

   _34 = {_29, _37};
   vect_iftmp.12_35 = VEC_COND_EXPR <_34, vect__1.7_28, vect__2.10_31>;

which can't be expanded to .VCOND, but we expand to series of BIT_FIELD_REFs:

   _40 = BIT_FIELD_REF <vect__2.10_31, 32, 0>;
   _22 = BIT_FIELD_REF <_34, 32, 0>;
   _10 = _22 != 0 ? _26 : _40;
   _16 = BIT_FIELD_REF <vect__2.10_31, 32, 32>;
   _18 = BIT_FIELD_REF <_34, 32, 32>;
   _41 = _18 == 0 ? _16 : _30;


> Can't you produce

> the same "bad" result when the comparison is used in a non-VEC_COND?


Theoretically yes.

Anyway, question is how much do we care about the fallout as it happens for MIPS and
it's only a worse code stuff?

Martin

> 

> 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

>>
Peter Bergner via Gcc-patches June 30, 2020, 1:03 p.m. | #3
On Tue, Jun 30, 2020 at 2:16 PM Martin Liška <mliska@suse.cz> wrote:
>

> On 6/30/20 12:38 PM, Richard Biener wrote:

> > 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

>

> Apparently because it's called for type:

> (gdb) p debug_tree(gimple_expr_type(stmt))

>   <vector_type 0x7ffff7721348

>      type <boolean_type 0x7ffff77212a0 SI

>          size <integer_cst 0x7ffff76338e8 constant 32>

>          unit-size <integer_cst 0x7ffff7633900 constant 4>

>          align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff77212a0 precision:32 min <integer_cst 0x7ffff773e750 -2147483648> max <integer_cst 0x7ffff773e768 2147483647>>

>      BLK

>      size <integer_cst 0x7ffff76336a8 type <integer_type 0x7ffff76370a8 bitsizetype> constant 64>

>      unit-size <integer_cst 0x7ffff76336c0 type <integer_type 0x7ffff7637000 sizetype> constant 8>

>      align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff7721348 nunits:2>

>

> while ...

>

> >      {

> >

> > 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?

>

> this has

>

>   <vector_type 0x7ffff76fb3f0

>      type <real_type 0x7ffff763e2a0 float SF

>          size <integer_cst 0x7ffff76338e8 constant 32>

>          unit-size <integer_cst 0x7ffff7633900 constant 4>

>          align:32 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff763e2a0 precision:32

>          pointer_to_this <pointer_type 0x7ffff763e7e0>>

>      V2SF

>      size <integer_cst 0x7ffff76336a8 type <integer_type 0x7ffff76370a8 bitsizetype> constant 64>

>      unit-size <integer_cst 0x7ffff76336c0 type <integer_type 0x7ffff7637000 sizetype> constant 8>

>      align:64 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff76fb3f0 nunits:2

>      pointer_to_this <pointer_type 0x7ffff7721000>>

>

> > What's special about the problematical mips testcase?

>

> That we end up with something like:

>

>    _34 = {_29, _37};

>    vect_iftmp.12_35 = VEC_COND_EXPR <_34, vect__1.7_28, vect__2.10_31>;

>

> which can't be expanded to .VCOND, but we expand to series of BIT_FIELD_REFs:

>

>    _40 = BIT_FIELD_REF <vect__2.10_31, 32, 0>;

>    _22 = BIT_FIELD_REF <_34, 32, 0>;

>    _10 = _22 != 0 ? _26 : _40;

>    _16 = BIT_FIELD_REF <vect__2.10_31, 32, 32>;

>    _18 = BIT_FIELD_REF <_34, 32, 32>;

>    _41 = _18 == 0 ? _16 : _30;

>

>

> > Can't you produce

> > the same "bad" result when the comparison is used in a non-VEC_COND?

>

> Theoretically yes.

>

> Anyway, question is how much do we care about the fallout as it happens for MIPS and

> it's only a worse code stuff?


We certainly care about the fact that vector lowering will lower the
condition first
and only then come along a VEC_COND_EXPR and lowers that as well.  It's
classical fallout of separating the compare from the condition.  I see
vector lowering works on BBs in "random" order and I've always thought that it
should work more like complex lowering using a lattice to be able to optimize
things better.

Now to simply restore previous behavior for this particular case we should
probably make expand_vector_comparison walk immediate uses as you do
but then call expand_vector_condition for each VEC_COND_EXPR use,
making that return whether it "consumed" the comparison.  If all uses
consumed the comparison we should DCE it, if not, we should lower it?

Richard.

> Martin

>

> >

> > 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

> >>

>
Martin Liška July 2, 2020, 8:20 a.m. | #4
On 6/30/20 3:03 PM, Richard Biener wrote:
> Now to simply restore previous behavior for this particular case we should

> probably make expand_vector_comparison walk immediate uses as you do

> but then call expand_vector_condition for each VEC_COND_EXPR use,

> making that return whether it "consumed" the comparison.  If all uses

> consumed the comparison we should DCE it, if not, we should lower it?


All right, I prepared patch for it.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin
From 366b61fb66efa9af0cfaa2a10df72a4224c708b8 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>

Date: Tue, 30 Jun 2020 08:57:27 +0200
Subject: [PATCH] VEC_COND_EXPR: do not expand comparisons feeding it

gcc/ChangeLog:

	* tree-vect-generic.c (expand_vector_condition): Forward declaration.
	(expand_vector_comparison): Do not expand a comparison if all
	uses are consumed by a VEC_COND_EXPR.
	(expand_vector_operation): Change void return type to bool.
	(expand_vector_operations_1): Pass dce_ssa_names.
---
 gcc/tree-vect-generic.c | 59 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index a4b56195903..92884fad2d3 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -371,14 +371,53 @@ expand_vector_addition (gimple_stmt_iterator *gsi,
 				    a, b, code);
 }
 
+static bool
+expand_vector_condition (gimple_stmt_iterator *gsi, auto_bitmap *dce_ssa_names);
+
 /* Try to expand vector comparison expression OP0 CODE OP1 by
    querying optab if the following expression:
 	VEC_COND_EXPR< OP0 CODE OP1, {-1,...}, {0,...}>
    can be expanded.  */
 static tree
 expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0,
-                          tree op1, enum tree_code code)
+			  tree op1, enum tree_code code,
+			  auto_bitmap *dce_ssa_names)
 {
+  tree lhs = gimple_assign_lhs (gsi_stmt (*gsi));
+  use_operand_p use_p;
+  imm_use_iterator iterator;
+  bool vec_cond_expr_only = true;
+
+  /* As seen in PR95830, we should not expand comparisons that are only
+     feeding a VEC_COND_EXPR statement.  */
+  auto_vec<gimple *> uses;
+  FOR_EACH_IMM_USE_FAST (use_p, iterator, lhs)
+    uses.safe_push (USE_STMT (use_p));
+
+  for (unsigned i = 0; i < uses.length (); i ++)
+    {
+      gassign *use = dyn_cast<gassign *> (uses[i]);
+      if (use != NULL
+	  && gimple_assign_rhs_code (use) == VEC_COND_EXPR
+	  && gimple_assign_rhs1 (use) == lhs)
+	{
+	  gimple_stmt_iterator it = gsi_for_stmt (use);
+	  if (!expand_vector_condition (&it, dce_ssa_names))
+	    {
+	      vec_cond_expr_only = false;
+	      break;
+	    }
+	}
+      else
+	{
+	  vec_cond_expr_only = false;
+	  break;
+	}
+    }
+
+  if (!uses.is_empty () && 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))
@@ -932,7 +971,8 @@ expand_vector_divmod (gimple_stmt_iterator *gsi, tree type, tree op0,
 
 /* Expand a vector condition to scalars, by using many conditions
    on the vector's elements.  */
-static void
+
+static bool
 expand_vector_condition (gimple_stmt_iterator *gsi, auto_bitmap *dce_ssa_names)
 {
   gassign *stmt = as_a <gassign *> (gsi_stmt (*gsi));
@@ -975,7 +1015,7 @@ 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;
+      return true;
     }
 
   /* Handle vector boolean types with bitmasks.  If there is a comparison
@@ -1006,7 +1046,7 @@ expand_vector_condition (gimple_stmt_iterator *gsi, auto_bitmap *dce_ssa_names)
       a = gimplify_build2 (gsi, BIT_IOR_EXPR, type, a1, a2);
       gimple_assign_set_rhs_from_tree (gsi, a);
       update_stmt (gsi_stmt (*gsi));
-      return;
+      return true;
     }
 
   /* TODO: try and find a smaller vector type.  */
@@ -1070,11 +1110,14 @@ expand_vector_condition (gimple_stmt_iterator *gsi, auto_bitmap *dce_ssa_names)
   if (a_is_comparison)
     bitmap_set_bit (*dce_ssa_names,
 		    SSA_NAME_VERSION (gimple_assign_lhs (assign)));
+
+  return false;
 }
 
 static tree
 expand_vector_operation (gimple_stmt_iterator *gsi, tree type, tree compute_type,
-			 gassign *assign, enum tree_code code)
+			 gassign *assign, enum tree_code code,
+			 auto_bitmap *dce_ssa_names)
 {
   machine_mode compute_mode = TYPE_MODE (compute_type);
 
@@ -1128,7 +1171,8 @@ expand_vector_operation (gimple_stmt_iterator *gsi, tree type, tree compute_type
 	  tree rhs1 = gimple_assign_rhs1 (assign);
 	  tree rhs2 = gimple_assign_rhs2 (assign);
 
-	  return expand_vector_comparison (gsi, type, rhs1, rhs2, code);
+	  return expand_vector_comparison (gsi, type, rhs1, rhs2, code,
+					   dce_ssa_names);
 	}
 
       case TRUNC_DIV_EXPR:
@@ -2213,7 +2257,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi,
   if (compute_type == type)
     return;
 
-  new_rhs = expand_vector_operation (gsi, type, compute_type, stmt, code);
+  new_rhs = expand_vector_operation (gsi, type, compute_type, stmt, code,
+				     dce_ssa_names);
 
   /* Leave expression untouched for later expansion.  */
   if (new_rhs == NULL_TREE)
-- 
2.27.0
Peter Bergner via Gcc-patches July 2, 2020, 9:06 a.m. | #5
On Thu, Jul 2, 2020 at 10:20 AM Martin Liška <mliska@suse.cz> wrote:
>

> On 6/30/20 3:03 PM, Richard Biener wrote:

> > Now to simply restore previous behavior for this particular case we should

> > probably make expand_vector_comparison walk immediate uses as you do

> > but then call expand_vector_condition for each VEC_COND_EXPR use,

> > making that return whether it "consumed" the comparison.  If all uses

> > consumed the comparison we should DCE it, if not, we should lower it?

>

> All right, I prepared patch for it.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.


OK.  Can you please replace 'auto_bitmap *' args with 'bitmap'?
auto_bitmap decays to bitmap just fine.

Thanks,
Richard.

> Ready to be installed?

> Thanks,

> Martin

Patch

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))