On 6/30/20 12:38 PM, Richard Biener wrote:
On Tue, Jun 30, 2020 at 11:44 AM Martin Liška <mli...@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


Reply via email to