On Sat, Sep 29, 2012 at 3:25 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> Hello,
>
> this patch does 2 things (I should have split it in 2, but the questions go
> together):
>
> 1) it handles constant folding of vector comparisons,
>
> 2) it fixes another place where vectors are not expected (I'll probably wait
> to have front-end support and testcases to do more of those, but there is
> something to discuss).
>
> I wasn't sure what integer_truep should test exactly. For integer: == 1 or
> != 0? For vectors: == -1 or < 0? I chose the one that worked best for the
> forwprop case where I used it.
>
> It seems that before this patch, the middle-end didn't know how comparison
> results were encoded (a good reason for VEC_COND_EXPR to require a
> comparison as its first argument). I am using the OpenCL encoding that what
> matters is the high bit of each vector element. I am not quite sure what
> happens for targets (are there any?) that use a different encoding. When
> expanding vcond, they can do the comparison as they like. When expanding an
> isolated comparison, I expect they have to expand it as vcond(a<b,-1,0). So
> it should be ok, but I could easily have missed something.
Comments below
>
> 2012-10-01 Marc Glisse <marc.gli...@inria.fr>
>
> gcc/
> * tree.c (integer_truep): New function.
> * tree.h (integer_truep): Declare.
> * tree-ssa-forwprop.c (forward_propagate_into_cond): Call it.
> Don't use boolean_type_node for vectors.
> * fold-const.c (fold_relational_const): Handle VECTOR_CST.
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/foldconst-6.c: New testcase.
>
> --
> Marc Glisse
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h (revision 191850)
> +++ gcc/tree.h (working copy)
> @@ -5272,20 +5272,25 @@ extern int integer_zerop (const_tree);
>
> /* integer_onep (tree x) is nonzero if X is an integer constant of value 1.
> */
>
> extern int integer_onep (const_tree);
>
> /* integer_all_onesp (tree x) is nonzero if X is an integer constant
> all of whose significant bits are 1. */
>
> extern int integer_all_onesp (const_tree);
>
> +/* integer_truep (tree x) is nonzero if X is an integer constant of value
> 1,
> + or a vector constant of value < 0. */
> +
> +extern bool integer_truep (const_tree);
> +
> /* integer_pow2p (tree x) is nonzero is X is an integer constant with
> exactly one bit 1. */
>
> extern int integer_pow2p (const_tree);
>
> /* integer_nonzerop (tree x) is nonzero if X is an integer constant
> with a nonzero value. */
>
> extern int integer_nonzerop (const_tree);
>
> Index: gcc/tree-ssa-forwprop.c
> ===================================================================
> --- gcc/tree-ssa-forwprop.c (revision 191850)
> +++ gcc/tree-ssa-forwprop.c (working copy)
> @@ -564,46 +564,46 @@ forward_propagate_into_cond (gimple_stmt
> enum tree_code code;
> tree name = cond;
> gimple def_stmt = get_prop_source_stmt (name, true, NULL);
> if (!def_stmt || !can_propagate_from (def_stmt))
> return 0;
>
> code = gimple_assign_rhs_code (def_stmt);
> if (TREE_CODE_CLASS (code) == tcc_comparison)
> tmp = fold_build2_loc (gimple_location (def_stmt),
> code,
> - boolean_type_node,
> + TREE_TYPE (cond),
That's obvious.
> gimple_assign_rhs1 (def_stmt),
> gimple_assign_rhs2 (def_stmt));
> else if ((code == BIT_NOT_EXPR
> && TYPE_PRECISION (TREE_TYPE (cond)) == 1)
> || (code == BIT_XOR_EXPR
> - && integer_onep (gimple_assign_rhs2 (def_stmt))))
> + && integer_truep (gimple_assign_rhs2 (def_stmt))))
See below.
> {
> tmp = gimple_assign_rhs1 (def_stmt);
> swap = true;
> }
> }
>
> if (tmp
> && is_gimple_condexpr (tmp))
> {
> if (dump_file && tmp)
> {
> fprintf (dump_file, " Replaced '");
> print_generic_expr (dump_file, cond, 0);
> fprintf (dump_file, "' with '");
> print_generic_expr (dump_file, tmp, 0);
> fprintf (dump_file, "'\n");
> }
>
> - if (integer_onep (tmp))
> + if (integer_truep (tmp))
> gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs2 (stmt));
> else if (integer_zerop (tmp))
> gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs3 (stmt));
> else
> {
> gimple_assign_set_rhs1 (stmt, unshare_expr (tmp));
> if (swap)
> {
> tree t = gimple_assign_rhs2 (stmt);
> gimple_assign_set_rhs2 (stmt, gimple_assign_rhs3 (stmt));
> Index: gcc/testsuite/gcc.dg/tree-ssa/foldconst-6.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/foldconst-6.c (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/foldconst-6.c (revision 0)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-ccp1" } */
> +
> +typedef long vec __attribute__ ((vector_size (2 * sizeof(long))));
> +
> +vec f ()
> +{
> + vec a = { -2, 666 };
> + vec b = { 3, 2 };
> + return a < b;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "666" "ccp1"} } */
> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/foldconst-6.c
> ___________________________________________________________________
> Added: svn:keywords
> + Author Date Id Revision URL
> Added: svn:eol-style
> + native
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 191850)
> +++ gcc/fold-const.c (working copy)
> @@ -16084,20 +16084,44 @@ fold_relational_const (enum tree_code co
> TREE_IMAGPART (op0),
> TREE_IMAGPART (op1));
> if (code == EQ_EXPR)
> return fold_build2 (TRUTH_ANDIF_EXPR, type, rcond, icond);
> else if (code == NE_EXPR)
> return fold_build2 (TRUTH_ORIF_EXPR, type, rcond, icond);
> else
> return NULL_TREE;
> }
>
> + if (TREE_CODE (op0) == VECTOR_CST && TREE_CODE (op1) == VECTOR_CST)
> + {
> + int count = VECTOR_CST_NELTS (op0);
> + tree *elts = XALLOCAVEC (tree, count);
> + gcc_assert (TREE_CODE (type) == VECTOR_TYPE);
> +
> + for (int i = 0; i < count; i++)
> + {
> + tree elem_type = TREE_TYPE (type);
> + tree elem0 = VECTOR_CST_ELT (op0, i);
> + tree elem1 = VECTOR_CST_ELT (op1, i);
> +
> + elts[i] = fold_relational_const (code, elem_type,
> + elem0, elem1);
> +
> + if(elts[i] == NULL_TREE)
> + return NULL_TREE;
> +
> + elts[i] = fold_negate_const (elts[i], elem_type);
I think you need to invent something new similar to STORE_FLAG_VALUE
or use STORE_FLAG_VALUE here. With the above you try to map
{0, 1} to {0, -1} which is only true if the operation on the element types
returns {0, 1} (thus, STORE_FLAG_VALUE is 1).
> + }
> +
> + return build_vector (type, elts);
> + }
> +
> /* From here on we only handle LT, LE, GT, GE, EQ and NE.
>
> To compute GT, swap the arguments and do LT.
> To compute GE, do LT and invert the result.
> To compute LE, swap the arguments, do LT and invert the result.
> To compute NE, do EQ and invert the result.
>
> Therefore, the code below must handle only EQ and LT. */
>
> if (code == LE_EXPR || code == GT_EXPR)
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c (revision 191850)
> +++ gcc/tree.c (working copy)
> @@ -1835,20 +1835,48 @@ integer_all_onesp (const_tree expr)
> else
> high_value = ((HOST_WIDE_INT) 1 << shift_amount) - 1;
>
> return (TREE_INT_CST_LOW (expr) == ~(unsigned HOST_WIDE_INT) 0
> && TREE_INT_CST_HIGH (expr) == high_value);
> }
> else
> return TREE_INT_CST_LOW (expr) == ((unsigned HOST_WIDE_INT) 1 << prec)
> - 1;
> }
>
> +/* Return true if EXPR is an integer constant representing true. */
> +
> +bool
> +integer_truep (const_tree expr)
> +{
> + STRIP_NOPS (expr);
> +
> + switch (TREE_CODE (expr))
> + {
> + case INTEGER_CST:
> + /* Do not just test != 0, some places expect the value 1. */
> + return (TREE_INT_CST_LOW (expr) == 1
> + && TREE_INT_CST_HIGH (expr) == 0);
I wonder if using STORE_FLAG_VALUE is better here (note that it
usually differs for FP vs. integral comparisons and the mode passed
to STORE_FLAG_VALUE is that of the comparison result).
That said, until we are sure what semantics we want here (forwprop
for example doesn't look at 'comparisons' but operations on special
values and types) I'd prefer to not introduce integer_truep ().
Thanks,
Richard.
> + case VECTOR_CST:
> + {
> + for (unsigned i = 0; i < VECTOR_CST_NELTS (expr); ++i)
> + {
> + tree elm = VECTOR_CST_ELT (expr, i);
> + if (TREE_CODE (elm) != INTEGER_CST || !tree_int_cst_sign_bit
> (elm))
> + return false;
> + }
> + return true;
> + }
> + default:
> + return false;
> + }
> +}
> +
> /* Return 1 if EXPR is an integer constant that is a power of 2 (i.e., has
> only
> one bit on). */
>
> int
> integer_pow2p (const_tree expr)
> {
> int prec;
> unsigned HOST_WIDE_INT high, low;
>
> STRIP_NOPS (expr);
>
