[patch tree-optimization]: Fix for PR/49806
Richard Guenther
richard.guenther@gmail.com
Mon Aug 1 12:16:00 GMT 2011
On Fri, Jul 29, 2011 at 2:07 PM, Kai Tietz <ktietz@redhat.com> wrote:
> Hello,
>
> this patch fixes regression of bug report PR middle-end/49806, which was caused due unhandled type-cast patterns reasoned by boolification of compares and type-cast preserving from/to boolean types.
>
>
> ChangeLog
>
> 2011-07-29 Kai Tietz <ktietz@redhat.com>
>
> PR middle-end/49806
> * tree-vrp.c (has_operand_boolean_range): Helper function.
> (simplify_truth_ops_using_ranges): Factored out code pattern
> into new has_operand_boolean_range function and use it.
> (simplify_converted_bool_expr_using_ranges): New function.
> (simplify_stmt_using_ranges): Add new simplification function
> call.
>
> * gcc.dg/tree-ssa/vrp47.c: Remove dom-dump and adjusted
> scan test for vrp result.
>
> Bootstrapped and regression tested for all languages (+ Ada, Obj-C++) on host x86_64-pc-linux-gnu. Ok for apply?
>
> Regards,
> Kai
>
> Index: gcc-head/gcc/tree-vrp.c
> ===================================================================
> --- gcc-head.orig/gcc/tree-vrp.c
> +++ gcc-head/gcc/tree-vrp.c
> @@ -6747,15 +6747,46 @@ varying:
> return SSA_PROP_VARYING;
> }
>
> +/* Returns true, if operand OP has either a one-bit type precision,
> + or if value-range of OP is between zero and one. Otherwise false
> + is returned. The destination of PSOP will be set to true, if a sign-
> + overflow on range-check occures. PSOP might be NULL. */
> +static bool
> +has_operand_boolean_range (tree op, bool *psop)
> +{
> + tree val = NULL;
> + value_range_t *vr;
> + bool sop = false;
> +
> + if (TYPE_PRECISION (TREE_TYPE (op)) == 1)
> + {
> + if (psop)
> + *psop = false;
> + return true;
> + }
> + if (TREE_CODE (op) != SSA_NAME)
> + return false;
> + vr = get_value_range (op);
> +
> + val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
> + if (!val || !integer_onep (val))
> + return false;
> +
> + val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
> + if (!val || !integer_onep (val))
> + return false;
There is a much simpler and cheaper way to detect these cases.
return vr->type == VR_RANGE
&&integer_zerop (vr->min) && integer_onep (vr->max);
and all the strict-overflow stuff with *psop is not necessary.
> + if (psop)
> + *psop = sop;
> + return true;
> +}
> +
> /* Simplify boolean operations if the source is known
> to be already a boolean. */
> static bool
> simplify_truth_ops_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
> {
> enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
> - tree val = NULL;
> tree op0, op1;
> - value_range_t *vr;
> bool sop = false;
> bool need_conversion;
>
> @@ -6763,20 +6794,8 @@ simplify_truth_ops_using_ranges (gimple_
> gcc_assert (rhs_code == EQ_EXPR || rhs_code == NE_EXPR);
>
> op0 = gimple_assign_rhs1 (stmt);
> - if (TYPE_PRECISION (TREE_TYPE (op0)) != 1)
> - {
> - if (TREE_CODE (op0) != SSA_NAME)
> - return false;
> - vr = get_value_range (op0);
> -
> - val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
> - if (!val || !integer_onep (val))
> - return false;
> -
> - val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
> - if (!val || !integer_onep (val))
> - return false;
> - }
> + if (!has_operand_boolean_range (op0, &sop))
sounds backward. We have op_with_constant_singledon_value_range,
so why not op_with_boolean_range_p instead?
> + return false;
>
> op1 = gimple_assign_rhs2 (stmt);
>
> @@ -6802,17 +6821,8 @@ simplify_truth_ops_using_ranges (gimple_
> if (rhs_code == EQ_EXPR)
> return false;
>
> - if (TYPE_PRECISION (TREE_TYPE (op1)) != 1)
> - {
> - vr = get_value_range (op1);
> - val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
> - if (!val || !integer_onep (val))
> - return false;
> -
> - val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
> - if (!val || !integer_onep (val))
> - return false;
> - }
> + if (!has_operand_boolean_range (op1, &sop))
> + return false;
> }
>
> if (sop && issue_strict_overflow_warning (WARN_STRICT_OVERFLOW_MISC))
> @@ -7320,6 +7330,126 @@ simplify_switch_using_ranges (gimple stm
> return false;
> }
>
> +/* Simplify an integeral boolean-typed casted expression for the
> + following cases:
> + 1) (type) ~ (bool) op1 -> op1 ^ 1
> + 2) (type) ((bool)op1[0..1] & (bool)op2[0..1]) -> op1 & op2
> + 3) (type) ((bool)op1[0..1] | (bool)op2[0..1]) -> op1 | op2
> + 4) (type) ((bool)op1[0..1] ^ (bool)op2[0..1]) -> op2 ^ op2
> + 5) (type) (val[0..1] == 0) -> val ^ 1
> + 6) (type) (val[0..1] != 0) -> val
2) to 4) don't look in any way special for 'bool' but should treat all
kinds of intermediate types.
The description does suggest that (type) ~(bool) op1 -> op1 ^ 1 is
always valid - it is not, unless op1 has a (sub-)range of [0..1].
> +
> + Assuming op1 and op2 hav\EDng type TYPE. */
> +
> +static bool
> +simplify_converted_bool_expr_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
> +{
> + tree finaltype, expr, op1, op2 = NULL_TREE;
> + gimple def;
> + enum tree_code expr_code;
> +
> + finaltype = TREE_TYPE (gimple_assign_lhs (stmt));
> + if (!INTEGRAL_TYPE_P (finaltype))
> + return false;
> + expr = gimple_assign_rhs1 (stmt);
> +
> + /* Check that cast is from a boolean-typed expression. */
> + if (TREE_CODE (TREE_TYPE (expr)) != BOOLEAN_TYPE)
> + return false;
No, you should use TYPE_PRECISION (...) == 1 here.
> + /* Check for assignment. */
That doesn't provide information that isn't trivially available. Please
instead write down the stmt patterns you match using the local
variables you end up using.
> + def = SSA_NAME_DEF_STMT (expr);
> + if (!is_gimple_assign (def))
> + return false;
> +
> + expr_code = gimple_assign_rhs_code (def);
> +
> + op1 = gimple_assign_rhs1 (def);
> +
excess vertical space.
> + switch (expr_code)
> + {
> + /* (TYPE) ~ (bool) X -> X ^ 1, if X has compatible type to final type
> + and truth-valued range. */
> + case BIT_NOT_EXPR:
> + /* Bitwise not is only a logical-not for 1-bit precisioned
> + types. */
> + if (TYPE_PRECISION (TREE_TYPE (expr)) != 1)
> + return false;
> +
> + /* Check that we have a type-conversion as operand of bitwise-not. */
> + def = SSA_NAME_DEF_STMT (op1);
> + if (!is_gimple_assign (def)
> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def)))
> + return false;
> + op1 = gimple_assign_rhs1 (def);
> + /* Has X compatible type to final type and truth-valued range? */
> + if (!useless_type_conversion_p (finaltype, TREE_TYPE (op1))
> + || !has_operand_boolean_range (op1, NULL))
> + return false;
> + expr_code = BIT_XOR_EXPR;
> + op2 = fold_convert (finaltype, integer_one_node);
build_one_cst (finaltype)
> + break;
> +
> + /* (TYPE) ((bool) X op (bool) Y) -> X op Y, if X and Y have compatible type
> + TYPE and having truth-valued ranges. */
> + case BIT_AND_EXPR:
> + case BIT_IOR_EXPR:
> + case BIT_XOR_EXPR:
See above - I think restricting these transformation to boolean ranges is
not necessary. In fact, see the patch(es) I posted as RFC.
> + op2 = gimple_assign_rhs2 (def);
> +
> + def = SSA_NAME_DEF_STMT (op1);
> + if (!is_gimple_assign (def)
> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def)))
> + return false;
> + op1 = gimple_assign_rhs1 (def);
> + /* Has X compatible type to final type and truth-valued range? */
> + if (!useless_type_conversion_p (finaltype, TREE_TYPE (op1))
> + || !has_operand_boolean_range (op1, NULL))
> + return false;
> +
> + def = SSA_NAME_DEF_STMT (op2);
> + if (!is_gimple_assign (def)
> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def)))
> + return false;
> + op2 = gimple_assign_rhs1 (def);
> + /* Has Y compatible type to final type and truth-valued range? */
> + if (!useless_type_conversion_p (finaltype, TREE_TYPE (op2))
> + || !has_operand_boolean_range (op2, NULL))
> + return false;
> + break;
> +
> + /* If X has compatible type to final type and has truth-valued range,
> + tranform:
> + (TYPE) (X == 0) -> X ^ 1
> + (TYPE) (X != 0) -> X */
> + case EQ_EXPR:
> + case NE_EXPR:
> + /* Is the comparison's second operand zero? */
> + op2 = gimple_assign_rhs2 (def);
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (op1))
> + || !integer_zerop (op2))
> + return false;
> + /* Is the type of comparison's first argument compatible to final type
> + and has it truth-valued range? */
> + if (!useless_type_conversion_p (finaltype, TREE_TYPE (op1))
> + || !has_operand_boolean_range (op1, NULL))
> + return false;
> +
> + op2 = NULL_TREE;
> + /* X == 0 -> X ^ 1. */
> + if (expr_code == EQ_EXPR)
> + op2 = fold_convert (finaltype, integer_one_node);
build_one_cst
> + expr_code = (!op2 ? SSA_NAME : BIT_XOR_EXPR);
!op2 ? TREE_CODE (op1) : BIT_XOR_EXPR
What about (T) x == 1? For truthvalue x that is x as well. (T) x != 1
is x ^ 1, too.
Btw, why doesn't this get handled in two steps already, first
changing the comparisons into the respective bit operation and then
eliminating the conversion? The first step should be handled by
simplify_truth_ops_using_ranges already, no? So doesn't this just
introduce duplicate code here?
Thanks,
Richard.
> + break;
> +
> + default:
> + return false;
> + }
> +
> + gimple_assign_set_rhs_with_ops (gsi, expr_code, op1, op2);
> + update_stmt (gsi_stmt (*gsi));
> + return true;
> +}
> +
> /* Simplify an integral conversion from an SSA name in STMT. */
>
> static bool
> @@ -7546,7 +7676,11 @@ simplify_stmt_using_ranges (gimple_stmt_
> CASE_CONVERT:
> if (TREE_CODE (rhs1) == SSA_NAME
> && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> - return simplify_conversion_using_ranges (stmt);
> + {
> + if (simplify_conversion_using_ranges (stmt)
> + || simplify_converted_bool_expr_using_ranges (gsi, stmt))
> + return true;
> + }
> break;
>
> case FLOAT_EXPR:
> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
> @@ -4,8 +4,8 @@
> jumps when evaluating an && condition. VRP is not able to optimize
> this. */
> /* { dg-do compile { target { ! "mips*-*-* s390*-*-* avr-*-* mn10300-*-*" } } } */
> -/* { dg-options "-O2 -fdump-tree-vrp -fdump-tree-dom" } */
> -/* { dg-options "-O2 -fdump-tree-vrp -fdump-tree-dom -march=i586" { target { i?86-*-* && ilp32 } } } */
> +/* { dg-options "-O2 -fdump-tree-vrp" } */
> +/* { dg-options "-O2 -fdump-tree-vrp -march=i586" { target { i?86-*-* && ilp32 } } } */
>
> int h(int x, int y)
> {
> @@ -36,13 +36,10 @@ int f(int x)
> 0 or 1. */
> /* { dg-final { scan-tree-dump-times "\[xy\]\[^ \]* !=" 0 "vrp1" } } */
>
> -/* This one needs more copy propagation that only happens in dom1. */
> -/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "dom1" } } */
> -/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "vrp1" { xfail *-*-* } } } */
> +/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "vrp1" } } */
>
> /* These two are fully simplified by VRP. */
> /* { dg-final { scan-tree-dump-times "x\[^ \]* \[|\] y" 1 "vrp1" } } */
> /* { dg-final { scan-tree-dump-times "x\[^ \]* \\^ 1" 1 "vrp1" } } */
>
> /* { dg-final { cleanup-tree-dump "vrp\[0-9\]" } } */
> -/* { dg-final { cleanup-tree-dump "dom\[0-9\]" } } */
>
More information about the Gcc-patches
mailing list