On Sat, Apr 6, 2013 at 2:48 PM, Jeff Law <l...@redhat.com> wrote:
>
> Given something like this:
>
> <bb 6>:
> _23 = changed_17 ^ 1;
> _12 = (_Bool) _23;
> if (_12 != 0)
> goto <bb 10>;
> else
> goto <bb 7>;
>
> Assume _23 and changed_17 have integer types wider than a boolean, but VRP
> has determined they have a range [0..1].
>
> We should be turning that into:
>
> <bb 6>:
> _23 = changed_17 ^ 1;
> _12 = (_Bool) _23;
> if (_23 != 0)
> goto <bb 10>;
> else
> goto <bb 7>;
>
> Note the change in the conditional. This also makes the statement
> _12 = (_Bool) _23 dead which should be eliminated by DCE.
>
> This kind of thing happens regularly in GCC itself and is fixed by the
> attached patch.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
>
> OK for the trunk?
>
>
> commit fd82eea6f208bb12646e0e0e307fb86f043c1649
> Author: Jeff Law <l...@redhat.com>
> Date: Sat Apr 6 06:46:58 2013 -0600
>
> * tree-vrp.c (simplify_cond_using_ranges): Simplify test of
> boolean
> when the boolean was created by converting a wider object which
> had a boolean range.
>
> * gcc.dg/tree-ssa/vrp87.c: New test
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 44797cc..d34ecde 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,5 +1,9 @@
> 2013-04-06 Jeff Law <l...@redhat.com>
>
> + * tree-vrp.c (simplify_cond_using_ranges): Simplify test of boolean
> + when the boolean was created by converting a wider object which
> + had a boolean range.
> +
> * gimple.c (canonicalize_cond_expr_cond): Rewrite x ^ y into
> x != y.
>
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 601ca66..6ed8af2 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,5 +1,7 @@
> 2013-04-06 Jeff Law <l...@redhat.com>
>
> + * gcc.dg/tree-ssa/vrp87.c: New test
> +
> * gcc.dg/tree-ssa/forwprop-25.c: New test
>
> 2013-04-03 Jeff Law <l...@redhat.com>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp87.c
> b/gcc/testsuite/gcc.dg/tree-ssa/vrp87.c
> new file mode 100644
> index 0000000..7feff81
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp87.c
> @@ -0,0 +1,81 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-vrp2-details -fdump-tree-cddce2-details" }
> */
> +
> +struct bitmap_head_def;
> +typedef struct bitmap_head_def *bitmap;
> +typedef const struct bitmap_head_def *const_bitmap;
> +
> +
> +typedef unsigned long BITMAP_WORD;
> +typedef struct bitmap_element_def
> +{
> + struct bitmap_element_def *next;
> + unsigned int indx;
> + BITMAP_WORD bits[((128 + (8 * 8 * 1u) - 1) / (8 * 8 * 1u))];
> +} bitmap_element;
> +
> +
> +
> +
> +
> +
> +typedef struct bitmap_head_def
> +{
> + bitmap_element *first;
> +
> +} bitmap_head;
> +
> +
> +
> +static __inline__ unsigned char
> +bitmap_elt_ior (bitmap dst, bitmap_element * dst_elt,
> + bitmap_element * dst_prev, const bitmap_element * a_elt,
> + const bitmap_element * b_elt, unsigned char changed)
> +{
> +
> + if (a_elt)
> + {
> +
> + if (!changed && dst_elt)
> + {
> + changed = 1;
> + }
> + }
> + else
> + {
> + changed = 1;
> + }
> + return changed;
> +}
> +
> +unsigned char
> +bitmap_ior_into (bitmap a, const_bitmap b)
> +{
> + bitmap_element *a_elt = a->first;
> + const bitmap_element *b_elt = b->first;
> + bitmap_element *a_prev = ((void *) 0);
> + unsigned char changed = 0;
> +
> + while (b_elt)
> + {
> +
> + if (!a_elt || a_elt->indx == b_elt->indx)
> + changed = bitmap_elt_ior (a, a_elt, a_prev, a_elt, b_elt, changed);
> + else if (a_elt->indx > b_elt->indx)
> + changed = 1;
> + b_elt = b_elt->next;
> +
> +
> + }
> +
> + return changed;
> +}
> +
> +/* Verify that VRP simplified an "if" statement. */
> +/* { dg-final { scan-tree-dump "Folded into: if.*" "vrp2"} } */
> +/* Verify that DCE after VRP2 eliminates a dead conversion
> + to a (Bool). */
> +/* { dg-final { scan-tree-dump "Deleting.*_Bool.*;" "cddce2"} } */
> +/* { dg-final { cleanup-tree-dump "vrp2" } } */
> +/* { dg-final { cleanup-tree-dump "cddce2" } } */
> +
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 250a506..d76cead 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -8584,6 +8584,43 @@ simplify_cond_using_ranges (gimple stmt)
> }
> }
>
> + /* If we have a comparison of a SSA_NAME boolean against
> + a constant (which obviously must be [0..1]). See if the
> + SSA_NAME was set by a type conversion where the source
> + of the conversion is another SSA_NAME with a range [0..1].
> +
> + If so, we can replace the SSA_NAME in the comparison with
> + the RHS of the conversion. This will often make the type
> + conversion dead code which DCE will clean up. */
> + if (TREE_CODE (op0) == SSA_NAME
> + && TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE
Use
(TREE_CODE (TREE_TYPE (op0)) == BOOLEAN_TYPE
|| (INTEGRAL_TYPE_P (TREE_TYPE (op0))
&& TYPE_PRECISION (TREE_TYPE (op0)) == 1))
to catch some more cases.
> + && is_gimple_min_invariant (op1))
In this case it's simpler to test TREE_CODE (op1) == INTEGER_CST.
> + {
> + gimple def_stmt = SSA_NAME_DEF_STMT (op0);
> + tree innerop;
> +
> + if (!is_gimple_assign (def_stmt)
> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
> + return false;
> +
> + innerop = gimple_assign_rhs1 (def_stmt);
> +
> + if (!SSA_NAME_OCCURS_IN_ABNORMAL_PHI (innerop))
As Steven said, the abnormal check is not necessary, but for completeness
you should check TREE_CODE (innerop) == SSA_NAME. Valid (but
unfolded) GIMPLE can have (_Bool) 1, too.
> + {
> + value_range_t *vr = get_value_range (innerop);
> +
> + if (vr->type == VR_RANGE
> + && operand_equal_p (vr->min, integer_zero_node, 0)
> + && operand_equal_p (vr->max, integer_one_node, 0))
> + {
> + tree newconst = fold_convert (TREE_TYPE (innerop), op1);
> + gimple_cond_set_lhs (stmt, innerop);
> + gimple_cond_set_rhs (stmt, newconst);
> + return true;
> + }
> + }
> + }
> +
> return false;
> }
Note that we already have code with similar functionality (see if a
conversion would alter the value of X) as part of optimizing
(T1)(T2)X to (T1)X in simplify_conversion_using_ranges. Maybe
a part of it can be split out and used to simplify conditions for
a bigger range of types than just compares against boolean 0/1.
Richard.
>
