| Message ID | CAEwic4ZRuexC_9PZDry_oXX=DMpmW=SAwzPvcVPd8PqnDcv7QQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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. > + 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); > + } > + 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). > || 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. > 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. > 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. Richard. > + } > break; > > CASE_CONVERT: >
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
On Fri, Jul 22, 2011 at 10:14 AM, Kai Tietz <ktietz70@googlemail.com> wrote: > 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. We would run into it right now without this patch and we don't. >>> + 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. I'm sure this comment is bogus. As with the above, we'd run into the bug right now, so just remove the TRUTH_* cases. >>> + 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. We don't. And we won't have to. > 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. The second patch puts an unrelated simplification into this function. > 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' No, because that would be a wrong transformation. >>> 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. It becomes stale if you follow my earlier comments. Richard. >> Richard. >> >>> + } >>> break; >>> >>> CASE_CONVERT: >>> > > 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. */ + 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) + 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. */ + 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); + } + 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) || 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); @@ -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))) { 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) { 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) 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: 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. */ 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); + } break; CASE_CONVERT: