From: Richard Guenther <richard.guenther@gmail.com>
To: Kai Tietz <ktietz70@googlemail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [patch tree-optimization]: [2 of 3]: Boolify compares & more
Date: Tue, 12 Jul 2011 09:29:00 -0000 [thread overview]
Message-ID: <CAFiYyc3eMq+1xOyDYEAjJ2H36kabdmYZ5sZr6bsjg3HS=5raUA@mail.gmail.com> (raw)
In-Reply-To: <CAEwic4aJcEw-KzR1kvc5syQ9EXvUjKmqaUHfTgOAfLjhi-WPsA@mail.gmail.com>
On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@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 <ktietz@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.
>
> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
> of branch it seems not to be invoked at all. At least not now without
> boolification of compares. One nit I found for GIMPLE_BINARY, as here
> just patterns getting replaced, which have fewer number of ops then
> original statement. This check looks a bit bogus.
> For getting this normalization right now in a consistant way,
> fold-const might be right now the better place to handle this.
It gets invoked (well, should get invoked) when anyone changes the
statement. If it is present in that form from the beginning then we
lack canonicalization in fold and/or gimplification.
I suppose you have a testcase?
Richard.
> Regards,
> Kai
>
next prev parent reply other threads:[~2011-07-12 9:22 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-07 16:08 Kai Tietz
2011-07-08 9:28 ` Richard Guenther
2011-07-08 11:35 ` Kai Tietz
2011-07-08 12:03 ` Richard Guenther
2011-07-11 15:57 ` Kai Tietz
2011-07-12 9:29 ` Richard Guenther [this message]
2011-07-12 10:00 ` Kai Tietz
2011-07-12 10:34 ` Richard Guenther
2011-07-12 12:25 ` Kai Tietz
2011-07-12 14:38 ` Richard Guenther
2011-07-19 12:08 ` Kai Tietz
2011-07-19 12:16 ` Richard Guenther
2011-07-19 22:24 ` Kai Tietz
2011-07-20 13:32 ` Kai Tietz
2011-07-20 13:41 ` Richard Guenther
2011-07-20 14:07 ` Kai Tietz
2011-07-20 14:29 ` Richard Guenther
2011-07-20 17:43 ` Kai Tietz
2011-07-21 11:34 ` Kai Tietz
2011-07-21 12:13 ` Richard Guenther
2011-07-21 12:48 ` Kai Tietz
2011-07-21 15:49 ` H.J. Lu
2011-07-21 15:52 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc3eMq+1xOyDYEAjJ2H36kabdmYZ5sZr6bsjg3HS=5raUA@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ktietz70@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).