2011/7/19 Richard Guenther <[email protected]>:
> On Tue, Jul 19, 2011 at 1:27 PM, Kai Tietz <[email protected]> wrote:
>> Hello,
>>
>> So this is the updated patch for boolifying of comparisons. There are
>> two different kind of regression caused by this. One is fixed by the
>> VRP patch I've sent and it waits for review. The other one is about
>> vectorization in gcc.dg/vect/vect-cond3.c
>> testcase. This is caused by vectorization pass, which can't handle
>> different types for vectorization, as now the conditions are always
>> boolean-typed.
>>
>> ChangeLog
>>
>> 2011-07-19 Kai Tietz <[email protected]>
>>
>> * fold-const.c (fold_unary_loc): Preserve
>> non-boolean-typed casts.
>> * gimplify.c (gimple_boolify): Handle boolification
>> of comparisons.
>> (gimplify_expr): Boolifiy non aggregate-typed
>> comparisons.
>> * tree-cfg.c (verify_gimple_comparison): Check result
>> type of comparison expression.
>> * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>> casts from/to boolean,
>> * tree-ssa-forwprop.c (forward_propagate_comparison):
>> Adjust test of condition result and disallow type-cast
>> sinking into comparison.
>>
>>
>> * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>> * gcc.dg/tree-ssa/pr21031.c: Likewise.
>> * gcc.dg/tree-ssa/pr30978.c: Likewise.
>> * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>> * gcc.dg/binop-xor1.c: Mark it as expected fail.
>> * gcc.dg/binop-xor3.c: Likewise.
>> * gcc.dg/uninit-15.c: Adjust reported message.
>>
>> Bootstrapped and regression tested for x86_64-pc-linux-gnu. Ok for apply?
>
> Please test and commit the fold-const.c and tree-ssa.c parts separately,
> they are pre-approved if they pass separate testing.
>
> I think
>
>> * gcc.dg/uninit-15.c: Adjust reported message.
>
> is a bogus change. Please figure out why you need it.
Well, somewhere the location for statement gets lost. I was searching
for the cause already some time and figured out it must be related to
some folding, which occures just by this type-preserving and might be
caused by some code using build2/3 instead of the build2/3_loc
variant. This was the cause why I added here to some foldings the
location preserving.
I tested this with just the approved part and see that this regression
occures already here.
> Likewise the builtin-expect-5.c change.
This I will address in my next patch for the approved part. This is
caused by the folding for builtin_expect in builtins.c. It gets
confused about type-casts on arguments for ANDIF/ORIF AFAICS.
>> Regards,
>> Kai
>>
>> PS: Might be that due your recent patch to forwprop, that patch
>> doesn't apply here cleanly.
>
> You will need to re-test ontop of that patch anyway I guess.
>
> And deal with the gcc.dg/vect/vect-cond3.c fail.
Well, this regression is caused by vectorizer inability to fold things
with two different types. It finds here a pattern with types
'int,int,_Bool,_Bool'. As by this patch conditions have always just
boolean-type. So this regression shows more a missing feature in
vectorizer.
I tested here some approachs to solve this, but I didn't found an easy
way to solve this until now.
> Richard.
>
>> Index: gcc-head/gcc/fold-const.c
>> ===================================================================
>> --- gcc-head.orig/gcc/fold-const.c
>> +++ gcc-head/gcc/fold-const.c
>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>> non-integral type.
>> Do not fold the result as that would not simplify further, also
>> folding again results in recursions. */
>> - if (INTEGRAL_TYPE_P (type))
>> + if (TREE_CODE (type) == BOOLEAN_TYPE)
>> return build2_loc (loc, TREE_CODE (op0), type,
>> TREE_OPERAND (op0, 0),
>> TREE_OPERAND (op0, 1));
>> - else
>> + else if (!INTEGRAL_TYPE_P (type))
>> return build3_loc (loc, COND_EXPR, type, op0,
>> fold_convert (type, boolean_true_node),
>> fold_convert (type, boolean_false_node));
>> Index: gcc-head/gcc/gimplify.c
>> ===================================================================
>> --- gcc-head.orig/gcc/gimplify.c
>> +++ gcc-head/gcc/gimplify.c
>> @@ -2860,18 +2860,23 @@ gimple_boolify (tree expr)
>>
>> case TRUTH_NOT_EXPR:
>> TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>> - /* FALLTHRU */
>>
>> - case EQ_EXPR: case NE_EXPR:
>> - case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>> /* These expressions always produce boolean results. */
>> - TREE_TYPE (expr) = boolean_type_node;
>> + if (TREE_CODE (type) != BOOLEAN_TYPE)
>> + TREE_TYPE (expr) = boolean_type_node;
>> return expr;
>>
>> default:
>> + if (COMPARISON_CLASS_P (expr))
>> + {
>> + /* There expressions always prduce boolean results. */
>> + if (TREE_CODE (type) != BOOLEAN_TYPE)
>> + TREE_TYPE (expr) = boolean_type_node;
>> + return expr;
>> + }
>> /* Other expressions that get here must have boolean values, but
>> might need to be converted to the appropriate mode. */
>> - if (type == boolean_type_node)
>> + if (TREE_CODE (type) == BOOLEAN_TYPE)
>> return expr;
>> return fold_convert_loc (loc, boolean_type_node, expr);
>> }
>> @@ -6791,7 +6796,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>> *expr_p = gimple_boolify (*expr_p);
>> if (!useless_type_conversion_p (orig_type, TREE_TYPE (*expr_p)))
>> {
>> - *expr_p = fold_convert_loc (saved_location, orig_type,
>> *expr_p);
>> + *expr_p = fold_convert_loc (input_location, orig_type,
>> *expr_p);
>> ret = GS_OK;
>> break;
>> }
>> @@ -7309,7 +7314,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>
>> if (!AGGREGATE_TYPE_P (type))
>> - goto expr_2;
>> + {
>> + tree org_type = TREE_TYPE (*expr_p);
>> + *expr_p = gimple_boolify (*expr_p);
>> + if (!useless_type_conversion_p (org_type,
>> + TREE_TYPE (*expr_p)))
>> + {
>> + *expr_p = fold_convert_loc (input_location,
>> + org_type, *expr_p);
>> + ret = GS_OK;
>> + }
>> + else
>> + goto expr_2;
>> + }
>> else if (TYPE_MODE (type) != BLKmode)
>> ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>> else
>> Index: gcc-head/gcc/tree-cfg.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-cfg.c
>> +++ gcc-head/gcc/tree-cfg.c
>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>> && (!POINTER_TYPE_P (op0_type)
>> || !POINTER_TYPE_P (op1_type)
>> || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>> - || !INTEGRAL_TYPE_P (type))
>> + || !INTEGRAL_TYPE_P (type)
>> + || (TREE_CODE (type) != BOOLEAN_TYPE
>> + && TYPE_PRECISION (type) != 1))
>> {
>> error ("type mismatch in comparison expression");
>> debug_generic_expr (type);
>> Index: gcc-head/gcc/tree-ssa.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-ssa.c
>> +++ gcc-head/gcc/tree-ssa.c
>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>> || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>> return false;
>>
>> - /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>> - one. */
>> - if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>> - && TREE_CODE (outer_type) == BOOLEAN_TYPE
>> + /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>> + of precision one. */
>> + if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>> + != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>> && TYPE_PRECISION (outer_type) != 1)
>> return false;
>>
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> @@ -11,5 +11,5 @@ f (int i, float j)
>>
>> /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>> /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>> "forwprop1"} } */
>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>> "forwprop1"} } */
>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>> 1\);\n[^\n]*if} "forwprop1"} } */
>> /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> @@ -16,5 +16,5 @@ foo (int a)
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>> /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> @@ -10,5 +10,5 @@ int foo(int a)
>> return e;
>> }
>>
>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>> /* { dg-final { cleanup-tree-dump "optimized" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> @@ -2,5 +2,5 @@
>> /* { dg-options "-O -fdump-tree-fre1-details" } */
>>
>> int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>> /* { dg-final { cleanup-tree-dump "fre1" } } */
>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>> @@ -1122,30 +1174,22 @@ forward_propagate_comparison (gimple stm
>> if (!use_stmt)
>> return false;
>>
>> - /* Conversion of the condition result to another integral type. */
>> + /* Test of the condition result. */
>> if (is_gimple_assign (use_stmt)
>> - && (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt))
>> - || TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
>> - == tcc_comparison
>> - || gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR)
>> - && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (use_stmt))))
>> + && ((TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
>> + == tcc_comparison)
>> + || gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR
>> + || (gimple_assign_rhs_code (use_stmt) == BIT_NOT_EXPR
>> + && TYPE_PRECISION (TREE_TYPE (gimple_assign_lhs (use_stmt))))))
>> {
>> tree lhs = gimple_assign_lhs (use_stmt);
>>
>> - /* We can propagate the condition into a conversion. */
>> - if (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt)))
>> - {
>> - /* Avoid using fold here as that may create a COND_EXPR with
>> - non-boolean condition as canonical form. */
>> - tmp = build2 (gimple_assign_rhs_code (stmt), TREE_TYPE (lhs),
>> - gimple_assign_rhs1 (stmt), gimple_assign_rhs2
>> (stmt));
>> - }
>> /* We can propagate the condition into X op CST where op
>> is EQ_EXPR or NE_EXPR and CST is either one or zero. */
>> - else if (TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
>> - == tcc_comparison
>> - && TREE_CODE (gimple_assign_rhs1 (use_stmt)) == SSA_NAME
>> - && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST)
>> + if (TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
>> + == tcc_comparison
>> + && TREE_CODE (gimple_assign_rhs1 (use_stmt)) == SSA_NAME
>> + && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST)
>> {
>> enum tree_code code = gimple_assign_rhs_code (use_stmt);
>> tree cst = gimple_assign_rhs2 (use_stmt);
>> @@ -1164,7 +1208,8 @@ forward_propagate_comparison (gimple stm
>> }
>> /* We can propagate the condition into a statement that
>> computes the logical negation of the comparison result. */
>> - else if (gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR)
>> + else if (gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR
>> + || gimple_assign_rhs_code (use_stmt) == BIT_NOT_EXPR)
>> {
>> tree type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>> bool nans = HONOR_NANS (TYPE_MODE (type));
>> @@ -1810,6 +1855,7 @@ simplify_bitwise_binary (gimple_stmt_ite
>> arg2));
>> tem = make_ssa_name (tem, newop);
>> gimple_assign_set_lhs (newop, tem);
>> + gimple_set_location (newop, gimple_location (stmt));
>> gsi_insert_before (gsi, newop, GSI_SAME_STMT);
>> gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
>> tem, NULL_TREE, NULL_TREE);
>> @@ -1839,6 +1885,7 @@ simplify_bitwise_binary (gimple_stmt_ite
>> newop = gimple_build_assign_with_ops (code, tem, def1_arg1, def2_arg1);
>> tem = make_ssa_name (tem, newop);
>> gimple_assign_set_lhs (newop, tem);
>> + gimple_set_location (newop, gimple_location (stmt));
>> gsi_insert_before (gsi, newop, GSI_SAME_STMT);
>> gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
>> tem, NULL_TREE, NULL_TREE);
>> @@ -1867,6 +1914,7 @@ simplify_bitwise_binary (gimple_stmt_ite
>> tem, def1_arg1, arg2);
>> tem = make_ssa_name (tem, newop);
>> gimple_assign_set_lhs (newop, tem);
>> + gimple_set_location (newop, gimple_location (stmt));
>> /* Make sure to re-process the new stmt as it's walking upwards. */
>> gsi_insert_before (gsi, newop, GSI_NEW_STMT);
>> gimple_assign_set_rhs1 (stmt, tem);
>> Index: gcc-head/gcc/testsuite/gcc.dg/binop-xor1.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/binop-xor1.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/binop-xor1.c
>> @@ -7,8 +7,5 @@ foo (int a, int b, int c)
>> return ((a && !b && c) || (!a && b && c));
>> }
>>
>> -/* We expect to see "<bb N>"; confirm that, so that we know to count
>> - it in the real test. */
>> -/* { dg-final { scan-tree-dump-times "<bb\[^>\]*>" 5 "optimized" } } */
>> -/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" } } */
>> +/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" { xfail
>> *-*-* } } } */
>> /* { dg-final { cleanup-tree-dump "optimized" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/binop-xor3.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/binop-xor3.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/binop-xor3.c
>> @@ -7,8 +7,5 @@ foo (int a, int b)
>> return ((a && !b) || (!a && b));
>> }
>>
>> -/* We expect to see "<bb N>"; confirm that, so that we know to count
>> - it in the real test. */
>> -/* { dg-final { scan-tree-dump-times "<bb\[^>\]*>" 1 "optimized" } } */
>> -/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" } } */
>> +/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" { xfail
>> *-*-* } } } */
>> /* { dg-final { cleanup-tree-dump "optimized" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/uninit-15.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/uninit-15.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/uninit-15.c
>> @@ -18,9 +18,9 @@ foo (int i)
>> void baz (void);
>>
>> void
>> -bar (void)
>> +bar (void) /* { dg-warning "'j' is used uninitialized" } */
>> {
>> int j; /* { dg-message "note: 'j' was declared here" "" { xfail *-*-* } }
>> */
>> - for (; foo (j); ++j) /* { dg-warning "'j' is used uninitialized" } */
>> + for (; foo (j); ++j)
>> baz ();
>> }
>>
>
--
| (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination
