On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktiet...@googlemail.com> wrote: > 2011/7/8 Richard Guenther <richard.guent...@gmail.com>: >> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktiet...@googlemail.com> wrote: >>> Hello, >>> >>> This patch - second of series - adds boolification of comparisions in >>> gimplifier. For this >>> casts from/to boolean are marked as not-useless. And in fold_unary_loc >>> casts to non-boolean integral types are preserved. >>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly >>> necessary - as long as fold-const handles 1-bit precision bitwise-expression >>> with truth-logic - but it has shown to short-cut some expensier folding. So >>> I kept it within this patch. >> >> Please split it out. Also ... >> >>> >>> The adjusted testcase gcc.dg/uninit-15.c indicates that due >>> optimization we loose >>> in this case variables declaration. But this might be to be expected. >>> >>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c >>> test-case. It's caused >>> by always having boolean-type on conditions. So vectorizer sees >>> different types, which >>> aren't handled by vectorizer right now. Maybe this issue could be >>> special-cased for >>> boolean-types in tree-vect-loop, by making operand for used condition >>> equal to vector-type. >>> But this is a subject for a different patch and not addressed by this >>> series. >>> >>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed >>> by the 3rd patch of this >>> series. >>> >>> Bootstrapped and regression tested for all standard-languages (plus >>> Ada and Obj-C++) on host x86_64-pc-linux-gnu. >>> >>> Ok for apply? >>> >>> Regards, >>> Kai >>> >>> >>> ChangeLog >>> >>> 2011-07-07 Kai Tietz <kti...@redhat.com> >>> >>> * 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 (combine_cond_expr_cond): Add simplification >>> support for one-bit-precision typed X for cases X != 0 and X == 0. >>> (forward_propagate_comparison): Adjust test of condition >>> result. >>> >>> >>> * 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. >>> >>> 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 >>> @@ -2842,18 +2842,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); >>> } >>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq >>> tree org_type = TREE_TYPE (*expr_p); >>> >>> *expr_p = gimple_boolify (*expr_p); >>> - if (org_type != boolean_type_node) >>> + if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p))) >>> { >>> *expr_p = fold_convert (org_type, *expr_p); >> >> Use fold_convert_loc with saved_location > > Oh, good catch. Yes, I will adjust that. > >>> ret = GS_OK; >>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq >>> fold_truth_not_expr) happily uses operand type and doesn't >>> automatically uses boolean_type as result, we need to keep >>> orignal type. */ >>> - if (org_type != boolean_type_node) >>> + if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p))) >>> { >>> *expr_p = fold_convert (org_type, *expr_p); >> >> Likewise. Maybe this fixes the diagnostic regression. >> >>> ret = GS_OK; >>> @@ -7288,7 +7293,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 (saved_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"} } */ >> >> Hm? Why that? >> >>> /* { 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 >>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc, >>> gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison); >>> >>> t = fold_binary_loc (loc, code, type, op0, op1); >>> + >>> + if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1)) >>> + && TYPE_PRECISION (TREE_TYPE (op1)) == 1 >>> + && (code == EQ_EXPR || code == NE_EXPR)) >>> + { >>> + if (TREE_CODE (op1) == INTEGER_CST) >>> + { >>> + if (integer_onep (op1)) >>> + { >>> + op1 = fold_convert_loc (loc, TREE_TYPE (op1), >>> integer_zero_node); >>> + code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR); >> >> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then >> recurse ... that doesn't make sense to me and is super-ugly. >> What's the testcase that made you add all this code? > > Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount > of cases to handle. As for truthvalued X the we have then just to > handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1). > The recursion is someting I saw as existing pattern (for the same > thing) in truth-op folding in fold-const. > > Actual I can remove this optimization here, as it should be convered > by VRP already (when VRP handles 1-bit precision bitwise ops proper).
We should have a canonical form for those compares and change them accordingly, best in fold_stmt. Richard.