From patchwork Sat Sep 8 08:17:55 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: combine BIT_FIELD_REF and VEC_PERM_EXPR Date: Fri, 07 Sep 2012 22:17:55 -0000 From: Marc Glisse X-Patchwork-Id: 182506 Message-Id: To: Richard Guenther Cc: gcc-patches@gcc.gnu.org On Mon, 3 Sep 2012, Richard Guenther wrote: > Please do the early outs where you compute the arguments. Thus, right > after getting op0 in this case or right after computing n for the n != 1 check. > > I think you need to verify that the type of 'op' is actually the element type > of op0. The BIT_FIELD_REF can happily access elements two and three > of { 1, 2, 3, 4 } as a long for example. See the BIT_FIELD_REF foldings > in fold-const.c. On Mon, 3 Sep 2012, Richard Guenther wrote: > If you use fold_build3 you need to check that the result is in expected form > (a is_gimple_invariant or an SSA_NAME). I first tried this: + if (TREE_CODE (tem) != SSA_NAME + && TREE_CODE (tem) != BIT_FIELD_REF + && !is_gimple_min_invariant (tem)) + return false; but then I thought that fold_stmt probably does the right thing (?) so I switched to it. >> Now that I look at this line, I wonder if I am missing some unshare_expr for >> p and/or op1. > > If either is a CONSTRUCTOR and its def stmt is not removed and it survives > into tem then yes ... I added an unconditional unshare_expr. I guess it would be possible to look at if the permutation is only used once and in that case maybe not call unshare_expr (?) and call remove_prop_source_from_use at the end, but it gets a bit complicated and I don't think it helps compared to waiting for the next DCE pass. >>> Please also add handling of code == CONSTRUCTOR. >> >> The cases I tried were already handled by fre1. I can add code for >> constructor, but I'll need to look for a testcase first. Can that go to a >> different patch? > > Yes. This is currently handled by FRE. The only testcase I have that reaches forwprop is bit_field_ref (vec_perm_expr (constructor)) and will require me to disable this patch so I can test that one... New version of the patch (I really think I already regtested it, but I'll do it again to be sure): 2012-09-08 Marc Glisse gcc/ * tree-ssa-forwprop.c (simplify_bitfield): New function. (ssa_forward_propagate_and_combine): Call it. gcc/testsuite/ * gcc.dg/tree-ssa/forwprop-21.c: New testcase. Index: tree-ssa-forwprop.c =================================================================== --- tree-ssa-forwprop.c (revision 191089) +++ tree-ssa-forwprop.c (working copy) @@ -2567,20 +2567,92 @@ combine_conversions (gimple_stmt_iterato gimple_assign_set_rhs_code (stmt, CONVERT_EXPR); update_stmt (stmt); return remove_prop_source_from_use (op0) ? 2 : 1; } } } return 0; } +/* Combine an element access with a shuffle. Returns true if there were + any changes made, else it returns false. */ + +static bool +simplify_bitfield (gimple_stmt_iterator *gsi) +{ + gimple stmt = gsi_stmt (*gsi); + gimple def_stmt; + tree op, op0, op1, op2; + tree elem_type; + unsigned idx, n, size; + enum tree_code code; + + op = gimple_assign_rhs1 (stmt); + gcc_checking_assert (TREE_CODE (op) == BIT_FIELD_REF); + + op0 = TREE_OPERAND (op, 0); + if (TREE_CODE (op0) != SSA_NAME + || TREE_CODE (TREE_TYPE (op0)) != VECTOR_TYPE) + return false; + + elem_type = TREE_TYPE (TREE_TYPE (op0)); + if (TREE_TYPE (op) != elem_type) + return false; + + size = TREE_INT_CST_LOW (TYPE_SIZE (elem_type)); + op1 = TREE_OPERAND (op, 1); + n = TREE_INT_CST_LOW (op1) / size; + if (n != 1) + return false; + + def_stmt = SSA_NAME_DEF_STMT (op0); + if (!def_stmt || !is_gimple_assign (def_stmt) + || !can_propagate_from (def_stmt)) + return false; + + op2 = TREE_OPERAND (op, 2); + idx = TREE_INT_CST_LOW (op2) / size; + + code = gimple_assign_rhs_code (def_stmt); + + if (code == VEC_PERM_EXPR) + { + tree p, m, index, tem; + unsigned nelts; + m = gimple_assign_rhs3 (def_stmt); + if (TREE_CODE (m) != VECTOR_CST) + return false; + nelts = VECTOR_CST_NELTS (m); + idx = TREE_INT_CST_LOW (VECTOR_CST_ELT (m, idx)); + idx %= 2 * nelts; + if (idx < nelts) + { + p = gimple_assign_rhs1 (def_stmt); + } + else + { + p = gimple_assign_rhs2 (def_stmt); + idx -= nelts; + } + index = build_int_cst (TREE_TYPE (TREE_TYPE (m)), idx * size); + tem = build3 (BIT_FIELD_REF, TREE_TYPE (op), + unshare_expr (p), op1, index); + gimple_assign_set_rhs1 (stmt, tem); + fold_stmt (gsi); + update_stmt (gsi_stmt (*gsi)); + return true; + } + + return false; +} + /* Determine whether applying the 2 permutations (mask1 then mask2) gives back one of the input. */ static int is_combined_permutation_identity (tree mask1, tree mask2) { tree mask; unsigned int nelts, i, j; bool maybe_identity1 = true; bool maybe_identity2 = true; @@ -2825,20 +2897,22 @@ ssa_forward_propagate_and_combine (void) cfg_changed = true; changed = did_something != 0; } else if (code == VEC_PERM_EXPR) { int did_something = simplify_permutation (&gsi); if (did_something == 2) cfg_changed = true; changed = did_something != 0; } + else if (code == BIT_FIELD_REF) + changed = simplify_bitfield (&gsi); break; } case GIMPLE_SWITCH: changed = simplify_gimple_switch (stmt); break; case GIMPLE_COND: { int did_something; Index: testsuite/gcc.dg/tree-ssa/forwprop-21.c =================================================================== --- testsuite/gcc.dg/tree-ssa/forwprop-21.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/forwprop-21.c (revision 0) @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-optimized" } */ +typedef int v4si __attribute__ ((vector_size (4 * sizeof(int)))); + +int +test (v4si *x, v4si *y) +{ + v4si m = { 2, 3, 6, 5 }; + v4si z = __builtin_shuffle (*x, *y, m); + return z[2]; +} +/* { dg-final { scan-tree-dump-not "VEC_PERM_EXPR" "optimized" } } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */