Re: [patch tree-optimization]: Move tree-vrp to use binary instead of truth-expressions

[Kai Tietz <ktietz70 at googlemail dot com>]

2011/7/21 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, Jul 21, 2011 at 3:09 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> this patch changes TRUTH-expression patterns into BIT-expression ones
>> and adjusts code-flow
>> for this.
>>
>> ChangeLog gcc
>>
>> 2011-07-21 ?Kai Tietz ?<ktietz@redhat.com>
>>
>> ? ? ? ?* tree-vrp.c (extract_range_from_binary_expr): Convert
>> ? ? ? ?truth expression to bimary expression,
>> ? ? ? ?(extract_range_from_unary_expr): Likewise.
>> ? ? ? ?(extract_range_from_assignment): Likewise.
>> ? ? ? ?(build_assert_expr_for): Likewise.
>> ? ? ? ?(register_edge_assert_for_1): Likewise.
>> ? ? ? ?(simplify_truth_ops_using_ranges): Likewise.
>> ? ? ? ?(simplify_stmt_using_ranges): Likewise.
>>
>> ? ? ? ?do_dce flag to deside, if BB's statements are scanned
>> ? ? ? ?in last to first, or first to last order.
>>
>> Bootstrapped and regression tested for all standard languages
>> (including Ada and Obj-C++) on
>> host x86_64-pc-linux-gnu. ?Ok for apply?
>
> *sigh*, you didn't address any of my comments.
>
> Again ...
>
>>
>> Regards,
>> Kai
>>
>> Index: gcc-head/gcc/tree-vrp.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-vrp.c
>> +++ gcc-head/gcc/tree-vrp.c
>> @@ -2172,8 +2172,6 @@ extract_range_from_binary_expr (value_ra
>> ? ? ? && code != MAX_EXPR
>> ? ? ? && code != BIT_AND_EXPR
>> - ? ? ?&& code != BIT_IOR_EXPR
>> - ? ? ?&& code != TRUTH_AND_EXPR
>> - ? ? ?&& code != TRUTH_OR_EXPR)
>> + ? ? ?&& code != BIT_IOR_EXPR)
>> ? ? {
>> ? ? ? /* We can still do constant propagation here. ?*/
>> ? ? ? tree const_op0 = op_with_constant_singleton_value_range (op0);
>> @@ -2228,8 +2227,7 @@ extract_range_from_binary_expr (value_ra
>> ? ? ?divisions. ?TODO, we may be able to derive anti-ranges in
>> ? ? ?some cases. ?*/
>> ? if (code != BIT_AND_EXPR
>> - ? ? ?&& code != TRUTH_AND_EXPR
>> - ? ? ?&& code != TRUTH_OR_EXPR
>> + ? ? ?&& code != BIT_IOR_EXPR
>> ? ? ? && code != TRUNC_DIV_EXPR
>> ? ? ? && code != FLOOR_DIV_EXPR
>> ? ? ? && code != CEIL_DIV_EXPR
>> @@ -2251,7 +2249,10 @@ extract_range_from_binary_expr (value_ra
>> ? ? ? || POINTER_TYPE_P (TREE_TYPE (op0))
>> ? ? ? || POINTER_TYPE_P (TREE_TYPE (op1)))
>> ? ? {
>> - ? ? ?if (code == MIN_EXPR || code == MAX_EXPR)
>> + ? ? ?/* We need to preserve here bitwise-or for pointer types. ?*/
>
> Preserve? ?I asked how you end up seeing BIT_IOR_EXPR here, you
> didn't answer that yet.

Well, we see that here in case of truth.  So we need to allow it at
top, and indeed I noticed that we have IOR patterns on address-typed
expressions, so I added here default handling for it, as otherwise we
would run into the gcc_unreachable for it.

>> + ? ? ?if (code == BIT_IOR_EXPR)
>> + ? ? ? ?set_value_range_to_varying (vr);
>> + ? ? ?else if (code == MIN_EXPR || code == MAX_EXPR)
>> ? ? ? ?{
>> ? ? ? ? ?/* For MIN/MAX expressions with pointers, we only care about
>> ? ? ? ? ? ? nullness, if both are non null, then the result is nonnull.
>> @@ -2296,57 +2297,8 @@ extract_range_from_binary_expr (value_ra
>>
>> ? /* For integer ranges, apply the operation to each end of the
>> ? ? ?range and see what we end up with. ?*/
>> - ?if (code == TRUTH_AND_EXPR
>> - ? ? ?|| code == TRUTH_OR_EXPR)
>> - ? ?{
>> - ? ? ?/* If one of the operands is zero, we know that the whole
>> - ? ? ? ?expression evaluates zero. ?*/
>> - ? ? ?if (code == TRUTH_AND_EXPR
>> - ? ? ? ? && ((vr0.type == VR_RANGE
>> - ? ? ? ? ? ? ?&& integer_zerop (vr0.min)
>> - ? ? ? ? ? ? ?&& integer_zerop (vr0.max))
>> - ? ? ? ? ? ? || (vr1.type == VR_RANGE
>> - ? ? ? ? ? ? ? ? && integer_zerop (vr1.min)
>> - ? ? ? ? ? ? ? ? && integer_zerop (vr1.max))))
>> - ? ? ? {
>> - ? ? ? ? type = VR_RANGE;
>> - ? ? ? ? min = max = build_int_cst (expr_type, 0);
>> - ? ? ? }
>> - ? ? ?/* If one of the operands is one, we know that the whole
>> - ? ? ? ?expression evaluates one. ?*/
>> - ? ? ?else if (code == TRUTH_OR_EXPR
>> - ? ? ? ? ? ? ?&& ((vr0.type == VR_RANGE
>> - ? ? ? ? ? ? ? ? ? && integer_onep (vr0.min)
>> - ? ? ? ? ? ? ? ? ? && integer_onep (vr0.max))
>> - ? ? ? ? ? ? ? ? ?|| (vr1.type == VR_RANGE
>> - ? ? ? ? ? ? ? ? ? ? ?&& integer_onep (vr1.min)
>> - ? ? ? ? ? ? ? ? ? ? ?&& integer_onep (vr1.max))))
>> - ? ? ? {
>> - ? ? ? ? type = VR_RANGE;
>> - ? ? ? ? min = max = build_int_cst (expr_type, 1);
>> - ? ? ? }
>> - ? ? ?else if (vr0.type != VR_VARYING
>> - ? ? ? ? ? ? ?&& vr1.type != VR_VARYING
>> - ? ? ? ? ? ? ?&& vr0.type == vr1.type
>> - ? ? ? ? ? ? ?&& !symbolic_range_p (&vr0)
>> - ? ? ? ? ? ? ?&& !overflow_infinity_range_p (&vr0)
>> - ? ? ? ? ? ? ?&& !symbolic_range_p (&vr1)
>> - ? ? ? ? ? ? ?&& !overflow_infinity_range_p (&vr1))
>> - ? ? ? {
>> - ? ? ? ? /* Boolean expressions cannot be folded with int_const_binop. ?*/
>> - ? ? ? ? min = fold_binary (code, expr_type, vr0.min, vr1.min);
>> - ? ? ? ? max = fold_binary (code, expr_type, vr0.max, vr1.max);
>> - ? ? ? }
>> - ? ? ?else
>> - ? ? ? {
>> - ? ? ? ? /* The result of a TRUTH_*_EXPR is always true or false. ?*/
>> - ? ? ? ? set_value_range_to_truthvalue (vr, expr_type);
>> - ? ? ? ? return;
>> - ? ? ? }
>> - ? ?}
>> - ?else if (code == PLUS_EXPR
>> - ? ? ? ? ?|| code == MIN_EXPR
>> - ? ? ? ? ?|| code == MAX_EXPR)
>
> The above hunk looks good, removing dead code, but ...
>
>> + ?if (code == PLUS_EXPR || code == MIN_EXPR
>> + ? ? ?|| code == MAX_EXPR)
>> ? ? {
>> ? ? ? /* If we have a PLUS_EXPR with two VR_ANTI_RANGEs, drop to
>> ? ? ? ? VR_VARYING. ?It would take more effort to compute a precise
>> @@ -2679,73 +2631,127 @@ extract_range_from_binary_expr (value_ra
>> ? ? ? double_int may_be_nonzero0, may_be_nonzero1;
>> ? ? ? double_int must_be_nonzero0, must_be_nonzero1;
>>
>> - ? ? ?vr0_int_cst_singleton_p = range_int_cst_singleton_p (&vr0);
>> - ? ? ?vr1_int_cst_singleton_p = range_int_cst_singleton_p (&vr1);
>> - ? ? ?int_cst_range0 = zero_nonzero_bits_from_vr (&vr0, &may_be_nonzero0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &must_be_nonzero0);
>> - ? ? ?int_cst_range1 = zero_nonzero_bits_from_vr (&vr1, &may_be_nonzero1,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &must_be_nonzero1);
>> -
>> - ? ? ?type = VR_RANGE;
>> - ? ? ?if (vr0_int_cst_singleton_p && vr1_int_cst_singleton_p)
>> - ? ? ? min = max = int_const_binop (code, vr0.max, vr1.max);
>> - ? ? ?else if (!int_cst_range0 && !int_cst_range1)
>> + ? ? ?/* If one of the operands is zero, we know that the whole
>> + ? ? ? ?expression evaluates zero. ?*/
>
> ... starting here it get's odd. ?You are 1:1 replacing the code here.
> Don't do that.
>
> Instead do nothing here in this patch.
>
>> + ? ? ?if (code == BIT_AND_EXPR
>> + ? ? ? ? && ((vr0.type == VR_RANGE
>> + ? ? ? ? ? ? ?&& integer_zerop (vr0.min)
>> + ? ? ? ? ? ? ?&& integer_zerop (vr0.max))
>> + ? ? ? ? ? ? || (vr1.type == VR_RANGE
>> + ? ? ? ? ? ? ? ? && integer_zerop (vr1.min)
>> + ? ? ? ? ? ? ? ? && integer_zerop (vr1.max))))
>> + ? ? ? {
>> + ? ? ? ? type = VR_RANGE;
>> + ? ? ? ? min = max = build_int_cst (expr_type, 0);
>> + ? ? ? }
>> + ? ? ?/* If one of the operands has all bits set to one, we know
>> + ? ? ? ? that the whole expression evaluates to this one. ?*/
>> + ? ? ?else if (code == BIT_IOR_EXPR
>> + ? ? ? ? ? ? ?&& (vr0.type == VR_RANGE
>> + ? ? ? ? ? ? ? ? ?&& integer_all_onesp (vr0.min)
>> + ? ? ? ? ? ? ? ? ?&& integer_all_onesp (vr0.max)))
>> + ? ? ? {
>> + ? ? ? ? type = VR_RANGE;
>> + ? ? ? ? min = max = fold_convert (expr_type, vr0.min);
>> + ? ? ? }
>> + ? ? ?else if (code == BIT_IOR_EXPR
>> + ? ? ? ? ? ? ?&& (vr1.type == VR_RANGE
>> + ? ? ? ? ? ? ? ? ?&& integer_all_onesp (vr1.min)
>> + ? ? ? ? ? ? ? ? ?&& integer_all_onesp (vr1.max)))
>> ? ? ? ?{
>> - ? ? ? ? set_value_range_to_varying (vr);
>> - ? ? ? ? return;
>> + ? ? ? ? type = VR_RANGE;
>> + ? ? ? ? min = max = fold_convert (expr_type, vr1.min);
>> ? ? ? ?}
>> - ? ? ?else if (code == BIT_AND_EXPR)
>> + ? ? ?else if (TYPE_PRECISION (TREE_TYPE (op1)) == 1)
>> ? ? ? ?{
>> - ? ? ? ? min = double_int_to_tree (expr_type,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_and (must_be_nonzero0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? must_be_nonzero1));
>> - ? ? ? ? max = double_int_to_tree (expr_type,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_and (may_be_nonzero0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? may_be_nonzero1));
>> - ? ? ? ? if (TREE_OVERFLOW (min) || tree_int_cst_sgn (min) < 0)
>> - ? ? ? ? ? min = NULL_TREE;
>> - ? ? ? ? if (TREE_OVERFLOW (max) || tree_int_cst_sgn (max) < 0)
>> - ? ? ? ? ? max = NULL_TREE;
>> - ? ? ? ? if (int_cst_range0 && tree_int_cst_sgn (vr0.min) >= 0)
>> - ? ? ? ? ? {
>> - ? ? ? ? ? ? if (min == NULL_TREE)
>> - ? ? ? ? ? ? ? min = build_int_cst (expr_type, 0);
>> - ? ? ? ? ? ? if (max == NULL_TREE || tree_int_cst_lt (vr0.max, max))
>> - ? ? ? ? ? ? ? max = vr0.max;
>> + ? ? ? ? if (vr0.type != VR_VARYING
>> + ? ? ? ? ? ? ? ? ?&& vr1.type != VR_VARYING
>> + ? ? ? ? ? ? ? ? ?&& vr0.type == vr1.type
>> + ? ? ? ? ? ? ? ? ?&& !symbolic_range_p (&vr0)
>> + ? ? ? ? ? ? ? ? ?&& !overflow_infinity_range_p (&vr0)
>> + ? ? ? ? ? ? ? ? ?&& !symbolic_range_p (&vr1)
>> + ? ? ? ? ? ? ? ? ?&& !overflow_infinity_range_p (&vr1))
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? /* Boolean expressions cannot be folded with int_const_binop. ?*/
>> + ? ? ? ? ? ? min = fold_binary (code, expr_type, vr0.min, vr1.min);
>> + ? ? ? ? ? ? max = fold_binary (code, expr_type, vr0.max, vr1.max);
>> + ? ? ? ? ? }

Right I kept this logic due the general code seem to have issues with
const_binop and Boolean expressions.  If you are sure this comment is
wrong, we might be able to remove here the complete hunks before and
just use old code for boolean-case, too.

>> + ? ? ? ? else
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? set_value_range_to_varying (vr);
>> + ? ? ? ? ? ? return;
>> ? ? ? ? ? ?}
>> - ? ? ? ? if (int_cst_range1 && tree_int_cst_sgn (vr1.min) >= 0)
>> + ? ? ? }
>> + ? ? ? else
>> + ? ? ? ?{
>> + ? ? ? ? vr0_int_cst_singleton_p = range_int_cst_singleton_p (&vr0);
>> + ? ? ? ? vr1_int_cst_singleton_p = range_int_cst_singleton_p (&vr1);
>> + ? ? ? ? int_cst_range0 = zero_nonzero_bits_from_vr (&vr0, &may_be_nonzero0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &must_be_nonzero0);
>> + ? ? ? ? int_cst_range1 = zero_nonzero_bits_from_vr (&vr1, &may_be_nonzero1,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &must_be_nonzero1);
>> +
>> + ? ? ? ? type = VR_RANGE;
>> + ? ? ? ? if (vr0_int_cst_singleton_p && vr1_int_cst_singleton_p)
>> + ? ? ? ? ? min = max = int_const_binop (code, vr0.max, vr1.max);
>> + ? ? ? ? else if (!int_cst_range0 && !int_cst_range1)
>> ? ? ? ? ? ?{
>> - ? ? ? ? ? ? if (min == NULL_TREE)
>> - ? ? ? ? ? ? ? min = build_int_cst (expr_type, 0);
>> - ? ? ? ? ? ? if (max == NULL_TREE || tree_int_cst_lt (vr1.max, max))
>> - ? ? ? ? ? ? ? max = vr1.max;
>> + ? ? ? ? ? ? set_value_range_to_varying (vr);
>> + ? ? ? ? ? ? return;
>> + ? ? ? ? ? }
>> + ? ? ? ? else if (code == BIT_AND_EXPR)
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? min = double_int_to_tree (expr_type,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_and (must_be_nonzero0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? must_be_nonzero1));
>> + ? ? ? ? ? ? max = double_int_to_tree (expr_type,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_and (may_be_nonzero0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? may_be_nonzero1));
>> + ? ? ? ? ? ? if (TREE_OVERFLOW (min) || tree_int_cst_sgn (min) < 0)
>> + ? ? ? ? ? ? ? min = NULL_TREE;
>> + ? ? ? ? ? ? if (TREE_OVERFLOW (max) || tree_int_cst_sgn (max) < 0)
>> + ? ? ? ? ? ? ? max = NULL_TREE;
>> + ? ? ? ? ? ? if (int_cst_range0 && tree_int_cst_sgn (vr0.min) >= 0)
>> + ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? if (min == NULL_TREE)
>> + ? ? ? ? ? ? ? ? ? min = build_int_cst (expr_type, 0);
>> + ? ? ? ? ? ? ? ? if (max == NULL_TREE || tree_int_cst_lt (vr0.max, max))
>> + ? ? ? ? ? ? ? ? ? max = vr0.max;
>> + ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? if (int_cst_range1 && tree_int_cst_sgn (vr1.min) >= 0)
>> + ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? if (min == NULL_TREE)
>> + ? ? ? ? ? ? ? ? ? min = build_int_cst (expr_type, 0);
>> + ? ? ? ? ? ? ? ? if (max == NULL_TREE || tree_int_cst_lt (vr1.max, max))
>> + ? ? ? ? ? ? ? ? ? max = vr1.max;
>> + ? ? ? ? ? ? ? }
>> + ? ? ? ? ? }
>> + ? ? ? ? else if (!int_cst_range0
>> + ? ? ? ? ? ? ? ? ?|| !int_cst_range1
>> + ? ? ? ? ? ? ? ? ?|| tree_int_cst_sgn (vr0.min) < 0
>> + ? ? ? ? ? ? ? ? ?|| tree_int_cst_sgn (vr1.min) < 0)
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? set_value_range_to_varying (vr);
>> + ? ? ? ? ? ? return;
>> ? ? ? ? ? ?}
>> - ? ? ? }
>> - ? ? ?else if (!int_cst_range0
>> - ? ? ? ? ? ? ?|| !int_cst_range1
>> - ? ? ? ? ? ? ?|| tree_int_cst_sgn (vr0.min) < 0
>> - ? ? ? ? ? ? ?|| tree_int_cst_sgn (vr1.min) < 0)
>> - ? ? ? {
>> - ? ? ? ? set_value_range_to_varying (vr);
>> - ? ? ? ? return;
>> - ? ? ? }
>> - ? ? ?else
>> - ? ? ? {
>> - ? ? ? ? min = double_int_to_tree (expr_type,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_ior (must_be_nonzero0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? must_be_nonzero1));
>> - ? ? ? ? max = double_int_to_tree (expr_type,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_ior (may_be_nonzero0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? may_be_nonzero1));
>> - ? ? ? ? if (TREE_OVERFLOW (min) || tree_int_cst_sgn (min) < 0)
>> - ? ? ? ? ? min = vr0.min;
>> ? ? ? ? ?else
>> - ? ? ? ? ? min = vrp_int_const_binop (MAX_EXPR, min, vr0.min);
>> - ? ? ? ? if (TREE_OVERFLOW (max) || tree_int_cst_sgn (max) < 0)
>> - ? ? ? ? ? max = NULL_TREE;
>> - ? ? ? ? min = vrp_int_const_binop (MAX_EXPR, min, vr1.min);
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? min = double_int_to_tree (expr_type,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_ior (must_be_nonzero0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? must_be_nonzero1));
>> + ? ? ? ? ? ? max = double_int_to_tree (expr_type,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? double_int_ior (may_be_nonzero0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? may_be_nonzero1));
>> + ? ? ? ? ? ? if (TREE_OVERFLOW (min) || tree_int_cst_sgn (min) < 0)
>> + ? ? ? ? ? ? ? min = vr0.min;
>> + ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? min = vrp_int_const_binop (MAX_EXPR, min, vr0.min);
>> + ? ? ? ? ? ? if (TREE_OVERFLOW (max) || tree_int_cst_sgn (max) < 0)
>> + ? ? ? ? ? ? ? max = NULL_TREE;
>> + ? ? ? ? ? ? min = vrp_int_const_binop (MAX_EXPR, min, vr1.min);
>> + ? ? ? ? ? }
>> ? ? ? ?}
>> - ? ?}
>> + ? ? }
>> ? else
>> ? ? gcc_unreachable ();
>>
>> @@ -2806,7 +2812,7 @@ extract_range_from_unary_expr (value_ran
>> ? ? ?cannot easily determine a resulting range. ?*/
>> ? if (code == FIX_TRUNC_EXPR
>> ? ? ? || code == FLOAT_EXPR
>> - ? ? ?|| code == BIT_NOT_EXPR
>> + ? ? ?|| (code == BIT_NOT_EXPR && TYPE_PRECISION (type) != 1)
>
> Likewise - this looks unrelated to TRUTH_* code removal. ?And I asked you
> to instead properly handle BOT_NOT_EXPR completely (in a separate
> patch).

Well, I don't think it is unrealted for truth to binary transition,
but you are right that this check
is for now dead code (see later comment why it isn't, and why we will
see here BIT_XOR_EXPR see second part).  To disallow here BIT_XOR_EXPR
might not be a big deal, but we might loose here some
folding-opportunites in vrp, which gets then done by later-passes
anyway.

>> ? ? ? || code == CONJ_EXPR)
>> ? ? {
>> ? ? ? /* We can still do constant propagation here. ?*/
>> @@ -3300,10 +3306,7 @@ extract_range_from_assignment (value_ran
>> ? ? extract_range_from_assert (vr, gimple_assign_rhs1 (stmt));
>> ? else if (code == SSA_NAME)
>> ? ? extract_range_from_ssa_name (vr, gimple_assign_rhs1 (stmt));
>> - ?else if (TREE_CODE_CLASS (code) == tcc_binary
>> - ? ? ? ? ?|| code == TRUTH_AND_EXPR
>> - ? ? ? ? ?|| code == TRUTH_OR_EXPR
>> - ? ? ? ? ?|| code == TRUTH_XOR_EXPR)
>> + ?else if (TREE_CODE_CLASS (code) == tcc_binary)
>> ? ? extract_range_from_binary_expr (vr, gimple_assign_rhs_code (stmt),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gimple_expr_type (stmt),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gimple_assign_rhs1 (stmt),
>> @@ -3973,9 +3976,10 @@ build_assert_expr_for (tree cond, tree v
>> ? ? ? tree a = build2 (ASSERT_EXPR, TREE_TYPE (v), v, cond);
>> ? ? ? assertion = gimple_build_assign (n, a);
>> ? ? }
>> - ?else if (TREE_CODE (cond) == TRUTH_NOT_EXPR)
>> + ?else if (TREE_CODE (cond) == BIT_NOT_EXPR
>> + ? ? ? ? ?&& TYPE_PRECISION (TREE_TYPE (cond)) == 1)
>> ? ? {
>> - ? ? ?/* Given !V, build the assignment N = false. ?*/
>> + ? ? ?/* Given ~V, build the assignment N = false. ?*/
>> ? ? ? tree op0 = TREE_OPERAND (cond, 0);
>> ? ? ? gcc_assert (op0 == v);
>> ? ? ? assertion = gimple_build_assign (n, boolean_false_node);
>
> See my previous comment. ?This is dead code, if you want to do a cleanup
> inline it into its sole caller.
>
>> @@ -4516,11 +4520,9 @@ register_edge_assert_for_1 (tree op, enu
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?invert);
>> ? ? }
>> ? else if ((code == NE_EXPR
>> - ? ? ? ? ? && (gimple_assign_rhs_code (op_def) == TRUTH_AND_EXPR
>> - ? ? ? ? ? ? ? || gimple_assign_rhs_code (op_def) == BIT_AND_EXPR))
>> + ? ? ? ? ? && gimple_assign_rhs_code (op_def) == BIT_AND_EXPR)
>> ? ? ? ? ? || (code == EQ_EXPR
>> - ? ? ? ? ? ? ?&& (gimple_assign_rhs_code (op_def) == TRUTH_OR_EXPR
>> - ? ? ? ? ? ? ? ? ?|| gimple_assign_rhs_code (op_def) == BIT_IOR_EXPR)))
>> + ? ? ? ? ? ? ?&& gimple_assign_rhs_code (op_def) == BIT_IOR_EXPR))
>> ? ? {
>> ? ? ? /* Recurse on each operand. ?*/
>> ? ? ? retval |= register_edge_assert_for_1 (gimple_assign_rhs1 (op_def),
>> @@ -4528,7 +4530,8 @@ register_edge_assert_for_1 (tree op, enu
>> ? ? ? retval |= register_edge_assert_for_1 (gimple_assign_rhs2 (op_def),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?code, e, bsi);
>> ? ? }
>> - ?else if (gimple_assign_rhs_code (op_def) == TRUTH_NOT_EXPR)
>> + ?else if (gimple_assign_rhs_code (op_def) == BIT_NOT_EXPR
>> + ? ? ? ? ?&& TYPE_PRECISION (TREE_TYPE (op)) == 1)
>> ? ? {
>> ? ? ? /* Recurse, flipping CODE. ?*/
>> ? ? ? code = invert_tree_comparison (code, false);
>> @@ -4585,8 +4588,8 @@ register_edge_assert_for (tree name, edg
>> ? ? ?the value zero or one, then we may be able to assert values
>> ? ? ?for SSA_NAMEs which flow into COND. ?*/
>>
>> - ?/* In the case of NAME == 1 or NAME != 0, for TRUTH_AND_EXPR defining
>> - ? ? statement of NAME we can assert both operands of the TRUTH_AND_EXPR
>> + ?/* In the case of NAME == 1 or NAME != 0, for BIT_AND_EXPR defining
>> + ? ? statement of NAME we can assert both operands of the BIT_AND_EXPR
>> ? ? ?have nonzero value. ?*/
>> ? if (((comp_code == EQ_EXPR && integer_onep (val))
>> ? ? ? ?|| (comp_code == NE_EXPR && integer_zerop (val))))
>> @@ -4594,8 +4597,7 @@ register_edge_assert_for (tree name, edg
>> ? ? ? gimple def_stmt = SSA_NAME_DEF_STMT (name);
>>
>> ? ? ? if (is_gimple_assign (def_stmt)
>> - ? ? ? ? && (gimple_assign_rhs_code (def_stmt) == TRUTH_AND_EXPR
>> - ? ? ? ? ? ? || gimple_assign_rhs_code (def_stmt) == BIT_AND_EXPR))
>> + ? ? ? ? && gimple_assign_rhs_code (def_stmt) == BIT_AND_EXPR)
>> ? ? ? ?{
>> ? ? ? ? ?tree op0 = gimple_assign_rhs1 (def_stmt);
>> ? ? ? ? ?tree op1 = gimple_assign_rhs2 (def_stmt);
>> @@ -4604,8 +4606,8 @@ register_edge_assert_for (tree name, edg
>> ? ? ? ?}
>> ? ? }
>>
>> - ?/* In the case of NAME == 0 or NAME != 1, for TRUTH_OR_EXPR defining
>> - ? ? statement of NAME we can assert both operands of the TRUTH_OR_EXPR
>> + ?/* In the case of NAME == 0 or NAME != 1, for BIT_IOR_EXPR defining
>> + ? ? statement of NAME we can assert both operands of the BIT_IOR_EXPR
>> ? ? ?have zero value. ?*/
>> ? if (((comp_code == EQ_EXPR && integer_zerop (val))
>> ? ? ? ?|| (comp_code == NE_EXPR && integer_onep (val))))
>> @@ -4613,11 +4615,12 @@ register_edge_assert_for (tree name, edg
>> ? ? ? gimple def_stmt = SSA_NAME_DEF_STMT (name);
>>
>> ? ? ? if (is_gimple_assign (def_stmt)
>> - ? ? ? ? && (gimple_assign_rhs_code (def_stmt) == TRUTH_OR_EXPR
>> + ? ? ? ? && ((gimple_assign_rhs_code (def_stmt) == BIT_IOR_EXPR
>> + ? ? ? ? ? ? ?&& TYPE_PRECISION (TREE_TYPE (name)) == 1)
>> ? ? ? ? ? ? ?/* For BIT_IOR_EXPR only if NAME == 0 both operands have
>> ? ? ? ? ? ? ? ? necessarily zero value. ?*/
>> ? ? ? ? ? ? ?|| (comp_code == EQ_EXPR
>> - ? ? ? ? ? ? ? ? && (gimple_assign_rhs_code (def_stmt) == BIT_IOR_EXPR))))
>> + ? ? ? ? ? ? ? ? && gimple_assign_rhs_code (def_stmt) == BIT_IOR_EXPR)))
>
> Again you ignored my previous comments - why do I even look at the
> patches? ?This code simplifies if you CSE the BIT_IOR_EXPR test
> and move and adjust the comment.
>
>> ? ? ? ?{
>> ? ? ? ? ?tree op0 = gimple_assign_rhs1 (def_stmt);
>> ? ? ? ? ?tree op1 = gimple_assign_rhs2 (def_stmt);
>> @@ -6772,7 +6775,7 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? return false;
>> ? ? }
>>
>> - ?if (rhs_code == TRUTH_NOT_EXPR)
>> + ?if (rhs_code == BIT_NOT_EXPR && TYPE_PRECISION (TREE_TYPE (op0)) == 1)
>> ? ? {
>
> We never want to transform BIT_NOT_EXPR to BIT_XOR_EXPR, so
> please instead remove this code and do not call
> simplify_thuth_ops_using_ranges for BIT_NOT_EXPRs.
>
>> ? ? ? rhs_code = NE_EXPR;
>> ? ? ? op1 = build_int_cst (TREE_TYPE (op0), 1);
>> @@ -6787,7 +6790,7 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? ? /* Exclude anything that should have been already folded. ?*/
>> ? ? ? ? ?if (rhs_code != EQ_EXPR
>> ? ? ? ? ? ? ?&& rhs_code != NE_EXPR
>> - ? ? ? ? ? ? && rhs_code != TRUTH_XOR_EXPR)
>> + ? ? ? ? ? ? && rhs_code != BIT_XOR_EXPR)
>
> Sure not, we are not calling this function with BIT_XOR_EXPR either.
>
>> ? ? ? ? ? ?return false;
>>
>> ? ? ? ? ?if (!integer_zerop (op1)
>> @@ -6799,6 +6802,8 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? ?if (rhs_code == EQ_EXPR)
>> ? ? ? ? ? ?{
>> ? ? ? ? ? ? ?rhs_code = NE_EXPR;
>> + ? ? ? ? ? ? /* We can use here TRUTH_NOT_EXPR for doing logical-not
>> + ? ? ? ? ? ? ? ?on constant. ?*/
>> ? ? ? ? ? ? ?op1 = fold_unary (TRUTH_NOT_EXPR, TREE_TYPE (op1), op1);
>> ? ? ? ? ? ?}
>> ? ? ? ?}
>> @@ -6831,14 +6836,9 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? else
>> ? ? ? ?location = gimple_location (stmt);
>>
>> - ? ? ?if (rhs_code == TRUTH_AND_EXPR || rhs_code == TRUTH_OR_EXPR)
>> - ? ? ? ?warning_at (location, OPT_Wstrict_overflow,
>> - ? ? ? ? ? ? ? ? ? _("assuming signed overflow does not occur when "
>> - ? ? ? ? ? ? ? ? ? ? "simplifying && or || to & or |"));
>> - ? ? ?else
>> - ? ? ? ?warning_at (location, OPT_Wstrict_overflow,
>> - ? ? ? ? ? ? ? ? ? _("assuming signed overflow does not occur when "
>> - ? ? ? ? ? ? ? ? ? ? "simplifying ==, != or ! to identity or ^"));
>> + ? ? ?warning_at (location, OPT_Wstrict_overflow,
>> + ? ? ? ? ? ? ? ? _("assuming signed overflow does not occur when "
>> + ? ? ? ? ? ? ? ? ? "simplifying ==, != or ! to identity or ^"));
>> ? ? }
>>
>> ? need_conversion =
>> @@ -6853,13 +6853,10 @@ simplify_truth_ops_using_ranges (gimple_
>>
>> ? switch (rhs_code)
>> ? ? {
>> - ? ?case TRUTH_AND_EXPR:
>> - ? ? ?rhs_code = BIT_AND_EXPR;
>> - ? ? ?break;
>> - ? ?case TRUTH_OR_EXPR:
>> - ? ? ?rhs_code = BIT_IOR_EXPR;
>> + ? ?case BIT_AND_EXPR:
>> + ? ?case BIT_IOR_EXPR:
>> ? ? ? break;
>> - ? ?case TRUTH_XOR_EXPR:
>> + ? ?case BIT_XOR_EXPR:
>
> We do not call this function for BIT_*_EXPR.

We have to, and we do.  I agree that for now this code here does not
much for BIT_IOR/BIT_AND.  But with doing result sinking into
type-cast, it does pretty much here.  Please see second patch about
why this code about BIT_*_EXPR is all but dead.
And in light of this even BIT_NOT_EXPR is all but dead-code (and
should be transformed to XOR), so that we are able to handle cases
like:

int x, D3;
_Bool D1, D2;

D1 = (bool) x;
D2 = ~x;
D3 = (int) D2

and transform them to 'D3 = x ^ 1'

>> ? ? case NE_EXPR:
>> ? ? ? if (integer_zerop (op1))
>> ? ? ? ?{
>> @@ -7412,16 +7409,15 @@ simplify_stmt_using_ranges (gimple_stmt_
>>
>> ? ? ? switch (rhs_code)
>> ? ? ? ?{
>> + ? ? ? case BIT_NOT_EXPR:
>> + ? ? ? ? if (TYPE_PRECISION (TREE_TYPE (rhs1)) != 1)
>> + ? ? ? ? ? break;
>> + ? ? ? ? /* Fall through. ?*/
>
> See above.

See comment above.

>> ? ? ? ?case EQ_EXPR:
>> ? ? ? ?case NE_EXPR:
>> - ? ? ? case TRUTH_NOT_EXPR:
>> - ? ? ? case TRUTH_AND_EXPR:
>> - ? ? ? case TRUTH_OR_EXPR:
>> - ? ? ? ?case TRUTH_XOR_EXPR:
>> - ? ? ? ? ?/* Transform EQ_EXPR, NE_EXPR, TRUTH_NOT_EXPR into BIT_XOR_EXPR
>> + ? ? ? ? ?/* Transform EQ_EXPR, NE_EXPR, BIT_NOT_EXPR into BIT_XOR_EXPR
>> ? ? ? ? ? ? or identity if the RHS is zero or one, and the LHS are known
>> - ? ? ? ? ? ?to be boolean values. ?Transform all TRUTH_*_EXPR into
>> - ? ? ? ? ? ? BIT_*_EXPR if both arguments are known to be boolean values. ?*/
>> + ? ? ? ? ? ?to be boolean values. ?*/
>> ? ? ? ? ?if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
>> ? ? ? ? ? ?return simplify_truth_ops_using_ranges (gsi, stmt);
>> ? ? ? ? ?break;
>> @@ -7449,7 +7445,11 @@ simplify_stmt_using_ranges (gimple_stmt_
>> ? ? ? ? ? ? if all the bits being cleared are already cleared or
>> ? ? ? ? ? ? all the bits being set are already set. ?*/
>> ? ? ? ? ?if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
>> - ? ? ? ? ? return simplify_bit_ops_using_ranges (gsi, stmt);
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? if (simplify_truth_ops_using_ranges (gsi, stmt))
>> + ? ? ? ? ? ? ? return true;
>> + ? ? ? ? ? ? return simplify_bit_ops_using_ranges (gsi, stmt);
>
> Stale hunk I suppose.

IMHO not stale. In fact the jumping point about handling here
bitwise-binary boolean typed expressions.  And for second patch an
essential thing.

> Richard.
>
>> + ? ? ? ? ? }
>> ? ? ? ? ?break;
>>
>> ? ? ? ?CASE_CONVERT:
>>

Regards,
Kai