On Mon, Oct 5, 2015 at 5:02 PM, Richard Sandiford
<richard.sandif...@arm.com> wrote:
> The upcoming patch to move sqrt and cbrt simplifications to match.pd
> caused a regression because the (abs @0)->@0 simplification didn't
> trigger for:
>
> (abs (convert (abs X)))
>
> The simplification is based on tree_expr_nonnegative_p, which is
> pretty weak for gimple (it gives up if it sees an SSA_NAME).
>
> We have the stronger gimple_val_nonnegative_real_p, but (a) as its
> name implies, it's specific to reals and (b) in its current form it
> doesn't handle converts. This patch:
>
> - generalises the routine all types
> - reuses tree_{unary,binary,call}_nonnegative_warnv_p for the leaf cases
> - makes the routine handle CONVERT_EXPR
> - allows a nesting depth of 1 for CONVERT_EXPR
> - uses the routine instead of tree_expr_nonnegative_p for gimple.
>
> Limiting the depth to 1 is a little arbitrary but adding a param seemed
> over the top.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu. I didn't write
> a specific test because this is already covered by the testsuite if
> the follow-on patch is also applied. OK to install?
Hmm. I don't like having essentially two copies of the same machinery.
Can you instead fold gimple_val_nonnegative_real_p into a
tree_ssa_name_nonnegative_warnv_p used by tree_expr_nonnegative_warnv_p?
For integers it's also possible to look at SSA name range info.
You'd still limit recursion appropriately (by passing down a depth arg
everywhere,
defaulted to 0 I guess).
Note that the comment in gimple_val_nonnegative_real_p is correct in that
we really shouldn't recurse (but maybe handle fixed patterns - like you do here)
as the appropriate way is to have a "nonnegative" lattice. SSA name range info
may already provide enough info here (well, not for reals - time to add basic
real range support to VRP!).
Thanks,
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * gimple-fold.h (gimple_val_nonnegative_real_p): Replace with...
> (gimple_val_nonnegative_p): ...this new function.
> * gimple-fold.c (gimple_val_nonnegative_real_p): Replace with...
> (gimple_val_nonnegative_p): ...this new function. Add a nesting
> depth. Handle conversions and allow them to be nested to a depth
> of 1. Generalize to non-reals. Use tree_binary_nonnegative_warnv_p,
> tree_unary_nonnegative_warnv_p and tree_call_nonnegative_warnv_p.
> * tree-ssa-math-opts.c (gimple_expand_builtin_pow): Update
> accordingly.
> * match.pd (nonnegative_p): New predicate. Use it instead of
> tree_expr_nonnegative_p to detect redundant abs expressions.
>
> Index: a/gcc/gimple-fold.c
> ===================================================================
> *** a/gcc/gimple-fold.c
> --- b/gcc/gimple-fold.c
> *************** gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree
> known_binfo,
> *** 5773,5787 ****
> }
>
> /* Return true iff VAL is a gimple expression that is known to be
> ! non-negative. Restricted to floating-point inputs. */
>
> bool
> ! gimple_val_nonnegative_real_p (tree val)
> {
> gimple *def_stmt;
>
> - gcc_assert (val && SCALAR_FLOAT_TYPE_P (TREE_TYPE (val)));
> -
> /* Use existing logic for non-gimple trees. */
> if (tree_expr_nonnegative_p (val))
> return true;
> --- 5773,5785 ----
> }
>
> /* Return true iff VAL is a gimple expression that is known to be
> ! non-negative. DEPTH is the nesting depth. */
>
> bool
> ! gimple_val_nonnegative_p (tree val, unsigned int depth)
> {
> gimple *def_stmt;
>
> /* Use existing logic for non-gimple trees. */
> if (tree_expr_nonnegative_p (val))
> return true;
> *************** gimple_val_nonnegative_real_p (tree val)
> *** 5789,5906 ****
> if (TREE_CODE (val) != SSA_NAME)
> return false;
>
> ! /* Currently we look only at the immediately defining statement
> ! to make this determination, since recursion on defining
> ! statements of operands can lead to quadratic behavior in the
> ! worst case. This is expected to catch almost all occurrences
> ! in practice. It would be possible to implement limited-depth
> ! recursion if important cases are lost. Alternatively, passes
> ! that need this information (such as the pow/powi lowering code
> ! in the cse_sincos pass) could be revised to provide it through
> dataflow propagation. */
>
> def_stmt = SSA_NAME_DEF_STMT (val);
>
> if (is_gimple_assign (def_stmt))
> {
> ! tree op0, op1;
> !
> ! /* See fold-const.c:tree_expr_nonnegative_p for additional
> ! cases that could be handled with recursion. */
> !
> ! switch (gimple_assign_rhs_code (def_stmt))
> {
> ! case ABS_EXPR:
> ! /* Always true for floating-point operands. */
> ! return true;
> !
> ! case MULT_EXPR:
> ! /* True if the two operands are identical (since we are
> ! restricted to floating-point inputs). */
> ! op0 = gimple_assign_rhs1 (def_stmt);
> ! op1 = gimple_assign_rhs2 (def_stmt);
> !
> ! if (op0 == op1
> ! || operand_equal_p (op0, op1, 0))
> ! return true;
>
> default:
> ! return false;
> }
> }
> else if (is_gimple_call (def_stmt))
> {
> tree fndecl = gimple_call_fndecl (def_stmt);
> ! if (fndecl
> ! && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> {
> ! tree arg1;
> !
> ! switch (DECL_FUNCTION_CODE (fndecl))
> ! {
> ! CASE_FLT_FN (BUILT_IN_ACOS):
> ! CASE_FLT_FN (BUILT_IN_ACOSH):
> ! CASE_FLT_FN (BUILT_IN_CABS):
> ! CASE_FLT_FN (BUILT_IN_COSH):
> ! CASE_FLT_FN (BUILT_IN_ERFC):
> ! CASE_FLT_FN (BUILT_IN_EXP):
> ! CASE_FLT_FN (BUILT_IN_EXP10):
> ! CASE_FLT_FN (BUILT_IN_EXP2):
> ! CASE_FLT_FN (BUILT_IN_FABS):
> ! CASE_FLT_FN (BUILT_IN_FDIM):
> ! CASE_FLT_FN (BUILT_IN_HYPOT):
> ! CASE_FLT_FN (BUILT_IN_POW10):
> ! return true;
> !
> ! CASE_FLT_FN (BUILT_IN_SQRT):
> ! /* sqrt(-0.0) is -0.0, and sqrt is not defined over other
> ! nonnegative inputs. */
> ! if (!HONOR_SIGNED_ZEROS (val))
> ! return true;
> !
> ! break;
> !
> ! CASE_FLT_FN (BUILT_IN_POWI):
> ! /* True if the second argument is an even integer. */
> ! arg1 = gimple_call_arg (def_stmt, 1);
> !
> ! if (TREE_CODE (arg1) == INTEGER_CST
> ! && (TREE_INT_CST_LOW (arg1) & 1) == 0)
> ! return true;
> !
> ! break;
> !
> ! CASE_FLT_FN (BUILT_IN_POW):
> ! /* True if the second argument is an even integer-valued
> ! real. */
> ! arg1 = gimple_call_arg (def_stmt, 1);
> !
> ! if (TREE_CODE (arg1) == REAL_CST)
> ! {
> ! REAL_VALUE_TYPE c;
> ! HOST_WIDE_INT n;
> !
> ! c = TREE_REAL_CST (arg1);
> ! n = real_to_integer (&c);
> !
> ! if ((n & 1) == 0)
> ! {
> ! REAL_VALUE_TYPE cint;
> ! real_from_integer (&cint, VOIDmode, n, SIGNED);
> ! if (real_identical (&c, &cint))
> ! return true;
> ! }
> ! }
> !
> ! break;
> !
> ! default:
> ! return false;
> ! }
> }
> }
>
> return false;
> }
>
> /* Given a pointer value OP0, return a simplified version of an
> --- 5787,5871 ----
> if (TREE_CODE (val) != SSA_NAME)
> return false;
>
> ! /* Currently we allow one level of recursion for conversions
> ! but look only at the immediately defining statement otherwise,
> ! since recursion on defining statements of operands can lead to
> ! quadratic behavior in the worst case. This is expected to catch
> ! almost all occurrences in practice. It would be possible to
> ! implement more recursion if important cases are lost. Alternatively,
> ! passes that need this information (such as the pow/powi lowering
> ! code in the cse_sincos pass) could be revised to provide it through
> dataflow propagation. */
> + #define RECURSE(X) (depth == 0 && gimple_val_nonnegative_p (X, depth + 1))
>
> def_stmt = SSA_NAME_DEF_STMT (val);
>
> if (is_gimple_assign (def_stmt))
> {
> ! enum tree_code code = gimple_assign_rhs_code (def_stmt);
> ! tree lhs_type = TREE_TYPE (gimple_assign_lhs (def_stmt));
> ! if (CONVERT_EXPR_CODE_P (code))
> {
> ! tree rhs = gimple_assign_rhs1 (def_stmt);
> ! tree rhs_type = TREE_TYPE (rhs);
> ! if (TREE_CODE (lhs_type) == REAL_TYPE)
> ! {
> ! if (TREE_CODE (rhs_type) == REAL_TYPE)
> ! return RECURSE (rhs);
> ! if (INTEGRAL_TYPE_P (rhs_type))
> ! {
> ! if (TYPE_UNSIGNED (rhs_type))
> ! return true;
> ! return RECURSE (rhs);
> ! }
> ! }
> ! else if (INTEGRAL_TYPE_P (lhs_type))
> ! {
> ! if (TREE_CODE (rhs_type) == REAL_TYPE)
> ! return RECURSE (rhs);
> ! if (INTEGRAL_TYPE_P (rhs_type))
> ! return (TYPE_PRECISION (rhs_type) < TYPE_PRECISION (lhs_type)
> ! && TYPE_UNSIGNED (rhs_type));
> ! }
> ! }
> ! bool strict_overflow_p = false;
> ! switch (TREE_CODE_CLASS (code))
> ! {
> ! case tcc_binary:
> ! case tcc_comparison:
> ! return tree_binary_nonnegative_warnv_p
> ! (code, lhs_type,
> ! gimple_assign_rhs1 (def_stmt),
> ! gimple_assign_rhs2 (def_stmt),
> ! &strict_overflow_p);
> !
> ! case tcc_unary:
> ! return tree_unary_nonnegative_warnv_p
> ! (code, lhs_type,
> ! gimple_assign_rhs1 (def_stmt),
> ! &strict_overflow_p);
>
> default:
> ! break;
> }
> }
> else if (is_gimple_call (def_stmt))
> {
> tree fndecl = gimple_call_fndecl (def_stmt);
> ! if (fndecl)
> {
> ! bool strict_overflow_p = false;
> ! unsigned int nargs = gimple_call_num_args (def_stmt);
> ! tree type = TREE_TYPE (gimple_call_lhs (def_stmt));
> ! tree arg0 = nargs > 0 ? gimple_call_arg (def_stmt, 0) : NULL_TREE;
> ! tree arg1 = nargs > 1 ? gimple_call_arg (def_stmt, 1) : NULL_TREE;
> ! return tree_call_nonnegative_warnv_p (type, fndecl, arg0, arg1,
> ! &strict_overflow_p);
> }
> }
>
> return false;
> + #undef RECURSE
> }
>
> /* Given a pointer value OP0, return a simplified version of an
> Index: a/gcc/gimple-fold.h
> ===================================================================
> *** a/gcc/gimple-fold.h
> --- b/gcc/gimple-fold.h
> *************** extern tree gimple_get_virt_method_for_binfo (HOST_WIDE_INT,
> tree,
> *** 48,54 ****
> extern tree gimple_get_virt_method_for_vtable (HOST_WIDE_INT, tree,
> unsigned HOST_WIDE_INT,
> bool *can_refer = NULL);
> ! extern bool gimple_val_nonnegative_real_p (tree);
> extern tree gimple_fold_indirect_ref (tree);
> extern bool arith_code_with_undefined_signed_overflow (tree_code);
> extern gimple_seq rewrite_to_defined_overflow (gimple *);
> --- 48,54 ----
> extern tree gimple_get_virt_method_for_vtable (HOST_WIDE_INT, tree,
> unsigned HOST_WIDE_INT,
> bool *can_refer = NULL);
> ! extern bool gimple_val_nonnegative_p (tree, unsigned int = 0);
> extern tree gimple_fold_indirect_ref (tree);
> extern bool arith_code_with_undefined_signed_overflow (tree_code);
> extern gimple_seq rewrite_to_defined_overflow (gimple *);
> Index: a/gcc/match.pd
> ===================================================================
> *** a/gcc/match.pd
> --- b/gcc/match.pd
> *************** along with GCC; see the file COPYING3. If not see
> *** 34,39 ****
> --- 34,45 ----
> integer_pow2p
> HONOR_NANS)
>
> + (match nonnegative_p
> + @0
> + (if (GIMPLE
> + ? gimple_val_nonnegative_p (@0)
> + : tree_expr_nonnegative_p (@0))))
> +
> /* Operator lists. */
> (define_operator_list tcc_comparison
> lt le eq ne ge gt unordered ordered unlt unle ungt unge uneq
> ltgt)
> *************** along with GCC; see the file COPYING3. If not see
> *** 512,518 ****
> (abs (negate @0))
> (abs @0))
> (simplify
> ! (abs tree_expr_nonnegative_p@0)
> @0)
>
> /* A few cases of fold-const.c negate_expr_p predicate. */
> --- 518,524 ----
> (abs (negate @0))
> (abs @0))
> (simplify
> ! (abs nonnegative_p@0)
> @0)
>
> /* A few cases of fold-const.c negate_expr_p predicate. */
> Index: a/gcc/tree-ssa-math-opts.c
> ===================================================================
> *** a/gcc/tree-ssa-math-opts.c
> --- b/gcc/tree-ssa-math-opts.c
> *************** gimple_expand_builtin_pow (gimple_stmt_iterator *gsi,
> location_t loc,
> *** 1522,1528 ****
>
> if (flag_unsafe_math_optimizations
> && cbrtfn
> ! && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
> && real_equal (&c, &dconst1_3))
> return build_and_insert_call (gsi, loc, cbrtfn, arg0);
>
> --- 1522,1528 ----
>
> if (flag_unsafe_math_optimizations
> && cbrtfn
> ! && (gimple_val_nonnegative_p (arg0) || !HONOR_NANS (mode))
> && real_equal (&c, &dconst1_3))
> return build_and_insert_call (gsi, loc, cbrtfn, arg0);
>
> *************** gimple_expand_builtin_pow (gimple_stmt_iterator *gsi,
> location_t loc,
> *** 1534,1540 ****
> if (flag_unsafe_math_optimizations
> && sqrtfn
> && cbrtfn
> ! && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
> && speed_p
> && hw_sqrt_exists
> && real_equal (&c, &dconst1_6))
> --- 1534,1540 ----
> if (flag_unsafe_math_optimizations
> && sqrtfn
> && cbrtfn
> ! && (gimple_val_nonnegative_p (arg0) || !HONOR_NANS (mode))
> && speed_p
> && hw_sqrt_exists
> && real_equal (&c, &dconst1_6))
> *************** gimple_expand_builtin_pow (gimple_stmt_iterator *gsi,
> location_t loc,
> *** 1589,1595 ****
>
> if (flag_unsafe_math_optimizations
> && cbrtfn
> ! && (gimple_val_nonnegative_real_p (arg0) || !HONOR_NANS (mode))
> && real_identical (&c2, &c)
> && !c2_is_int
> && optimize_function_for_speed_p (cfun)
> --- 1589,1595 ----
>
> if (flag_unsafe_math_optimizations
> && cbrtfn
> ! && (gimple_val_nonnegative_p (arg0) || !HONOR_NANS (mode))
> && real_identical (&c2, &c)
> && !c2_is_int
> && optimize_function_for_speed_p (cfun)
>
