On Mon, Aug 04, 2014 at 02:04:36PM +0200, Richard Biener wrote: > > Looks like .fre can optimize "q - (q - 1)" into 1: > > <bb 2>: > > q.0_3 = (long int) &MEM[(void *)&i + 4B]; > > _5 = (long int) &i; > > - _6 = q.0_3 - _5; > > - t.1_7 = _6 /[ex] 4; > > - t ={v} t.1_7; > > + t ={v} 1; > > i ={v} {CLOBBER}; > > return; > > > > But associate_plusminus doesn't optimize it: > > else if (code == MINUS_EXPR > > && CONVERT_EXPR_CODE_P (def_code) > > && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME > > && TREE_CODE (rhs2) == SSA_NAME) > > { > > /* (T)(P + A) - (T)P -> (T)A. */ > > becase gimple_assign_rhs1 (def_stmt) is not an SSA_NAME, but ADDR_EXPR (it's > > &MEM[(void *)&i + 4B]). Then there's transformation A - (A +- B) -> -+ B > > below, but that doesn't handle casts. > > > > So - should I try to handle it in associate_plusminus? > > Yes please, with a (few) testcase(s).
Ok, so the following adds the (T)P - (T)(P + A) -> (T)-A transformation. It is based on code hunk that does the (T)(P + A) - (T)P -> (T)A transformation. The difference it makes is in the .optimized dump something like: int fn2(int, int) (int p, int i) { - unsigned int p.2_2; + unsigned int _3; int _4; - unsigned int _5; unsigned int _6; - int _7; <bb 2>: - p.2_2 = (unsigned int) p_1(D); - _4 = p_1(D) + i_3(D); - _5 = (unsigned int) _4; - _6 = p.2_2 - _5; - _7 = (int) _6; - return _7; + _6 = (unsigned int) i_2(D); + _3 = -_6; + _4 = (int) _3; + return _4; i.e., the PLUS_EXPR and MINUS_EXPR are gone, and NEGATE_EXPR is used instead. During bootstrap with --enable-languages=c,c++ this optimization triggered 238 times. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-05 Marek Polacek <pola...@redhat.com> PR c/61240 * tree-ssa-forwprop.c (associate_plusminus): Add (T)P - (T)(P + A) -> (T)-A transformation. c/ * c-typeck.c (pointer_diff): Remove P - (P + CST) optimization. testsuite/ * gcc.dg/pr61240.c: New test. * gcc.dg/tree-ssa/forwprop-29.c: New test. diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index fe9440c..5f46944 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -3460,7 +3460,6 @@ pointer_diff (location_t loc, tree op0, tree op1) addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op0))); addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op1))); tree target_type = TREE_TYPE (TREE_TYPE (op0)); - tree con0, con1, lit0, lit1; tree orig_op1 = op1; /* If the operands point into different address spaces, we need to @@ -3490,7 +3489,6 @@ pointer_diff (location_t loc, tree op0, tree op1) else inttype = restype; - if (TREE_CODE (target_type) == VOID_TYPE) pedwarn (loc, OPT_Wpointer_arith, "pointer of type %<void *%> used in subtraction"); @@ -3498,50 +3496,6 @@ pointer_diff (location_t loc, tree op0, tree op1) pedwarn (loc, OPT_Wpointer_arith, "pointer to a function used in subtraction"); - /* If the conversion to ptrdiff_type does anything like widening or - converting a partial to an integral mode, we get a convert_expression - that is in the way to do any simplifications. - (fold-const.c doesn't know that the extra bits won't be needed. - split_tree uses STRIP_SIGN_NOPS, which leaves conversions to a - different mode in place.) - So first try to find a common term here 'by hand'; we want to cover - at least the cases that occur in legal static initializers. */ - if (CONVERT_EXPR_P (op0) - && (TYPE_PRECISION (TREE_TYPE (op0)) - == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0))))) - con0 = TREE_OPERAND (op0, 0); - else - con0 = op0; - if (CONVERT_EXPR_P (op1) - && (TYPE_PRECISION (TREE_TYPE (op1)) - == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op1, 0))))) - con1 = TREE_OPERAND (op1, 0); - else - con1 = op1; - - if (TREE_CODE (con0) == POINTER_PLUS_EXPR) - { - lit0 = TREE_OPERAND (con0, 1); - con0 = TREE_OPERAND (con0, 0); - } - else - lit0 = integer_zero_node; - - if (TREE_CODE (con1) == POINTER_PLUS_EXPR) - { - lit1 = TREE_OPERAND (con1, 1); - con1 = TREE_OPERAND (con1, 0); - } - else - lit1 = integer_zero_node; - - if (operand_equal_p (con0, con1, 0)) - { - op0 = lit0; - op1 = lit1; - } - - /* First do the subtraction as integers; then drop through to build the divide operator. Do not do default conversions on the minus operator diff --git gcc/testsuite/gcc.dg/pr61240.c gcc/testsuite/gcc.dg/pr61240.c index e69de29..115e415 100644 --- gcc/testsuite/gcc.dg/pr61240.c +++ gcc/testsuite/gcc.dg/pr61240.c @@ -0,0 +1,13 @@ +/* PR c/61240 */ +/* { dg-do compile } */ + +void +foo (void) +{ + volatile __PTRDIFF_TYPE__ t; + int i; + int *p = &i; + int *q = &i + 1; + t = q - (q - 1); + t = p - (p - 1); +} diff --git gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c index e69de29..745a494 100644 --- gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c +++ gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c @@ -0,0 +1,85 @@ +/* { dg-do run } */ +/* { dg-options "-O -fdump-tree-forwprop1" } */ + +__PTRDIFF_TYPE__ __attribute__((noinline, noclone)) +fn1 (int *p, long int i) +{ + return p - (p + i); +} + +int __attribute__((noinline, noclone)) +fn2 (int p, int i) +{ + return (long) p - (p + i); +} + +long int __attribute__((noinline, noclone)) +fn3 (long int c, long int i) +{ + return (unsigned long) c - (c + i); +} + +int +main () +{ + int l = 10; + if (fn1 (&l, 1) != -1 + || fn1 (&l, -1) != 1 + || fn1 (&l, 0) != 0 + || fn1 (&l, 0x186A0) != -0x186A0 + || fn1 (&l, 0xb3c2) != -0xb3c2 + || fn1 (&l, 0xb3) != -0xb3 + || fn1 (&l, 0xacf) != -0xacf + || fn1 (&l, 0xf) != -0xf + || fn1 (&l, 0x2468acf) != -0x2468acf + || fn1 (&l, 0x91a2b3c) != -0x91a2b3c + || fn1 (&l, -0x186A0) != 0x186A0 + || fn1 (&l, -0xb3c2) != 0xb3c2 + || fn1 (&l, -0xb3) != 0xb3 + || fn1 (&l, -0xacf) != 0xacf + || fn1 (&l, -0xf) != 0xf + || fn1 (&l, -0x2468acf) != 0x2468acf + || fn1 (&l, -0x91a2b3c) != 0x91a2b3c) + __builtin_abort (); + if (fn2 (l, 1) != -1 + || fn2 (l, -1) != 1 + || fn2 (l, 0) != 0 + || fn2 (l, 0x186A0) != -0x186A0 + || fn2 (l, 0xb3c2) != -0xb3c2 + || fn2 (l, 0xb3) != -0xb3 + || fn2 (l, 0xacf) != -0xacf + || fn2 (l, 0xf) != -0xf + || fn2 (l, 0x2468acf) != -0x2468acf + || fn2 (l, 0x91a2b3c) != -0x91a2b3c + || fn2 (l, -0x186A0) != 0x186A0 + || fn2 (l, -0xb3c2) != 0xb3c2 + || fn2 (l, -0xb3) != 0xb3 + || fn2 (l, -0xacf) != 0xacf + || fn2 (l, -0xf) != 0xf + || fn2 (l, -0x2468acf) != 0x2468acf + || fn2 (l, -0x91a2b3c) != 0x91a2b3c) + __builtin_abort (); + if (fn3 (l, 1) != -1 + || fn3 (l, -1) != 1 + || fn3 (l, 0) != 0 + || fn3 (l, 0x186A0) != -0x186A0 + || fn3 (l, 0xb3c2) != -0xb3c2 + || fn3 (l, 0xb3) != -0xb3 + || fn3 (l, 0xacf) != -0xacf + || fn3 (l, 0xf) != -0xf + || fn3 (l, 0x2468acf) != -0x2468acf + || fn3 (l, 0x91a2b3c) != -0x91a2b3c + || fn3 (l, -0x186A0) != 0x186A0 + || fn3 (l, -0xb3c2) != 0xb3c2 + || fn3 (l, -0xb3) != 0xb3 + || fn3 (l, -0xacf) != 0xacf + || fn3 (l, -0xf) != 0xf + || fn3 (l, -0x2468acf) != 0x2468acf + || fn3 (l, -0x91a2b3c) != 0x91a2b3c) + __builtin_abort (); + + return 0; +} + +/* { dg-final { scan-tree-dump-not " - " "forwprop1" } } */ +/* { dg-final { cleanup-tree-dump "forwprop1" } } */ diff --git gcc/tree-ssa-forwprop.c gcc/tree-ssa-forwprop.c index 0284301..c59f1d7 100644 --- gcc/tree-ssa-forwprop.c +++ gcc/tree-ssa-forwprop.c @@ -2534,13 +2534,14 @@ associate_plusminus (gimple_stmt_iterator *gsi) (CST +- A) +- CST -> CST +- A (A +- CST) +- CST -> A +- CST ~A + A -> -1 - ~A + 1 -> -A + ~A + 1 -> -A A - (A +- B) -> -+ B A +- (B +- A) -> +- B CST +- (CST +- A) -> CST +- A CST +- (A +- CST) -> CST +- A A + ~A -> -1 (T)(P + A) - (T)P -> (T)A + (T)P - (T)(P + A) -> (T)-A via commutating the addition and contracting operations to zero by reassociation. */ @@ -2799,6 +2800,76 @@ associate_plusminus (gimple_stmt_iterator *gsi) gimple_set_modified (stmt, true); } } + else if (code == MINUS_EXPR + && CONVERT_EXPR_CODE_P (def_code) + && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME + && TREE_CODE (rhs1) == SSA_NAME) + { + /* (T)P - (T)(P + A) -> (T)-A. */ + gimple def_stmt2 = SSA_NAME_DEF_STMT (rhs1); + if (is_gimple_assign (def_stmt2) + && can_propagate_from (def_stmt2) + && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt2)) + && TREE_CODE (gimple_assign_rhs1 (def_stmt2)) == SSA_NAME) + { + /* Now we have (T)P - (T)X. */ + tree p = gimple_assign_rhs1 (def_stmt2); + def_stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (def_stmt)); + if (is_gimple_assign (def_stmt2) + && can_propagate_from (def_stmt2) + && (gimple_assign_rhs_code (def_stmt2) == POINTER_PLUS_EXPR + || gimple_assign_rhs_code (def_stmt2) == PLUS_EXPR) + && gimple_assign_rhs1 (def_stmt2) == p) + { + /* And finally (T)P - (T)(P + A). */ + tree a = gimple_assign_rhs2 (def_stmt2); + if (TYPE_PRECISION (TREE_TYPE (rhs1)) + <= TYPE_PRECISION (TREE_TYPE (a)) + /* For integer types, if A has a smaller type + than T the result depends on the possible + overflow in P + A. + E.g. T=size_t, A=(unsigned)429497295, P>0. + However, if an overflow in P + A would cause + undefined behavior, we can assume that there + is no overflow. */ + || (INTEGRAL_TYPE_P (TREE_TYPE (p)) + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (p))) + /* For pointer types, if the conversion of A to the + final type requires a sign- or zero-extension, + then we have to punt - it is not defined which + one is correct. */ + || (POINTER_TYPE_P (TREE_TYPE (p)) + && TREE_CODE (a) == INTEGER_CST + && tree_int_cst_sign_bit (a) == 0)) + { + if (issue_strict_overflow_warning + (WARN_STRICT_OVERFLOW_MISC) + && TYPE_PRECISION (TREE_TYPE (rhs1)) + > TYPE_PRECISION (TREE_TYPE (a)) + && INTEGRAL_TYPE_P (TREE_TYPE (p))) + warning_at (gimple_location (stmt), + OPT_Wstrict_overflow, + "assuming signed overflow does not " + "occur when assuming that " + "(T)P - (T)(P + A) is always (T)-A"); + if (!useless_type_conversion_p (TREE_TYPE (rhs1), + TREE_TYPE (a))) + { + a = fold_convert (TREE_TYPE (rhs1), a); + a = force_gimple_operand_gsi (gsi, a, true, + NULL_TREE, true, + GSI_SAME_STMT); + } + rhs1 = a; + rhs2 = NULL_TREE; + gimple_assign_set_rhs_with_ops (gsi, NEGATE_EXPR, + rhs1, rhs2); + gcc_assert (gsi_stmt (*gsi) == stmt); + gimple_set_modified (stmt, true); + } + } + } + } } } Marek