On Thu, Jun 13, 2013 at 8:44 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
> On Wed, 12 Jun 2013, Jeff Law wrote:
>
>>> 2013-06-13 Marc Glisse <marc.gli...@inria.fr>
>>>
>>> * tree-ssa-forwprop.c (simplify_bitwise_binary,
>>> associate_plusminus):
>>> Generalize to complex and vector.
>>> * tree.c (build_all_ones_cst): New function.
>>> * tree.h (build_all_ones_cst): Declare it.
>>
>> This is OK.
>>
>> Extra credit if you create some testcases.
>
>
> While writing the testcase, I fixed a bug in the current code (mix up
> between rhs1 and rhs2 for ~A + 1 -> -A which prevents it from ever
> triggering) and generalized the pattern (A + CST) +- CST -> A + CST to also
> allow '-' (it isn't clear to me why it wasn't handled, did I miss a
> reason?). It passes the bootstrap+testsuite on x86_64-unknown-linux-gnu.
>
> Are you ok with the (A +- CST) +- CST -> A +- CST part?
Yes, that looks fine. Please also update the overall comment before the
cases:
/* Second match patterns that allow contracting a plus-minus pair
irrespective of overflow issues.
(A +- B) - A -> +- B
(A +- B) -+ B -> A
(CST +- A) +- CST -> CST +- A
(A + CST) +- CST -> A + CST
~A + A -> -1
~A + 1 -> -A
A - (A +- B) -> -+ B
A +- (B +- A) -> +- B
CST +- (CST +- A) -> CST +- A
CST +- (A +- CST) -> CST +- A
A + ~A -> -1
you can see that we do handle
CST +- (A +- CST) -> CST +- A
so I cannot think of a reason to not handle the commutated variant.
Thus, ok with the above adjusted.
Thanks,
Richard.
> Extra piece of ChangeLog:
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/forwprop-27.c: New testcase.
>
> --
> Marc Glisse
> Index: tree-ssa-forwprop.c
> ===================================================================
> --- tree-ssa-forwprop.c (revision 200044)
> +++ tree-ssa-forwprop.c (working copy)
> @@ -1971,22 +1971,22 @@ simplify_bitwise_binary (gimple_stmt_ite
> gimple_assign_set_rhs2 (stmt, b);
> gimple_assign_set_rhs_code (stmt, def1_code);
> update_stmt (stmt);
> return true;
> }
> }
>
> /* (a | CST1) & CST2 -> (a & CST2) | (CST1 & CST2). */
> if (code == BIT_AND_EXPR
> && def1_code == BIT_IOR_EXPR
> - && TREE_CODE (arg2) == INTEGER_CST
> - && TREE_CODE (def1_arg2) == INTEGER_CST)
> + && CONSTANT_CLASS_P (arg2)
> + && CONSTANT_CLASS_P (def1_arg2))
> {
> tree cst = fold_build2 (BIT_AND_EXPR, TREE_TYPE (arg2),
> arg2, def1_arg2);
> tree tem;
> gimple newop;
> if (integer_zerop (cst))
> {
> gimple_assign_set_rhs1 (stmt, def1_arg1);
> update_stmt (stmt);
> return true;
> @@ -2002,34 +2002,33 @@ simplify_bitwise_binary (gimple_stmt_ite
> gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR);
> update_stmt (stmt);
> return true;
> }
>
> /* Combine successive equal operations with constants. */
> if ((code == BIT_AND_EXPR
> || code == BIT_IOR_EXPR
> || code == BIT_XOR_EXPR)
> && def1_code == code
> - && TREE_CODE (arg2) == INTEGER_CST
> - && TREE_CODE (def1_arg2) == INTEGER_CST)
> + && CONSTANT_CLASS_P (arg2)
> + && CONSTANT_CLASS_P (def1_arg2))
> {
> tree cst = fold_build2 (code, TREE_TYPE (arg2),
> arg2, def1_arg2);
> gimple_assign_set_rhs1 (stmt, def1_arg1);
> gimple_assign_set_rhs2 (stmt, cst);
> update_stmt (stmt);
> return true;
> }
>
> /* Canonicalize X ^ ~0 to ~X. */
> if (code == BIT_XOR_EXPR
> - && TREE_CODE (arg2) == INTEGER_CST
> && integer_all_onesp (arg2))
> {
> gimple_assign_set_rhs_with_ops (gsi, BIT_NOT_EXPR, arg1, NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> update_stmt (stmt);
> return true;
> }
>
> /* Try simple folding for X op !X, and X op X. */
> res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2);
> @@ -2472,73 +2471,75 @@ associate_plusminus (gimple_stmt_iterato
> && code != def_code)
> {
> /* (A +- B) -+ B -> A. */
> code = TREE_CODE (def_rhs1);
> rhs1 = def_rhs1;
> rhs2 = NULL_TREE;
> gimple_assign_set_rhs_with_ops (gsi, code, rhs1,
> NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> gimple_set_modified (stmt, true);
> }
> - else if (TREE_CODE (rhs2) == INTEGER_CST
> - && TREE_CODE (def_rhs1) == INTEGER_CST)
> + else if (CONSTANT_CLASS_P (rhs2)
> + && CONSTANT_CLASS_P (def_rhs1))
> {
> /* (CST +- A) +- CST -> CST +- A. */
> tree cst = fold_binary (code, TREE_TYPE (rhs1),
> def_rhs1, rhs2);
> if (cst && !TREE_OVERFLOW (cst))
> {
> code = def_code;
> gimple_assign_set_rhs_code (stmt, code);
> rhs1 = cst;
> gimple_assign_set_rhs1 (stmt, rhs1);
> rhs2 = def_rhs2;
> gimple_assign_set_rhs2 (stmt, rhs2);
> gimple_set_modified (stmt, true);
> }
> }
> - else if (TREE_CODE (rhs2) == INTEGER_CST
> - && TREE_CODE (def_rhs2) == INTEGER_CST
> - && def_code == PLUS_EXPR)
> + else if (CONSTANT_CLASS_P (rhs2)
> + && CONSTANT_CLASS_P (def_rhs2))
> {
> - /* (A + CST) +- CST -> A + CST. */
> - tree cst = fold_binary (code, TREE_TYPE (rhs1),
> + /* (A +- CST) +- CST -> A +- CST. */
> + enum tree_code mix = (code == def_code)
> + ? PLUS_EXPR : MINUS_EXPR;
> + tree cst = fold_binary (mix, TREE_TYPE (rhs1),
> def_rhs2, rhs2);
> if (cst && !TREE_OVERFLOW (cst))
> {
> - code = PLUS_EXPR;
> + code = def_code;
> gimple_assign_set_rhs_code (stmt, code);
> rhs1 = def_rhs1;
> gimple_assign_set_rhs1 (stmt, rhs1);
> rhs2 = cst;
> gimple_assign_set_rhs2 (stmt, rhs2);
> gimple_set_modified (stmt, true);
> }
> }
> }
> - else if (def_code == BIT_NOT_EXPR
> - && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> + else if (def_code == BIT_NOT_EXPR && code == PLUS_EXPR)
> {
> tree def_rhs1 = gimple_assign_rhs1 (def_stmt);
> - if (code == PLUS_EXPR
> - && operand_equal_p (def_rhs1, rhs2, 0))
> + if (operand_equal_p (def_rhs1, rhs2, 0))
> {
> /* ~A + A -> -1. */
> - code = INTEGER_CST;
> - rhs1 = build_int_cst_type (TREE_TYPE (rhs2), -1);
> + rhs1 = build_all_ones_cst (TREE_TYPE (rhs2));
> rhs2 = NULL_TREE;
> + code = TREE_CODE (rhs1);
> gimple_assign_set_rhs_with_ops (gsi, code, rhs1,
> NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> gimple_set_modified (stmt, true);
> }
> - else if (code == PLUS_EXPR
> - && integer_onep (rhs1))
> + else if ((TREE_CODE (TREE_TYPE (rhs2)) != COMPLEX_TYPE
> + && integer_onep (rhs2))
> + || (TREE_CODE (rhs2) == COMPLEX_CST
> + && integer_onep (TREE_REALPART (rhs2))
> + && integer_onep (TREE_IMAGPART (rhs2))))
> {
> /* ~A + 1 -> -A. */
> code = NEGATE_EXPR;
> rhs1 = def_rhs1;
> rhs2 = NULL_TREE;
> gimple_assign_set_rhs_with_ops (gsi, code, rhs1,
> NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> gimple_set_modified (stmt, true);
> }
> }
> @@ -2573,66 +2574,65 @@ associate_plusminus (gimple_stmt_iterato
> {
> /* A +- (B +- A) -> +- B. */
> code = ((code == PLUS_EXPR)
> ? TREE_CODE (def_rhs1) : NEGATE_EXPR);
> rhs1 = def_rhs1;
> rhs2 = NULL_TREE;
> gimple_assign_set_rhs_with_ops (gsi, code, rhs1,
> NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> gimple_set_modified (stmt, true);
> }
> - else if (TREE_CODE (rhs1) == INTEGER_CST
> - && TREE_CODE (def_rhs1) == INTEGER_CST)
> + else if (CONSTANT_CLASS_P (rhs1)
> + && CONSTANT_CLASS_P (def_rhs1))
> {
> /* CST +- (CST +- A) -> CST +- A. */
> tree cst = fold_binary (code, TREE_TYPE (rhs2),
> rhs1, def_rhs1);
> if (cst && !TREE_OVERFLOW (cst))
> {
> code = (code == def_code ? PLUS_EXPR : MINUS_EXPR);
> gimple_assign_set_rhs_code (stmt, code);
> rhs1 = cst;
> gimple_assign_set_rhs1 (stmt, rhs1);
> rhs2 = def_rhs2;
> gimple_assign_set_rhs2 (stmt, rhs2);
> gimple_set_modified (stmt, true);
> }
> }
> - else if (TREE_CODE (rhs1) == INTEGER_CST
> - && TREE_CODE (def_rhs2) == INTEGER_CST)
> + else if (CONSTANT_CLASS_P (rhs1)
> + && CONSTANT_CLASS_P (def_rhs2))
> {
> /* CST +- (A +- CST) -> CST +- A. */
> tree cst = fold_binary (def_code == code
> ? PLUS_EXPR : MINUS_EXPR,
> TREE_TYPE (rhs2),
> rhs1, def_rhs2);
> if (cst && !TREE_OVERFLOW (cst))
> {
> rhs1 = cst;
> gimple_assign_set_rhs1 (stmt, rhs1);
> rhs2 = def_rhs1;
> gimple_assign_set_rhs2 (stmt, rhs2);
> gimple_set_modified (stmt, true);
> }
> }
> }
> - else if (def_code == BIT_NOT_EXPR
> - && INTEGRAL_TYPE_P (TREE_TYPE (rhs2)))
> + else if (def_code == BIT_NOT_EXPR)
> {
> tree def_rhs1 = gimple_assign_rhs1 (def_stmt);
> if (code == PLUS_EXPR
> && operand_equal_p (def_rhs1, rhs1, 0))
> {
> /* A + ~A -> -1. */
> - code = INTEGER_CST;
> - rhs1 = build_int_cst_type (TREE_TYPE (rhs1), -1);
> + rhs1 = build_all_ones_cst (TREE_TYPE (rhs1));
> rhs2 = NULL_TREE;
> + code = TREE_CODE (rhs1);
> gimple_assign_set_rhs_with_ops (gsi, code, rhs1,
> NULL_TREE);
> gcc_assert (gsi_stmt (*gsi) == stmt);
> gimple_set_modified (stmt, true);
> }
> }
> }
> }
>
> out:
> if (gimple_modified_p (stmt))
> Index: tree.c
> ===================================================================
> --- tree.c (revision 200044)
> +++ tree.c (working copy)
> @@ -1636,20 +1636,35 @@ build_one_cst (tree type)
> case COMPLEX_TYPE:
> return build_complex (type,
> build_one_cst (TREE_TYPE (type)),
> build_zero_cst (TREE_TYPE (type)));
>
> default:
> gcc_unreachable ();
> }
> }
>
> +/* Return an integer of type TYPE containing all 1's in as much precision
> as
> + it contains, or a complex or vector whose subparts are such integers.
> */
> +
> +tree
> +build_all_ones_cst (tree type)
> +{
> + if (TREE_CODE (type) == COMPLEX_TYPE)
> + {
> + tree scalar = build_all_ones_cst (TREE_TYPE (type));
> + return build_complex (type, scalar, scalar);
> + }
> + else
> + return build_minus_one_cst (type);
> +}
> +
> /* Return a constant of arithmetic type TYPE which is the
> opposite of the multiplicative identity of the set TYPE. */
>
> tree
> build_minus_one_cst (tree type)
> {
> switch (TREE_CODE (type))
> {
> case INTEGER_TYPE: case ENUMERAL_TYPE: case BOOLEAN_TYPE:
> case POINTER_TYPE: case REFERENCE_TYPE:
> Index: tree.h
> ===================================================================
> --- tree.h (revision 200044)
> +++ tree.h (working copy)
> @@ -4761,20 +4761,21 @@ extern tree build_vector_stat (tree, tre
> extern tree build_vector_from_ctor (tree, vec<constructor_elt, va_gc> *);
> extern tree build_vector_from_val (tree, tree);
> extern tree build_constructor (tree, vec<constructor_elt, va_gc> *);
> extern tree build_constructor_single (tree, tree, tree);
> extern tree build_constructor_from_list (tree, tree);
> extern tree build_constructor_va (tree, int, ...);
> extern tree build_real_from_int_cst (tree, const_tree);
> extern tree build_complex (tree, tree, tree);
> extern tree build_one_cst (tree);
> extern tree build_minus_one_cst (tree);
> +extern tree build_all_ones_cst (tree);
> extern tree build_zero_cst (tree);
> extern tree build_string (int, const char *);
> extern tree build_tree_list_stat (tree, tree MEM_STAT_DECL);
> #define build_tree_list(t,q) build_tree_list_stat(t,q MEM_STAT_INFO)
> extern tree build_tree_list_vec_stat (const vec<tree, va_gc>
> *MEM_STAT_DECL);
> #define build_tree_list_vec(v) build_tree_list_vec_stat (v MEM_STAT_INFO)
> extern tree build_decl_stat (location_t, enum tree_code,
> tree, tree MEM_STAT_DECL);
> extern tree build_fn_decl (const char *, tree);
> #define build_decl(l,c,t,q) build_decl_stat (l,c,t,q MEM_STAT_INFO)
> Index: testsuite/gcc.dg/tree-ssa/forwprop-27.c
> ===================================================================
> --- testsuite/gcc.dg/tree-ssa/forwprop-27.c (revision 0)
> +++ testsuite/gcc.dg/tree-ssa/forwprop-27.c (revision 0)
> @@ -0,0 +1,40 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-forwprop1" } */
> +
> +typedef int V __attribute__((vector_size(2*sizeof(int))));
> +typedef __complex__ int C;
> +
> +void f (V *v1, V *v2){
> + V w1 = *v1;
> + V x1 = ~w1;
> + *v1 = x1 + 1;
> + V w2 = *v2;
> + V x2 = ~w2;
> + *v2 = x2 + w2;
> +}
> +
> +void g (V *v1, V *v2){
> + V c1 = { 5, -10 };
> + V c2 = { 32, 13 };
> + *v1 = (*v1|c1)&c2;
> + *v2 = (*v2^c1)^c2;
> +}
> +
> +void h (C *v1, C *v2){
> + C w = *v2;
> + C x = *v1 - w;
> + *v1 = x + w;
> +}
> +
> +void i (V *v1, V *v2){
> + V c1 = { 5, -10 };
> + V c2 = { 32, 13 };
> + *v1 = (*v1-c1)+c2;
> + *v2 = (c1-*v2)+c2;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "\\\+" "forwprop1"} } */
> +/* { dg-final { scan-tree-dump "{ 0, 4 }" "forwprop1"} } */
> +/* { dg-final { scan-tree-dump "{ 37, -5 }" "forwprop1"} } */
> +/* { dg-final { cleanup-tree-dump "forwprop1" } } */
> +
>
> Property changes on: testsuite/gcc.dg/tree-ssa/forwprop-27.c
> ___________________________________________________________________
> Added: svn:keywords
> + Author Date Id Revision URL
> Added: svn:eol-style
> + native
>
>
