Re: [patch tree-optimization]: Fix regression about vrp47.c (and co)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Sun, Jul 31, 2011 at 5:00 AM, NightStrike <nightstrike@gmail.com> wrote:
> Ping

This patch is obsolete IIRC, a new version was posted.

Richard.

> On Thu, Jul 21, 2011 at 9:20 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> this patch adds the ability for bitwise-truth operations to sink into
>> use-statement, if it is a cast, if type of it is compatible.
>>
>> By this we can sink cases like
>>
>> _Bool D1, D2, D3;
>> int R, x, y;
>>
>> D1 = (bool) x;
>> D2 = (bool) y;
>> D3 = D1 & D2
>> R = (int) D3;
>>
>> into R-statment as
>> R = x & y;
>>
>> This fixes known vrp47.c regression.
>>
>> ChangeLog gcc
>>
>> 2011-07-21 ?Kai Tietz ?<ktietz@redhat.com>
>>
>> ? ? ? ?* tree-vrp.c (ssa_name_get_inner_ssa_name_p): New helper.
>> ? ? ? ?(ssa_name_get_cast_to_p): Likewise.
>> ? ? ? ?(simplify_truth_ops_using_ranges): Try to use type-cast
>> ? ? ? ?for simplification of bitwise-binary expressions.
>> ? ? ? ?(simplify_stmt_using_ranges): Try to sink into cast for
>> ? ? ? ?bitwise-truth operations.
>>
>> 2011-07-21 ?Kai Tietz ?<ktietz@redhat.com>
>>
>> ? ? ? ?* gcc.dg/tree-ssa/vrp47.c: Adjust testcase.
>>
>> Bootstrapped and regression tested for all standard languages
>> (including Ada and Obj-C++) on
>> host x86_64-pc-linux-gnu. ?Ok for apply?
>>
>>
>> Regards,
>> Kai
>>
>> Index: gcc-head/gcc/tree-vrp.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-vrp.c
>> +++ gcc-head/gcc/tree-vrp.c
>> @@ -6747,19 +6746,92 @@ varying:
>> ? return SSA_PROP_VARYING;
>> ?}
>>
>> +/* Returns operand1 of ssa-name with SSA_NAME as code, Otherwise it
>> + ? returns NULL_TREE. ?*/
>> +static tree
>> +ssa_name_get_inner_ssa_name_p (tree op)
>> +{
>> + ?gimple stmt;
>> +
>> + ?if (TREE_CODE (op) != SSA_NAME
>> + ? ? ?|| !is_gimple_assign (SSA_NAME_DEF_STMT (op)))
>> + ? ?return NULL_TREE;
>> + ?stmt = SSA_NAME_DEF_STMT (op);
>> + ?if (gimple_assign_rhs_code (stmt) != SSA_NAME)
>> + ? ?return NULL_TREE;
>> + ?return gimple_assign_rhs1 (stmt);
>> +}
>> +
>> +/* Returns operand of cast operation, if OP is a type-conversion. Otherwise
>> + ? return NULL_TREE. ?*/
>> +static tree
>> +ssa_name_get_cast_to_p (tree op)
>> +{
>> + ?gimple stmt;
>> +
>> + ?if (TREE_CODE (op) != SSA_NAME
>> + ? ? ?|| !is_gimple_assign (SSA_NAME_DEF_STMT (op)))
>> + ? ?return NULL_TREE;
>> + ?stmt = SSA_NAME_DEF_STMT (op);
>> + ?if (!CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt)))
>> + ? ?return NULL_TREE;
>> + ?return gimple_assign_rhs1 (stmt);
>> +}
>> +
>> ?/* Simplify boolean operations if the source is known
>> ? ?to be already a boolean. ?*/
>> ?static bool
>> ?simplify_truth_ops_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
>> ?{
>> ? enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
>> + ?gimple stmt2 = stmt;
>> ? tree val = NULL;
>> ? tree op0, op1;
>> ? value_range_t *vr;
>> ? bool sop = false;
>> ? bool need_conversion;
>> + ?location_t loc = gimple_location (stmt);
>>
>> ? op0 = gimple_assign_rhs1 (stmt);
>> + ?op1 = NULL_TREE;
>> +
>> + ?/* Handle cases with prefixed type-cast. ?*/
>> + ?if (CONVERT_EXPR_CODE_P (rhs_code)
>> + ? ? ?&& INTEGRAL_TYPE_P (TREE_TYPE (op0))
>> + ? ? ?&& TREE_CODE (op0) == SSA_NAME
>> + ? ? ?&& is_gimple_assign (SSA_NAME_DEF_STMT (op0))
>> + ? ? ?&& INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
>> + ? ?{
>> + ? ? ?stmt2 = SSA_NAME_DEF_STMT (op0);
>> + ? ? ?op0 = gimple_assign_rhs1 (stmt2);
>> + ? ? ?if (!INTEGRAL_TYPE_P (TREE_TYPE (op0)))
>> + ? ? ? return false;
>> + ? ? ?rhs_code = gimple_assign_rhs_code (stmt2);
>> + ? ? ?if (rhs_code != BIT_NOT_EXPR
>> + ? ? ? ? && rhs_code != BIT_AND_EXPR
>> + ? ? ? ? && rhs_code != BIT_IOR_EXPR
>> + ? ? ? ? && rhs_code != BIT_XOR_EXPR
>> + ? ? ? ? && rhs_code != NE_EXPR && rhs_code != EQ_EXPR)
>> + ? ? ? return false;
>> +
>> + ? ? ?if (rhs_code != BIT_NOT_EXPR)
>> + ? ? ? op1 = gimple_assign_rhs2 (stmt2);
>> +
>> + ? ? ?if (gimple_has_location (stmt2))
>> + ? ? ? ?loc = gimple_location (stmt2);
>> + ? ?}
>> + ?else if (CONVERT_EXPR_CODE_P (rhs_code))
>> + ? ?return false;
>> + ?else if (rhs_code != BIT_NOT_EXPR)
>> + ? ?op1 = gimple_assign_rhs2 (stmt);
>> +
>> + ?/* ~X is only equivalent to !X, if type-precision is one and X has
>> + ? ? an integral type. ?*/
>> + ?if (rhs_code == BIT_NOT_EXPR
>> + ? ? ?&& (!INTEGRAL_TYPE_P (TREE_TYPE (op0))
>> + ? ? ? ? || TYPE_PRECISION (TREE_TYPE (op0)) != 1))
>> + ? ?return false;
>> +
>> ? if (TYPE_PRECISION (TREE_TYPE (op0)) != 1)
>> ? ? {
>> ? ? ? if (TREE_CODE (op0) != SSA_NAME)
>> @@ -6775,19 +6847,83 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? return false;
>> ? ? }
>>
>> - ?if (rhs_code == BIT_NOT_EXPR && TYPE_PRECISION (TREE_TYPE (op0)) == 1)
>> + ?need_conversion =
>> + ? ?!useless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_TYPE (op0));
>> + ?/* As comparisons X != 0 getting folded to (bool) X by VRP,
>> + ? ? but X == 0 might be not folded for none boolean type of X
>> + ? ? to (bool) (X ^ 1), we need to handle this case special
>> + ? ? to simplify this.
>> + ? ? For bitwise-binary operations we have three cases to handle:
>> + ? ? a) ((bool) X) op ((bool) Y)
>> + ? ? b) ((bool) X) op (Y == 0) -OR- (X == 0) op ((bool) Y)
>> + ? ? c) (X == 0) op (Y == 0)
>> + ? ? The later two cases can't be handled for now, as we would beed to
>> + ? ? insert new statements. ?*/
>> + ?if (need_conversion
>> + ? ? ?&& (rhs_code == BIT_XOR_EXPR
>> + ? ? ? ? || rhs_code == BIT_AND_EXPR
>> + ? ? ? ? || rhs_code == BIT_IOR_EXPR)
>> + ? ? ?&& TREE_CODE (op1) == SSA_NAME
>> + ? ? ?&& TREE_CODE (op0) == SSA_NAME)
>> ? ? {
>> - ? ? ?rhs_code = NE_EXPR;
>> - ? ? ?op1 = build_int_cst (TREE_TYPE (op0), 1);
>> + ? ? ?tree cop0 = ssa_name_get_cast_to_p (op0);
>> + ? ? ?tree cop1 = ssa_name_get_cast_to_p (op1);
>> +
>> + ? ? ?/* We would need an new statment for cases b and c, and we can't
>> + ? ? ? ?due vr table, so bail out. ?*/
>> + ? ? ?if (!cop0 || !cop1)
>> + ? ? ? ?return false;
>> +
>> + ? ? ?if (!INTEGRAL_TYPE_P (TREE_TYPE (cop0))
>> + ? ? ? ? || !types_compatible_p (TREE_TYPE (cop0), TREE_TYPE (cop1)))
>> + ? ? ? return false;
>> +
>> + ? ? ?need_conversion =
>> + ? ? ? !useless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_TYPE (cop0));
>> + ? ? ?if (need_conversion)
>> + ? ? ? return false;
>> +
>> + ? ? ?op0 = cop0;
>> + ? ? ?op1 = cop1;
>> +
>> + ? ? ?/* We need to re-check if value ranges for new operands
>> + ? ? ? ? for 1-bit precision/range. ?*/
>> + ? ? ?if (TYPE_PRECISION (TREE_TYPE (op0)) != 1)
>> + ? ? ? {
>> + ? ? ? ? if (TREE_CODE (op0) != SSA_NAME)
>> + ? ? ? ? ? return false;
>> + ? ? ? ? vr = get_value_range (op0);
>> +
>> + ? ? ? ? val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
>> + ? ? ? ? if (!val || !integer_onep (val))
>> + ? ? ? ? ? return false;
>> +
>> + ? ? ? ? val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
>> + ? ? ? ? if (!val || !integer_onep (val))
>> + ? ? ? ? ? return false;
>> + ? ? ? }
>> +
>> + ? ? ?if (TYPE_PRECISION (TREE_TYPE (op1)) != 1)
>> + ? ? ? {
>> + ? ? ? ? vr = get_value_range (op1);
>> + ? ? ? ? val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
>> + ? ? ? ? if (!val || !integer_onep (val))
>> + ? ? ? ? ? return false;
>> +
>> + ? ? ? ? val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
>> + ? ? ? ? if (!val || !integer_onep (val))
>> + ? ? ? ? ? return false;
>> + ? ? ? }
>> ? ? }
>> - ?else
>> - ? ?{
>> - ? ? ?op1 = gimple_assign_rhs2 (stmt);
>>
>> - ? ? ?/* Reduce number of cases to handle. ?*/
>> + ?/* Reduce number of cases to handle. ?*/
>> + ?if (op1 != NULL_TREE)
>> + ? ?{
>> ? ? ? if (is_gimple_min_invariant (op1))
>> ? ? ? ?{
>> - ? ? ? ? ?/* Exclude anything that should have been already folded. ?*/
>> + ? ? ? ? /* Exclude anything that should have been already folded. ?*/
>> ? ? ? ? ?if (rhs_code != EQ_EXPR
>> ? ? ? ? ? ? ?&& rhs_code != NE_EXPR
>> ? ? ? ? ? ? ?&& rhs_code != BIT_XOR_EXPR)
>> @@ -6818,32 +6954,34 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? ? ? ?vr = get_value_range (op1);
>> ? ? ? ? ? ? ?val = compare_range_with_value (GE_EXPR, vr, integer_zero_node, &sop);
>> ? ? ? ? ? ? ?if (!val || !integer_onep (val))
>> - ? ? ? ? ? ? ? return false;
>> + ? ? ? ? ? ? ? return false;
>>
>> ? ? ? ? ? ? ?val = compare_range_with_value (LE_EXPR, vr, integer_one_node, &sop);
>> ? ? ? ? ? ? ?if (!val || !integer_onep (val))
>> - ? ? ? ? ? ? ? return false;
>> + ? ? ? ? ? ? ? return false;
>> ? ? ? ? ? ?}
>> ? ? ? ?}
>> - ? ?}
>>
>> - ?if (sop && issue_strict_overflow_warning (WARN_STRICT_OVERFLOW_MISC))
>> - ? ?{
>> - ? ? ?location_t location;
>> + ? ? ?if (sop && issue_strict_overflow_warning (WARN_STRICT_OVERFLOW_MISC))
>> + ? ? ? {
>> + ? ? ? ? location_t location;
>>
>> - ? ? ?if (!gimple_has_location (stmt))
>> - ? ? ? location = input_location;
>> - ? ? ?else
>> - ? ? ? location = gimple_location (stmt);
>> + ? ? ? ? if (!gimple_has_location (stmt))
>> + ? ? ? ? ? location = input_location;
>> + ? ? ? ? else
>> + ? ? ? ? ? location = gimple_location (stmt);
>>
>> - ? ? ?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 =
>> - ? ?!useless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_TYPE (op0));
>> + ?if (rhs_code == BIT_NOT_EXPR)
>> + ? ?{
>> + ? ? ?rhs_code = NE_EXPR;
>> + ? ? ?op1 = build_int_cst (TREE_TYPE (op0), 1);
>> + ? ?}
>>
>> ? /* Make sure to not sign-extend -1 as a boolean value. ?*/
>> ? if (need_conversion
>> @@ -6863,6 +7001,7 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? ? ?gimple_assign_set_rhs_with_ops (gsi,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?need_conversion ? NOP_EXPR : SSA_NAME,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?op0, NULL);
>> + ? ? ? ? gimple_set_location (stmt, loc);
>> ? ? ? ? ?update_stmt (gsi_stmt (*gsi));
>> ? ? ? ? ?return true;
>> ? ? ? ?}
>> @@ -6873,10 +7012,20 @@ simplify_truth_ops_using_ranges (gimple_
>> ? ? ? gcc_unreachable ();
>> ? ? }
>>
>> + ?/* We can't insert here new expression as otherwise
>> + ? ? tracked vr tables getting out of bounds. ?*/
>> ? if (need_conversion)
>> ? ? return false;
>>
>> + ?/* Reduce here simple SSA_NAME assignments to other expressions.
>> + ? ? This produces better results. ?*/
>> + ?while ((val = ssa_name_get_inner_ssa_name_p (op0)) != NULL_TREE)
>> + ? ?op0 = val;
>> + ?while ((val = ssa_name_get_inner_ssa_name_p (op1)) != NULL_TREE)
>> + ? ?op1 = val;
>> +
>> ? gimple_assign_set_rhs_with_ops (gsi, rhs_code, op0, op1);
>> + ?gimple_set_location (stmt, loc);
>> ? update_stmt (gsi_stmt (*gsi));
>> ? return true;
>> ?}
>> @@ -7455,7 +7604,11 @@ simplify_stmt_using_ranges (gimple_stmt_
>> ? ? ? ?CASE_CONVERT:
>> ? ? ? ? ?if (TREE_CODE (rhs1) == SSA_NAME
>> ? ? ? ? ? ? ?&& INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
>> - ? ? ? ? ? return simplify_conversion_using_ranges (stmt);
>> + ? ? ? ? ? {
>> + ? ? ? ? ? ? if (simplify_truth_ops_using_ranges (gsi, stmt))
>> + ? ? ? ? ? ? ? return true;
>> + ? ? ? ? ? ? return simplify_conversion_using_ranges (stmt);
>> + ? ? ? ? ? }
>> ? ? ? ? ?break;
>>
>> ? ? ? ?default:
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/vrp47.c
>> @@ -4,7 +4,7 @@
>> ? ?jumps when evaluating an && condition. ?VRP is not able to optimize
>> ? ?this. ?*/
>> ?/* { dg-do compile { target { ! "mips*-*-* s390*-*-* ?avr-*-*
>> mn10300-*-*" } } } */
>> -/* { dg-options "-O2 -fdump-tree-vrp -fdump-tree-dom" } */
>> +/* { dg-options "-O2 -fdump-tree-vrp" } */
>> ?/* { dg-options "-O2 -fdump-tree-vrp -fdump-tree-dom -march=i586" {
>> target { i?86-*-* && ilp32 } } } */
>>
>> ?int h(int x, int y)
>> @@ -36,13 +36,10 @@ int f(int x)
>> ? ?0 or 1. ?*/
>> ?/* { dg-final { scan-tree-dump-times "\[xy\]\[^ \]* !=" 0 "vrp1" } } */
>>
>> -/* This one needs more copy propagation that only happens in dom1. ?*/
>> -/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "dom1" } } */
>> -/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "vrp1" { xfail
>> *-*-* } } } */
>> +/* { dg-final { scan-tree-dump-times "x\[^ \]* & y" 1 "vrp1" } } */
>>
>> ?/* These two are fully simplified by VRP. ?*/
>> ?/* { dg-final { scan-tree-dump-times "x\[^ \]* \[|\] y" 1 "vrp1" } } */
>> ?/* { dg-final { scan-tree-dump-times "x\[^ \]* \\^ 1" 1 "vrp1" } } */
>>
>> ?/* { dg-final { cleanup-tree-dump "vrp\[0-9\]" } } */
>> -/* { dg-final { cleanup-tree-dump "dom\[0-9\]" } } */
>>
>