* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:12 ` Richard Guenther
@ 2011-05-26 11:20 ` Richard Guenther
2011-05-26 11:52 ` Kai Tietz
2011-05-26 11:30 ` Richard Guenther
2011-05-26 11:39 ` Kai Tietz
2 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 11:20 UTC (permalink / raw)
To: Kai Tietz; +Cc: gcc-patches
On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>> Hello,
>>
>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>
>> ChangeLog
>>
>> 2011-05-26 Kai Tietz
>>
>> * gimplify.c (gimple_boolify): Boolify all comparison
>> expressions.
>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>> boolean-type special.
>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>> or compatible types.
>> (verify_gimple_assign_unary): Likewise.
>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>> boolean case special.
>>
>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>
> It obviously isn't ok to apply before a patch has gone in that will resolve
> all of the FAILs you specify. Comments on the patch:
>
> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
> plain wrong if bitfields are involved. */
> {
> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
> + tree org_type = TREE_TYPE (*expr_p);
> +
> + if (!useless_type_conversion_p (org_type, boolean_type_node))
> + {
> + TREE_TYPE (*expr_p) = boolean_type_node;
> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
> + ret = GS_OK;
> + goto dont_recalculate;
> + }
>
> The above should be only done for !AGGREGATE_TYPE_P. Probably then
> the strange dont_recalcuate goto can go away as well.
>
> if (!AGGREGATE_TYPE_P (type))
> - goto expr_2;
> + {
> + enum gimplify_status r0, r1;
> +
> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> +
> + ret = MIN (r0, r1);
> + }
> +
>
> why change this?
>
> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
> }
> else if (COMPARISON_CLASS_P (arg0))
> {
> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
> + Otherwise this will lead to race-condition on gimplification
> + trying to boolify comparison expression. */
> + if (TREE_TYPE (op0) == boolean_type_node)
> + return NULL_TREE;
> +
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
>
> The code leading here looks quite strange to me ...
>
> tree
> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
> {
> ...
> if (TREE_CODE_CLASS (code) == tcc_unary)
> {
> ...
> else if (COMPARISON_CLASS_P (arg0))
> {
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
> TREE_TYPE (arg0) = type;
> return arg0;
> }
> else if (TREE_CODE (type) != INTEGER_TYPE)
> return fold_build3_loc (loc, COND_EXPR, type, arg0,
> fold_build1_loc (loc, code, type,
> integer_one_node),
> fold_build1_loc (loc, code, type,
> integer_zero_node));
> }
>
> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
> in which case they should be dropped or moved below where we
> handle conversions explicitly.
>
> That said - does anyone remember anything about that above code?
> Trying to do some svn blame history tracking now ...
Oh, the patch continues...
@@ -3208,7 +3208,10 @@ 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))
+ || !(TREE_CODE (type) == BOOLEAN_TYPE
+ || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
+ && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
+ || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
{
why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
that.
@@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
case TRUTH_NOT_EXPR:
/* We require two-valued operand types. */
if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
+ || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
+ && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
|| (INTEGRAL_TYPE_P (rhs1_type)
&& TYPE_PRECISION (rhs1_type) == 1)))
{
likewise.
Richard.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:20 ` Richard Guenther
@ 2011-05-26 11:52 ` Kai Tietz
2011-05-26 12:04 ` Richard Guenther
0 siblings, 1 reply; 14+ messages in thread
From: Kai Tietz @ 2011-05-26 11:52 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>> Hello,
>>>
>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>
>>> ChangeLog
>>>
>>> 2011-05-26 Kai Tietz
>>>
>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>> expressions.
>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>> boolean-type special.
>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>> or compatible types.
>>> (verify_gimple_assign_unary): Likewise.
>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>> boolean case special.
>>>
>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>
>> It obviously isn't ok to apply before a patch has gone in that will resolve
>> all of the FAILs you specify. Comments on the patch:
>>
>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>> plain wrong if bitfields are involved. */
>> {
>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>> + tree org_type = TREE_TYPE (*expr_p);
>> +
>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>> + {
>> + TREE_TYPE (*expr_p) = boolean_type_node;
>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>> + ret = GS_OK;
>> + goto dont_recalculate;
>> + }
>>
>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>> the strange dont_recalcuate goto can go away as well.
>>
>> if (!AGGREGATE_TYPE_P (type))
>> - goto expr_2;
>> + {
>> + enum gimplify_status r0, r1;
>> +
>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>> + post_p, is_gimple_val, fb_rvalue);
>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>> + post_p, is_gimple_val, fb_rvalue);
>> +
>> + ret = MIN (r0, r1);
>> + }
>> +
>>
>> why change this?
>>
>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>> }
>> else if (COMPARISON_CLASS_P (arg0))
>> {
>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>> + Otherwise this will lead to race-condition on gimplification
>> + trying to boolify comparison expression. */
>> + if (TREE_TYPE (op0) == boolean_type_node)
>> + return NULL_TREE;
>> +
>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>> {
>> arg0 = copy_node (arg0);
>>
>> The code leading here looks quite strange to me ...
>>
>> tree
>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>> {
>> ...
>> if (TREE_CODE_CLASS (code) == tcc_unary)
>> {
>> ...
>> else if (COMPARISON_CLASS_P (arg0))
>> {
>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>> {
>> arg0 = copy_node (arg0);
>> TREE_TYPE (arg0) = type;
>> return arg0;
>> }
>> else if (TREE_CODE (type) != INTEGER_TYPE)
>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>> fold_build1_loc (loc, code, type,
>> integer_one_node),
>> fold_build1_loc (loc, code, type,
>> integer_zero_node));
>> }
>>
>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>> in which case they should be dropped or moved below where we
>> handle conversions explicitly.
>>
>> That said - does anyone remember anything about that above code?
>> Trying to do some svn blame history tracking now ...
>
> Oh, the patch continues...
>
> @@ -3208,7 +3208,10 @@ 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))
> + || !(TREE_CODE (type) == BOOLEAN_TYPE
> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
> {
>
> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
> that.
>
> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
> case TRUTH_NOT_EXPR:
> /* We require two-valued operand types. */
> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
> || (INTEGRAL_TYPE_P (rhs1_type)
> && TYPE_PRECISION (rhs1_type) == 1)))
> {
>
> likewise.
Well, those checks are necessary for Ada and its crippled
boolean_type_node and computed boolean-based integer construct. Ada
derives here the boolean-type to an integer with range 0..1 and the
only way to find out that it is in fact such a beast is by looking
into TREE_TYPE of type. See here Ada's code for getting base-type
information.
As such things are treated as compatible they can appear for TRUTH_NOT
expressions and comparisons.
Regards,
Kai
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:52 ` Kai Tietz
@ 2011-05-26 12:04 ` Richard Guenther
2011-05-26 12:05 ` Kai Tietz
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 12:04 UTC (permalink / raw)
To: Kai Tietz; +Cc: Kai Tietz, gcc-patches
On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>> Hello,
>>>>
>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>
>>>> ChangeLog
>>>>
>>>> 2011-05-26 Kai Tietz
>>>>
>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>> expressions.
>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>> boolean-type special.
>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>> or compatible types.
>>>> (verify_gimple_assign_unary): Likewise.
>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>> boolean case special.
>>>>
>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>
>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>> all of the FAILs you specify. Comments on the patch:
>>>
>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>> plain wrong if bitfields are involved. */
>>> {
>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>> + tree org_type = TREE_TYPE (*expr_p);
>>> +
>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>> + {
>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>> + ret = GS_OK;
>>> + goto dont_recalculate;
>>> + }
>>>
>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>> the strange dont_recalcuate goto can go away as well.
>>>
>>> if (!AGGREGATE_TYPE_P (type))
>>> - goto expr_2;
>>> + {
>>> + enum gimplify_status r0, r1;
>>> +
>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>> + post_p, is_gimple_val, fb_rvalue);
>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>> + post_p, is_gimple_val, fb_rvalue);
>>> +
>>> + ret = MIN (r0, r1);
>>> + }
>>> +
>>>
>>> why change this?
>>>
>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>> }
>>> else if (COMPARISON_CLASS_P (arg0))
>>> {
>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>> + Otherwise this will lead to race-condition on gimplification
>>> + trying to boolify comparison expression. */
>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>> + return NULL_TREE;
>>> +
>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>> {
>>> arg0 = copy_node (arg0);
>>>
>>> The code leading here looks quite strange to me ...
>>>
>>> tree
>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>> {
>>> ...
>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>> {
>>> ...
>>> else if (COMPARISON_CLASS_P (arg0))
>>> {
>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>> {
>>> arg0 = copy_node (arg0);
>>> TREE_TYPE (arg0) = type;
>>> return arg0;
>>> }
>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>> fold_build1_loc (loc, code, type,
>>> integer_one_node),
>>> fold_build1_loc (loc, code, type,
>>> integer_zero_node));
>>> }
>>>
>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>> in which case they should be dropped or moved below where we
>>> handle conversions explicitly.
>>>
>>> That said - does anyone remember anything about that above code?
>>> Trying to do some svn blame history tracking now ...
>>
>> Oh, the patch continues...
>>
>> @@ -3208,7 +3208,10 @@ 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))
>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>> {
>>
>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>> that.
>>
>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>> case TRUTH_NOT_EXPR:
>> /* We require two-valued operand types. */
>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>> || (INTEGRAL_TYPE_P (rhs1_type)
>> && TYPE_PRECISION (rhs1_type) == 1)))
>> {
>>
>> likewise.
>
> Well, those checks are necessary for Ada and its crippled
> boolean_type_node and computed boolean-based integer construct. Ada
> derives here the boolean-type to an integer with range 0..1 and the
> only way to find out that it is in fact such a beast is by looking
> into TREE_TYPE of type. See here Ada's code for getting base-type
> information.
> As such things are treated as compatible they can appear for TRUTH_NOT
> expressions and comparisons.
No they can't as we boolify them.
Richard.
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 12:04 ` Richard Guenther
@ 2011-05-26 12:05 ` Kai Tietz
2011-05-26 12:05 ` Richard Guenther
0 siblings, 1 reply; 14+ messages in thread
From: Kai Tietz @ 2011-05-26 12:05 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>>> Hello,
>>>>>
>>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>>
>>>>> ChangeLog
>>>>>
>>>>> 2011-05-26 Kai Tietz
>>>>>
>>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>>> expressions.
>>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>>> boolean-type special.
>>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>>> or compatible types.
>>>>> (verify_gimple_assign_unary): Likewise.
>>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>>> boolean case special.
>>>>>
>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>>
>>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>>> all of the FAILs you specify. Comments on the patch:
>>>>
>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>> plain wrong if bitfields are involved. */
>>>> {
>>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>> + tree org_type = TREE_TYPE (*expr_p);
>>>> +
>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>> + {
>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>>> + ret = GS_OK;
>>>> + goto dont_recalculate;
>>>> + }
>>>>
>>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>>> the strange dont_recalcuate goto can go away as well.
>>>>
>>>> if (!AGGREGATE_TYPE_P (type))
>>>> - goto expr_2;
>>>> + {
>>>> + enum gimplify_status r0, r1;
>>>> +
>>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>>> + post_p, is_gimple_val, fb_rvalue);
>>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>>> + post_p, is_gimple_val, fb_rvalue);
>>>> +
>>>> + ret = MIN (r0, r1);
>>>> + }
>>>> +
>>>>
>>>> why change this?
>>>>
>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>>> }
>>>> else if (COMPARISON_CLASS_P (arg0))
>>>> {
>>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>>> + Otherwise this will lead to race-condition on gimplification
>>>> + trying to boolify comparison expression. */
>>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>>> + return NULL_TREE;
>>>> +
>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>> {
>>>> arg0 = copy_node (arg0);
>>>>
>>>> The code leading here looks quite strange to me ...
>>>>
>>>> tree
>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>>> {
>>>> ...
>>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>>> {
>>>> ...
>>>> else if (COMPARISON_CLASS_P (arg0))
>>>> {
>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>> {
>>>> arg0 = copy_node (arg0);
>>>> TREE_TYPE (arg0) = type;
>>>> return arg0;
>>>> }
>>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>>> fold_build1_loc (loc, code, type,
>>>> integer_one_node),
>>>> fold_build1_loc (loc, code, type,
>>>> integer_zero_node));
>>>> }
>>>>
>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>>> in which case they should be dropped or moved below where we
>>>> handle conversions explicitly.
>>>>
>>>> That said - does anyone remember anything about that above code?
>>>> Trying to do some svn blame history tracking now ...
>>>
>>> Oh, the patch continues...
>>>
>>> @@ -3208,7 +3208,10 @@ 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))
>>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>>> {
>>>
>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>>> that.
>>>
>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>>> case TRUTH_NOT_EXPR:
>>> /* We require two-valued operand types. */
>>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>>> || (INTEGRAL_TYPE_P (rhs1_type)
>>> && TYPE_PRECISION (rhs1_type) == 1)))
>>> {
>>>
>>> likewise.
>>
>> Well, those checks are necessary for Ada and its crippled
>> boolean_type_node and computed boolean-based integer construct. Ada
>> derives here the boolean-type to an integer with range 0..1 and the
>> only way to find out that it is in fact such a beast is by looking
>> into TREE_TYPE of type. See here Ada's code for getting base-type
>> information.
>> As such things are treated as compatible they can appear for TRUTH_NOT
>> expressions and comparisons.
>
> No they can't as we boolify them.
>
> Richard.
Well, they do. Test can prove this by running fortran's dg tests.
The logic we do in gimplification is:
1) save original type of expression.
2) set expression's type to boolean_type_node.
3) if conversion from boolean_type_node to saved type isn't useless
(and here we have the issue about Fortran's different booleans with
different modes, which make those kinds not useless) then cast
expression's result to original type and retry gimplification.
4) Otherwise gimplify operands and continue.
Regards,
Kai
Kai
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 12:05 ` Kai Tietz
@ 2011-05-26 12:05 ` Richard Guenther
2011-05-26 12:06 ` Kai Tietz
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 12:05 UTC (permalink / raw)
To: Kai Tietz; +Cc: Kai Tietz, gcc-patches
On Thu, May 26, 2011 at 1:20 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>>>> Hello,
>>>>>>
>>>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>>>
>>>>>> ChangeLog
>>>>>>
>>>>>> 2011-05-26 Kai Tietz
>>>>>>
>>>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>>>> expressions.
>>>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>>>> boolean-type special.
>>>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>>>> or compatible types.
>>>>>> (verify_gimple_assign_unary): Likewise.
>>>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>>>> boolean case special.
>>>>>>
>>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>>>
>>>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>>>> all of the FAILs you specify. Comments on the patch:
>>>>>
>>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>> plain wrong if bitfields are involved. */
>>>>> {
>>>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>> + tree org_type = TREE_TYPE (*expr_p);
>>>>> +
>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>> + {
>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>>>> + ret = GS_OK;
>>>>> + goto dont_recalculate;
>>>>> + }
>>>>>
>>>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>>>> the strange dont_recalcuate goto can go away as well.
>>>>>
>>>>> if (!AGGREGATE_TYPE_P (type))
>>>>> - goto expr_2;
>>>>> + {
>>>>> + enum gimplify_status r0, r1;
>>>>> +
>>>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>> +
>>>>> + ret = MIN (r0, r1);
>>>>> + }
>>>>> +
>>>>>
>>>>> why change this?
>>>>>
>>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>>>> }
>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>> {
>>>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>>>> + Otherwise this will lead to race-condition on gimplification
>>>>> + trying to boolify comparison expression. */
>>>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>>>> + return NULL_TREE;
>>>>> +
>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>> {
>>>>> arg0 = copy_node (arg0);
>>>>>
>>>>> The code leading here looks quite strange to me ...
>>>>>
>>>>> tree
>>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>>>> {
>>>>> ...
>>>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>>>> {
>>>>> ...
>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>> {
>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>> {
>>>>> arg0 = copy_node (arg0);
>>>>> TREE_TYPE (arg0) = type;
>>>>> return arg0;
>>>>> }
>>>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>>>> fold_build1_loc (loc, code, type,
>>>>> integer_one_node),
>>>>> fold_build1_loc (loc, code, type,
>>>>> integer_zero_node));
>>>>> }
>>>>>
>>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>>>> in which case they should be dropped or moved below where we
>>>>> handle conversions explicitly.
>>>>>
>>>>> That said - does anyone remember anything about that above code?
>>>>> Trying to do some svn blame history tracking now ...
>>>>
>>>> Oh, the patch continues...
>>>>
>>>> @@ -3208,7 +3208,10 @@ 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))
>>>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>>>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>>>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>>>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>>>> {
>>>>
>>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>>>> that.
>>>>
>>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>>>> case TRUTH_NOT_EXPR:
>>>> /* We require two-valued operand types. */
>>>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>>>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>>>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>>>> || (INTEGRAL_TYPE_P (rhs1_type)
>>>> && TYPE_PRECISION (rhs1_type) == 1)))
>>>> {
>>>>
>>>> likewise.
>>>
>>> Well, those checks are necessary for Ada and its crippled
>>> boolean_type_node and computed boolean-based integer construct. Ada
>>> derives here the boolean-type to an integer with range 0..1 and the
>>> only way to find out that it is in fact such a beast is by looking
>>> into TREE_TYPE of type. See here Ada's code for getting base-type
>>> information.
>>> As such things are treated as compatible they can appear for TRUTH_NOT
>>> expressions and comparisons.
>>
>> No they can't as we boolify them.
>>
>> Richard.
>
> Well, they do. Test can prove this by running fortran's dg tests.
>
> The logic we do in gimplification is:
>
> 1) save original type of expression.
> 2) set expression's type to boolean_type_node.
> 3) if conversion from boolean_type_node to saved type isn't useless
> (and here we have the issue about Fortran's different booleans with
> different modes, which make those kinds not useless) then cast
> expression's result to original type and retry gimplification.
> 4) Otherwise gimplify operands and continue.
You don't make sense. Btw,
>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>> + {
>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>> + *expr_p = fold_convert_loc (saved_location, org_type,
is bogus it should be
TREE_TYPE (*expr_p) = boolean_type_node;
if (!useless_type_conversion_p (org_type, boolean_type_node))
{
*expr_p = fold_convert_loc (saved_location, org_type,
...
> Regards,
> Kai
>
> Kai
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 12:05 ` Richard Guenther
@ 2011-05-26 12:06 ` Kai Tietz
2011-05-26 12:18 ` Richard Guenther
0 siblings, 1 reply; 14+ messages in thread
From: Kai Tietz @ 2011-05-26 12:06 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 26, 2011 at 1:20 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>>>>
>>>>>>> ChangeLog
>>>>>>>
>>>>>>> 2011-05-26 Kai Tietz
>>>>>>>
>>>>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>>>>> expressions.
>>>>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>>>>> boolean-type special.
>>>>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>>>>> or compatible types.
>>>>>>> (verify_gimple_assign_unary): Likewise.
>>>>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>>>>> boolean case special.
>>>>>>>
>>>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>>>>
>>>>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>>>>> all of the FAILs you specify. Comments on the patch:
>>>>>>
>>>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>> plain wrong if bitfields are involved. */
>>>>>> {
>>>>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>> + tree org_type = TREE_TYPE (*expr_p);
>>>>>> +
>>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>>> + {
>>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>>>>> + ret = GS_OK;
>>>>>> + goto dont_recalculate;
>>>>>> + }
>>>>>>
>>>>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>>>>> the strange dont_recalcuate goto can go away as well.
>>>>>>
>>>>>> if (!AGGREGATE_TYPE_P (type))
>>>>>> - goto expr_2;
>>>>>> + {
>>>>>> + enum gimplify_status r0, r1;
>>>>>> +
>>>>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>> +
>>>>>> + ret = MIN (r0, r1);
>>>>>> + }
>>>>>> +
>>>>>>
>>>>>> why change this?
>>>>>>
>>>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>>>>> }
>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>> {
>>>>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>>>>> + Otherwise this will lead to race-condition on gimplification
>>>>>> + trying to boolify comparison expression. */
>>>>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>>>>> + return NULL_TREE;
>>>>>> +
>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>> {
>>>>>> arg0 = copy_node (arg0);
>>>>>>
>>>>>> The code leading here looks quite strange to me ...
>>>>>>
>>>>>> tree
>>>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>>>>> {
>>>>>> ...
>>>>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>>>>> {
>>>>>> ...
>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>> {
>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>> {
>>>>>> arg0 = copy_node (arg0);
>>>>>> TREE_TYPE (arg0) = type;
>>>>>> return arg0;
>>>>>> }
>>>>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>>>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>>>>> fold_build1_loc (loc, code, type,
>>>>>> integer_one_node),
>>>>>> fold_build1_loc (loc, code, type,
>>>>>> integer_zero_node));
>>>>>> }
>>>>>>
>>>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>>>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>>>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>>>>> in which case they should be dropped or moved below where we
>>>>>> handle conversions explicitly.
>>>>>>
>>>>>> That said - does anyone remember anything about that above code?
>>>>>> Trying to do some svn blame history tracking now ...
>>>>>
>>>>> Oh, the patch continues...
>>>>>
>>>>> @@ -3208,7 +3208,10 @@ 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))
>>>>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>>>>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>>>>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>>>>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>>>>> {
>>>>>
>>>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>>>>> that.
>>>>>
>>>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>>>>> case TRUTH_NOT_EXPR:
>>>>> /* We require two-valued operand types. */
>>>>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>>>>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>>>>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>>>>> || (INTEGRAL_TYPE_P (rhs1_type)
>>>>> && TYPE_PRECISION (rhs1_type) == 1)))
>>>>> {
>>>>>
>>>>> likewise.
>>>>
>>>> Well, those checks are necessary for Ada and its crippled
>>>> boolean_type_node and computed boolean-based integer construct. Ada
>>>> derives here the boolean-type to an integer with range 0..1 and the
>>>> only way to find out that it is in fact such a beast is by looking
>>>> into TREE_TYPE of type. See here Ada's code for getting base-type
>>>> information.
>>>> As such things are treated as compatible they can appear for TRUTH_NOT
>>>> expressions and comparisons.
>>>
>>> No they can't as we boolify them.
>>>
>>> Richard.
>>
>> Well, they do. Test can prove this by running fortran's dg tests.
>>
>> The logic we do in gimplification is:
>>
>> 1) save original type of expression.
>> 2) set expression's type to boolean_type_node.
>> 3) if conversion from boolean_type_node to saved type isn't useless
>> (and here we have the issue about Fortran's different booleans with
>> different modes, which make those kinds not useless) then cast
>> expression's result to original type and retry gimplification.
>> 4) Otherwise gimplify operands and continue.
>
> You don't make sense. Btw,
>
>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>> + {
>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>> + *expr_p = fold_convert_loc (saved_location, org_type,
>
> is bogus it should be
>
> TREE_TYPE (*expr_p) = boolean_type_node;
> if (!useless_type_conversion_p (org_type, boolean_type_node))
> {
> *expr_p = fold_convert_loc (saved_location, org_type,
Well, I spared here the type conversion to boolean_type_node for
useless conversion, but it doesn't change anything about the endless
recursion here. As long as fold_convert_loc modifies *expr_p's type
back to original type it will loop for ever for those cases I
described above.
Please don't forget that boolean_type_node is FE type here and not the
later 1-bit BOOL_SIZE type set in free_lang_data.
Kai
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 12:06 ` Kai Tietz
@ 2011-05-26 12:18 ` Richard Guenther
2011-05-26 12:23 ` Kai Tietz
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 12:18 UTC (permalink / raw)
To: Kai Tietz; +Cc: Kai Tietz, gcc-patches
On Thu, May 26, 2011 at 1:28 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, May 26, 2011 at 1:20 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>>>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>>>>>
>>>>>>>> ChangeLog
>>>>>>>>
>>>>>>>> 2011-05-26 Kai Tietz
>>>>>>>>
>>>>>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>>>>>> expressions.
>>>>>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>>>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>>>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>>>>>> boolean-type special.
>>>>>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>>>>>> or compatible types.
>>>>>>>> (verify_gimple_assign_unary): Likewise.
>>>>>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>>>>>> boolean case special.
>>>>>>>>
>>>>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>>>>>
>>>>>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>>>>>> all of the FAILs you specify. Comments on the patch:
>>>>>>>
>>>>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>> plain wrong if bitfields are involved. */
>>>>>>> {
>>>>>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>> + tree org_type = TREE_TYPE (*expr_p);
>>>>>>> +
>>>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>>>> + {
>>>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>>>>>> + ret = GS_OK;
>>>>>>> + goto dont_recalculate;
>>>>>>> + }
>>>>>>>
>>>>>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>>>>>> the strange dont_recalcuate goto can go away as well.
>>>>>>>
>>>>>>> if (!AGGREGATE_TYPE_P (type))
>>>>>>> - goto expr_2;
>>>>>>> + {
>>>>>>> + enum gimplify_status r0, r1;
>>>>>>> +
>>>>>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>>> +
>>>>>>> + ret = MIN (r0, r1);
>>>>>>> + }
>>>>>>> +
>>>>>>>
>>>>>>> why change this?
>>>>>>>
>>>>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>> }
>>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>>> {
>>>>>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>>>>>> + Otherwise this will lead to race-condition on gimplification
>>>>>>> + trying to boolify comparison expression. */
>>>>>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>>>>>> + return NULL_TREE;
>>>>>>> +
>>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>> {
>>>>>>> arg0 = copy_node (arg0);
>>>>>>>
>>>>>>> The code leading here looks quite strange to me ...
>>>>>>>
>>>>>>> tree
>>>>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>>>>>> {
>>>>>>> ...
>>>>>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>>>>>> {
>>>>>>> ...
>>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>>> {
>>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>> {
>>>>>>> arg0 = copy_node (arg0);
>>>>>>> TREE_TYPE (arg0) = type;
>>>>>>> return arg0;
>>>>>>> }
>>>>>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>>>>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>>>>>> fold_build1_loc (loc, code, type,
>>>>>>> integer_one_node),
>>>>>>> fold_build1_loc (loc, code, type,
>>>>>>> integer_zero_node));
>>>>>>> }
>>>>>>>
>>>>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>>>>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>>>>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>>>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>>>>>> in which case they should be dropped or moved below where we
>>>>>>> handle conversions explicitly.
>>>>>>>
>>>>>>> That said - does anyone remember anything about that above code?
>>>>>>> Trying to do some svn blame history tracking now ...
>>>>>>
>>>>>> Oh, the patch continues...
>>>>>>
>>>>>> @@ -3208,7 +3208,10 @@ 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))
>>>>>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>>>>>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>>>>>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>>>>>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>>>>>> {
>>>>>>
>>>>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>>>>>> that.
>>>>>>
>>>>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>>>>>> case TRUTH_NOT_EXPR:
>>>>>> /* We require two-valued operand types. */
>>>>>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>>>>>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>>>>>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>>>>>> || (INTEGRAL_TYPE_P (rhs1_type)
>>>>>> && TYPE_PRECISION (rhs1_type) == 1)))
>>>>>> {
>>>>>>
>>>>>> likewise.
>>>>>
>>>>> Well, those checks are necessary for Ada and its crippled
>>>>> boolean_type_node and computed boolean-based integer construct. Ada
>>>>> derives here the boolean-type to an integer with range 0..1 and the
>>>>> only way to find out that it is in fact such a beast is by looking
>>>>> into TREE_TYPE of type. See here Ada's code for getting base-type
>>>>> information.
>>>>> As such things are treated as compatible they can appear for TRUTH_NOT
>>>>> expressions and comparisons.
>>>>
>>>> No they can't as we boolify them.
>>>>
>>>> Richard.
>>>
>>> Well, they do. Test can prove this by running fortran's dg tests.
>>>
>>> The logic we do in gimplification is:
>>>
>>> 1) save original type of expression.
>>> 2) set expression's type to boolean_type_node.
>>> 3) if conversion from boolean_type_node to saved type isn't useless
>>> (and here we have the issue about Fortran's different booleans with
>>> different modes, which make those kinds not useless) then cast
>>> expression's result to original type and retry gimplification.
>>> 4) Otherwise gimplify operands and continue.
>>
>> You don't make sense. Btw,
>>
>>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>>> + {
>>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>>> + *expr_p = fold_convert_loc (saved_location, org_type,
>>
>> is bogus it should be
>>
>> TREE_TYPE (*expr_p) = boolean_type_node;
>> if (!useless_type_conversion_p (org_type, boolean_type_node))
>> {
>> *expr_p = fold_convert_loc (saved_location, org_type,
>
> Well, I spared here the type conversion to boolean_type_node for
> useless conversion,
useless_type_conversion_p isn't symmetric, so you can't do that.
> but it doesn't change anything about the endless
> recursion here. As long as fold_convert_loc modifies *expr_p's type
> back to original type it will loop for ever for those cases I
> described above.
Indeed which is why we also touch fold-const.c!
> Please don't forget that boolean_type_node is FE type here and not the
> later 1-bit BOOL_SIZE type set in free_lang_data.
I perfectly know that. Still no valid reason.
Richard.
> Kai
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 12:18 ` Richard Guenther
@ 2011-05-26 12:23 ` Kai Tietz
0 siblings, 0 replies; 14+ messages in thread
From: Kai Tietz @ 2011-05-26 12:23 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 26, 2011 at 1:28 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>> On Thu, May 26, 2011 at 1:20 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>>> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>>>>>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>>>>>>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>>>>>>>
>>>>>>>>> ChangeLog
>>>>>>>>>
>>>>>>>>> 2011-05-26 Kai Tietz
>>>>>>>>>
>>>>>>>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>>>>>>>> expressions.
>>>>>>>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>>>>>>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>>>>>>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>>>>>>>> boolean-type special.
>>>>>>>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>>>>>>>> or compatible types.
>>>>>>>>> (verify_gimple_assign_unary): Likewise.
>>>>>>>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>>>>>>>> boolean case special.
>>>>>>>>>
>>>>>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>>>>>>>
>>>>>>>> It obviously isn't ok to apply before a patch has gone in that will resolve
>>>>>>>> all of the FAILs you specify. Comments on the patch:
>>>>>>>>
>>>>>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>> plain wrong if bitfields are involved. */
>>>>>>>> {
>>>>>>>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>>> + tree org_type = TREE_TYPE (*expr_p);
>>>>>>>> +
>>>>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>>>>> + {
>>>>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>>>>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>>>>>>>> + ret = GS_OK;
>>>>>>>> + goto dont_recalculate;
>>>>>>>> + }
>>>>>>>>
>>>>>>>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>>>>>>>> the strange dont_recalcuate goto can go away as well.
>>>>>>>>
>>>>>>>> if (!AGGREGATE_TYPE_P (type))
>>>>>>>> - goto expr_2;
>>>>>>>> + {
>>>>>>>> + enum gimplify_status r0, r1;
>>>>>>>> +
>>>>>>>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>>>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>>>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>>>>>>>> + post_p, is_gimple_val, fb_rvalue);
>>>>>>>> +
>>>>>>>> + ret = MIN (r0, r1);
>>>>>>>> + }
>>>>>>>> +
>>>>>>>>
>>>>>>>> why change this?
>>>>>>>>
>>>>>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>>> }
>>>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>>>> {
>>>>>>>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>>>>>>>> + Otherwise this will lead to race-condition on gimplification
>>>>>>>> + trying to boolify comparison expression. */
>>>>>>>> + if (TREE_TYPE (op0) == boolean_type_node)
>>>>>>>> + return NULL_TREE;
>>>>>>>> +
>>>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>> {
>>>>>>>> arg0 = copy_node (arg0);
>>>>>>>>
>>>>>>>> The code leading here looks quite strange to me ...
>>>>>>>>
>>>>>>>> tree
>>>>>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>>>>>>>> {
>>>>>>>> ...
>>>>>>>> if (TREE_CODE_CLASS (code) == tcc_unary)
>>>>>>>> {
>>>>>>>> ...
>>>>>>>> else if (COMPARISON_CLASS_P (arg0))
>>>>>>>> {
>>>>>>>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>> {
>>>>>>>> arg0 = copy_node (arg0);
>>>>>>>> TREE_TYPE (arg0) = type;
>>>>>>>> return arg0;
>>>>>>>> }
>>>>>>>> else if (TREE_CODE (type) != INTEGER_TYPE)
>>>>>>>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>>>>>>>> fold_build1_loc (loc, code, type,
>>>>>>>> integer_one_node),
>>>>>>>> fold_build1_loc (loc, code, type,
>>>>>>>> integer_zero_node));
>>>>>>>> }
>>>>>>>>
>>>>>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>>>>>>>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>>>>>>>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>>>>>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>>>>>>>> in which case they should be dropped or moved below where we
>>>>>>>> handle conversions explicitly.
>>>>>>>>
>>>>>>>> That said - does anyone remember anything about that above code?
>>>>>>>> Trying to do some svn blame history tracking now ...
>>>>>>>
>>>>>>> Oh, the patch continues...
>>>>>>>
>>>>>>> @@ -3208,7 +3208,10 @@ 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))
>>>>>>> + || !(TREE_CODE (type) == BOOLEAN_TYPE
>>>>>>> + || (TREE_TYPE (type) && TREE_CODE (type) == INTEGER_TYPE
>>>>>>> + && TREE_CODE (TREE_TYPE (type)) == BOOLEAN_TYPE)
>>>>>>> + || (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (type) == 1)))
>>>>>>> {
>>>>>>>
>>>>>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? Drop
>>>>>>> that.
>>>>>>>
>>>>>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt)
>>>>>>> case TRUTH_NOT_EXPR:
>>>>>>> /* We require two-valued operand types. */
>>>>>>> if (!(TREE_CODE (rhs1_type) == BOOLEAN_TYPE
>>>>>>> + || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_type) == INTEGER_TYPE
>>>>>>> + && TREE_CODE (TREE_TYPE (rhs1_type)) == BOOLEAN_TYPE)
>>>>>>> || (INTEGRAL_TYPE_P (rhs1_type)
>>>>>>> && TYPE_PRECISION (rhs1_type) == 1)))
>>>>>>> {
>>>>>>>
>>>>>>> likewise.
>>>>>>
>>>>>> Well, those checks are necessary for Ada and its crippled
>>>>>> boolean_type_node and computed boolean-based integer construct. Ada
>>>>>> derives here the boolean-type to an integer with range 0..1 and the
>>>>>> only way to find out that it is in fact such a beast is by looking
>>>>>> into TREE_TYPE of type. See here Ada's code for getting base-type
>>>>>> information.
>>>>>> As such things are treated as compatible they can appear for TRUTH_NOT
>>>>>> expressions and comparisons.
>>>>>
>>>>> No they can't as we boolify them.
>>>>>
>>>>> Richard.
>>>>
>>>> Well, they do. Test can prove this by running fortran's dg tests.
>>>>
>>>> The logic we do in gimplification is:
>>>>
>>>> 1) save original type of expression.
>>>> 2) set expression's type to boolean_type_node.
>>>> 3) if conversion from boolean_type_node to saved type isn't useless
>>>> (and here we have the issue about Fortran's different booleans with
>>>> different modes, which make those kinds not useless) then cast
>>>> expression's result to original type and retry gimplification.
>>>> 4) Otherwise gimplify operands and continue.
>>>
>>> You don't make sense. Btw,
>>>
>>>>>>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>>>>>>> + {
>>>>>>> + TREE_TYPE (*expr_p) = boolean_type_node;
>>>>>>> + *expr_p = fold_convert_loc (saved_location, org_type,
>>>
>>> is bogus it should be
>>>
>>> TREE_TYPE (*expr_p) = boolean_type_node;
>>> if (!useless_type_conversion_p (org_type, boolean_type_node))
>>> {
>>> *expr_p = fold_convert_loc (saved_location, org_type,
>>
>> Well, I spared here the type conversion to boolean_type_node for
>> useless conversion,
>
> useless_type_conversion_p isn't symmetric, so you can't do that.
Right, I am aware of that. I will move in updated patch the setting
boolean before the if here. As type conversion for the cast back to
original-type via fold_convert_loc is only of interest AFAICS, when
this cast isn't useless (means outter is org_type, and inner is
boolean_type_node). But well, we can replace the if here to the check
against org_type != boolean_type_node, if you prefer this.
Kai
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:12 ` Richard Guenther
2011-05-26 11:20 ` Richard Guenther
@ 2011-05-26 11:30 ` Richard Guenther
2011-06-06 16:10 ` Diego Novillo
2011-05-26 11:39 ` Kai Tietz
2 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 11:30 UTC (permalink / raw)
To: Kai Tietz; +Cc: gcc-patches, Diego Novillo
On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>> Hello,
>>
>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>
>> ChangeLog
>>
>> 2011-05-26 Kai Tietz
>>
>> * gimplify.c (gimple_boolify): Boolify all comparison
>> expressions.
>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>> boolean-type special.
>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>> or compatible types.
>> (verify_gimple_assign_unary): Likewise.
>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>> boolean case special.
>>
>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>
> It obviously isn't ok to apply before a patch has gone in that will resolve
> all of the FAILs you specify. Comments on the patch:
>
> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
> plain wrong if bitfields are involved. */
> {
> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
> + tree org_type = TREE_TYPE (*expr_p);
> +
> + if (!useless_type_conversion_p (org_type, boolean_type_node))
> + {
> + TREE_TYPE (*expr_p) = boolean_type_node;
> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
> + ret = GS_OK;
> + goto dont_recalculate;
> + }
>
> The above should be only done for !AGGREGATE_TYPE_P. Probably then
> the strange dont_recalcuate goto can go away as well.
>
> if (!AGGREGATE_TYPE_P (type))
> - goto expr_2;
> + {
> + enum gimplify_status r0, r1;
> +
> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> +
> + ret = MIN (r0, r1);
> + }
> +
>
> why change this?
>
> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
> }
> else if (COMPARISON_CLASS_P (arg0))
> {
> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
> + Otherwise this will lead to race-condition on gimplification
> + trying to boolify comparison expression. */
> + if (TREE_TYPE (op0) == boolean_type_node)
> + return NULL_TREE;
> +
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
>
> The code leading here looks quite strange to me ...
>
> tree
> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
> {
> ...
> if (TREE_CODE_CLASS (code) == tcc_unary)
> {
> ...
> else if (COMPARISON_CLASS_P (arg0))
> {
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
> TREE_TYPE (arg0) = type;
> return arg0;
> }
> else if (TREE_CODE (type) != INTEGER_TYPE)
> return fold_build3_loc (loc, COND_EXPR, type, arg0,
> fold_build1_loc (loc, code, type,
> integer_one_node),
> fold_build1_loc (loc, code, type,
> integer_zero_node));
> }
>
> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
> in which case they should be dropped or moved below where we
> handle conversions explicitly.
>
> That said - does anyone remember anything about that above code?
> Trying to do some svn blame history tracking now ...
Bah ...
330 rms if (TREE_CODE_CLASS (code) == '1')
330 rms {
...
330 rms else if (TREE_CODE_CLASS (TREE_CODE (arg0)) == '<')
330 rms return fold (build (COND_EXPR, type, arg0,
330 rms fold (build1 (code, type, integer_on
e_node)),
330 rms fold (build1 (code, type, integer_ze
ro_node))));
and the other case is from
81764 dnovillo {
81764 dnovillo if (TREE_CODE (type) == BOOLEAN_TYPE)
81764 dnovillo {
81764 dnovillo arg0 = copy_node (arg0);
81764 dnovillo TREE_TYPE (arg0) = type;
81764 dnovillo return arg0;
81764 dnovillo }
(famous tree-ssa merge revision).
So - lets ask who still is around. Diego? ;)
Richard.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:30 ` Richard Guenther
@ 2011-06-06 16:10 ` Diego Novillo
0 siblings, 0 replies; 14+ messages in thread
From: Diego Novillo @ 2011-06-06 16:10 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
On Thu, May 26, 2011 at 07:02, Richard Guenther
<richard.guenther@gmail.com> wrote:
> 81764 dnovillo {
> 81764 dnovillo if (TREE_CODE (type) == BOOLEAN_TYPE)
> 81764 dnovillo {
> 81764 dnovillo arg0 = copy_node (arg0);
> 81764 dnovillo TREE_TYPE (arg0) = type;
> 81764 dnovillo return arg0;
> 81764 dnovillo }
>
> (famous tree-ssa merge revision).
>
> So - lets ask who still is around. Diego? ;)
I just saw this in my inbox, sorry. What's the question? Whether I
remember this code in fold? Not offhand. If it looks weird and/or is
not working right, feel free to toss it.
Diego.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:12 ` Richard Guenther
2011-05-26 11:20 ` Richard Guenther
2011-05-26 11:30 ` Richard Guenther
@ 2011-05-26 11:39 ` Kai Tietz
2011-05-26 12:00 ` Richard Guenther
2 siblings, 1 reply; 14+ messages in thread
From: Kai Tietz @ 2011-05-26 11:39 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, gcc-patches
2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>> Hello,
>>
>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>
>> ChangeLog
>>
>> 2011-05-26 Kai Tietz
>>
>> * gimplify.c (gimple_boolify): Boolify all comparison
>> expressions.
>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>> boolean-type special.
>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>> or compatible types.
>> (verify_gimple_assign_unary): Likewise.
>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>> boolean case special.
>>
>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>
> It obviously isn't ok to apply before a patch has gone in that will resolve
> all of the FAILs you specify. Comments on the patch:
>
> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
> plain wrong if bitfields are involved. */
> {
> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
> + tree org_type = TREE_TYPE (*expr_p);
> +
> + if (!useless_type_conversion_p (org_type, boolean_type_node))
> + {
> + TREE_TYPE (*expr_p) = boolean_type_node;
> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
> + ret = GS_OK;
> + goto dont_recalculate;
> + }
>
> The above should be only done for !AGGREGATE_TYPE_P. Probably then
> the strange dont_recalcuate goto can go away as well.
I thought so too, but boolification is required for all kind of
comparisons. This goto is a bit hacky, but the best way to prevent
here an useless recalculation. Without doing this, we get
bootstrap/regression test failures, as in tree-cfg we check that
comparison type is of boolean (or compatible kind).
> if (!AGGREGATE_TYPE_P (type))
> - goto expr_2;
> + {
> + enum gimplify_status r0, r1;
> +
> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
> + post_p, is_gimple_val, fb_rvalue);
> +
> + ret = MIN (r0, r1);
> + }
> +
>
> why change this?
This part can be reverted to the goto expr_2 again.
> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
> }
> else if (COMPARISON_CLASS_P (arg0))
> {
> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
> + Otherwise this will lead to race-condition on gimplification
> + trying to boolify comparison expression. */
> + if (TREE_TYPE (op0) == boolean_type_node)
> + return NULL_TREE;
> +
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
>
> The code leading here looks quite strange to me ...
Issue is that Fortran (and this is the cause for this
boolean_type_node check) has more then one boolean type. Gimplifier
normalizes to boolean_type_node. If now a different boolean-type
(with different mode) of Fortran is used, it leads to an endless-loop
in type conversion. An example boolean-kind(mode-size=1) and
boolean-kind(mode-size=2) are no useless type conversion. So by
gimplifier type gets set to boolean-kind(mode-size=1) (if this is
default boolean_type_node's definition), and then casted to
boolean-kind(mode-size=2) expression.
> tree
> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
> {
> ...
> if (TREE_CODE_CLASS (code) == tcc_unary)
> {
> ...
> else if (COMPARISON_CLASS_P (arg0))
> {
> if (TREE_CODE (type) == BOOLEAN_TYPE)
> {
> arg0 = copy_node (arg0);
> TREE_TYPE (arg0) = type;
> return arg0;
> }
> else if (TREE_CODE (type) != INTEGER_TYPE)
> return fold_build3_loc (loc, COND_EXPR, type, arg0,
> fold_build1_loc (loc, code, type,
> integer_one_node),
> fold_build1_loc (loc, code, type,
> integer_zero_node));
> }
>
> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
> in which case they should be dropped or moved below where we
> handle conversions explicitly.
>
> That said - does anyone remember anything about that above code?
> Trying to do some svn blame history tracking now ...
>
> Richard.
>
>> The following tests are failing due this change (what is to be expected here and needs some additional handling in forwprop).
>> FAIL: gcc.dg/binop-xor1.c scan-tree-dump-times optimized "<bb[^>]*>" 5
>> FAIL: gcc.dg/binop-xor1.c scan-tree-dump-times optimized "\^" 1
>> FAIL: gcc.dg/binop-xor3.c scan-tree-dump-times optimized "<bb[^>]*>" 1
>> FAIL: gcc.dg/binop-xor3.c scan-tree-dump-times optimized "\^" 1
>> XPASS: gcc.dg/tree-ssa/20030807-7.c scan-tree-dump-times vrp1 "if " 1
>> FAIL: gcc.dg/tree-ssa/builtin-expect-5.c scan-tree-dump forwprop1 "builtin_expect[^\n]*, 1\);\n[^\n]*if"
>> FAIL: gcc.dg/tree-ssa/pr21031.c scan-tree-dump-times forwprop1 "Replaced" 2
>> FAIL: gcc.dg/tree-ssa/pr30978.c scan-tree-dump optimized "e_. = a_..D. > 0;"
>> FAIL: gcc.dg/tree-ssa/ssa-fre-6.c scan-tree-dump-times fre1 "Replaced " 5
>> FAIL: gcc.dg/tree-ssa/vrp47.c scan-tree-dump-times dom1 "x[^ ]* & y" 1
>> FAIL: gcc.dg/tree-ssa/vrp47.c scan-tree-dump-times vrp1 "x[^ ]* \^ 1" 1
>> FAIL: gcc.dg/vect/pr49038.c execution test
>> FAIL: gcc.dg/vect/pr49038.c -flto execution test
>> FAIL: gcc.target/i386/andor-2.c scan-assembler-not sete
>>
>> The Ada, Obj-C, Obj-C++, C++, Fortran, and Java testsuite don't show any new regressions by this.
>>
>> Some failing testcases are simply caused by different folding behavior and producing simplier code. The binop-xor tests 1 and 3 might be better removed for now, or marked as being expect to fail. The cause for their failing is in doing tree-analysis via fold-const on gimplified trees, which now don't allow folding here to look through.
>> To illustrate required changes for other tests, I attach here some required changes for testsuite
>>
>> Index: gcc.dg/tree-ssa/pr30978.c
>> ===================================================================
>> --- gcc.dg/tree-ssa/pr30978.c (revision 174264)
>> +++ gcc.dg/tree-ssa/pr30978.c (working copy)
>> @@ -10,5 +10,5 @@
>> return e;
>> }
>>
>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;\n[^\n]*e_. = \\\(int\\\)" "optimized" } } */
>> /* { dg-final { cleanup-tree-dump "optimized" } } */
>>
>> Index: gcc.dg/tree-ssa/builtin-expect-5.c
>> ===================================================================
>> --- gcc.dg/tree-ssa/builtin-expect-5.c (revision 174264)
>> +++ gcc.dg/tree-ssa/builtin-expect-5.c (working copy)
>> @@ -11,5 +11,5 @@
>>
>> /* { 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 {builtin_expect[^\n]*, 1\);} "forwprop1"} } */
>> /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>
>> Index: gcc.dg/tree-ssa/pr21031.c
>> ===================================================================
>> --- gcc.dg/tree-ssa/pr21031.c (revision 174264)
>> +++ gcc.dg/tree-ssa/pr21031.c (working copy)
>> @@ -16,5 +16,5 @@
>> 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.dg/tree-ssa/ssa-fre-6.c
>> ===================================================================
>> --- gcc.dg/tree-ssa/ssa-fre-6.c (revision 174264)
>> +++ gcc.dg/tree-ssa/ssa-fre-6.c (working copy)
>> @@ -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" } } */
>>
>> Regards,
>> Kai
>>
Regards,
Kai
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification
2011-05-26 11:39 ` Kai Tietz
@ 2011-05-26 12:00 ` Richard Guenther
0 siblings, 0 replies; 14+ messages in thread
From: Richard Guenther @ 2011-05-26 12:00 UTC (permalink / raw)
To: Kai Tietz; +Cc: Kai Tietz, gcc-patches
On Thu, May 26, 2011 at 1:05 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/26 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <ktietz@redhat.com> wrote:
>>> Hello,
>>>
>>> this patch ensures that after gimplification also comparison expressions using FE's boolean_type_node. As we need to deal here with C/C++'s (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this patch alters some checks in tree-cfg for Ada's sake, and we need to deal in fold-const about type-conversion of comparisons special.
>>> Additionally it takes care that in forwprop pass we don't do type hoising for boolean types.
>>>
>>> ChangeLog
>>>
>>> 2011-05-26 Kai Tietz
>>>
>>> * gimplify.c (gimple_boolify): Boolify all comparison
>>> expressions.
>>> (gimplify_expr): Use 'useless_type_conversion_p' for comparing
>>> org_type with boolean_type_node for TRUTH-expressions and comparisons.
>>> * fold-const.c (fold_unary_loc): Handle comparison conversions with
>>> boolean-type special.
>>> * tree-cfg.c (verify_gimple_comparison): Adjust check for boolean
>>> or compatible types.
>>> (verify_gimple_assign_unary): Likewise.
>>> * tree-ssa-forwprop.c (forward_propagate_comparison): Handle
>>> boolean case special.
>>>
>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for all standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. Ok for apply?
>>
>> It obviously isn't ok to apply before a patch has gone in that will resolve
>> all of the FAILs you specify. Comments on the patch:
>>
>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq
>> plain wrong if bitfields are involved. */
>> {
>> tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>> + tree org_type = TREE_TYPE (*expr_p);
>> +
>> + if (!useless_type_conversion_p (org_type, boolean_type_node))
>> + {
>> + TREE_TYPE (*expr_p) = boolean_type_node;
>> + *expr_p = fold_convert_loc (saved_location, org_type, *expr_p);
>> + ret = GS_OK;
>> + goto dont_recalculate;
>> + }
>>
>> The above should be only done for !AGGREGATE_TYPE_P. Probably then
>> the strange dont_recalcuate goto can go away as well.
>
> I thought so too, but boolification is required for all kind of
> comparisons. This goto is a bit hacky, but the best way to prevent
> here an useless recalculation. Without doing this, we get
> bootstrap/regression test failures, as in tree-cfg we check that
> comparison type is of boolean (or compatible kind).
That doesn't make sense if you look at the other two cases which
both will return GS_OK and thus re-enter this. The aggregate
variable size path also will use memcmp which returns int,so
first boolifying doesn't make sense.
>> if (!AGGREGATE_TYPE_P (type))
>> - goto expr_2;
>> + {
>> + enum gimplify_status r0, r1;
>> +
>> + r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
>> + post_p, is_gimple_val, fb_rvalue);
>> + r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
>> + post_p, is_gimple_val, fb_rvalue);
>> +
>> + ret = MIN (r0, r1);
>> + }
>> +
>>
>> why change this?
>
> This part can be reverted to the goto expr_2 again.
>
>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre
>> }
>> else if (COMPARISON_CLASS_P (arg0))
>> {
>> + /* Don't optimize type change, if op0 is of kind boolean_type_node.
>> + Otherwise this will lead to race-condition on gimplification
>> + trying to boolify comparison expression. */
>> + if (TREE_TYPE (op0) == boolean_type_node)
>> + return NULL_TREE;
>> +
>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>> {
>> arg0 = copy_node (arg0);
>>
>> The code leading here looks quite strange to me ...
>
> Issue is that Fortran (and this is the cause for this
> boolean_type_node check) has more then one boolean type. Gimplifier
> normalizes to boolean_type_node. If now a different boolean-type
> (with different mode) of Fortran is used, it leads to an endless-loop
> in type conversion. An example boolean-kind(mode-size=1) and
> boolean-kind(mode-size=2) are no useless type conversion. So by
> gimplifier type gets set to boolean-kind(mode-size=1) (if this is
> default boolean_type_node's definition), and then casted to
> boolean-kind(mode-size=2) expression.
I said the whole code looks strange, it should be reworked instead
of addign this kind of other non-obviousness.
Richard.
>
>> tree
>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>> {
>> ...
>> if (TREE_CODE_CLASS (code) == tcc_unary)
>> {
>> ...
>> else if (COMPARISON_CLASS_P (arg0))
>> {
>> if (TREE_CODE (type) == BOOLEAN_TYPE)
>> {
>> arg0 = copy_node (arg0);
>> TREE_TYPE (arg0) = type;
>> return arg0;
>> }
>> else if (TREE_CODE (type) != INTEGER_TYPE)
>> return fold_build3_loc (loc, COND_EXPR, type, arg0,
>> fold_build1_loc (loc, code, type,
>> integer_one_node),
>> fold_build1_loc (loc, code, type,
>> integer_zero_node));
>> }
>>
>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE,
>> return arg0 ... sure. Same for the 2nd case. ~ (a == b) isn't
>> the same as a == b ? ~1 : ~0. I _suppose_ those cases were
>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary,
>> in which case they should be dropped or moved below where we
>> handle conversions explicitly.
>>
>> That said - does anyone remember anything about that above code?
>> Trying to do some svn blame history tracking now ...
>>
>> Richard.
>>
>>> The following tests are failing due this change (what is to be expected here and needs some additional handling in forwprop).
>>> FAIL: gcc.dg/binop-xor1.c scan-tree-dump-times optimized "<bb[^>]*>" 5
>>> FAIL: gcc.dg/binop-xor1.c scan-tree-dump-times optimized "\^" 1
>>> FAIL: gcc.dg/binop-xor3.c scan-tree-dump-times optimized "<bb[^>]*>" 1
>>> FAIL: gcc.dg/binop-xor3.c scan-tree-dump-times optimized "\^" 1
>>> XPASS: gcc.dg/tree-ssa/20030807-7.c scan-tree-dump-times vrp1 "if " 1
>>> FAIL: gcc.dg/tree-ssa/builtin-expect-5.c scan-tree-dump forwprop1 "builtin_expect[^\n]*, 1\);\n[^\n]*if"
>>> FAIL: gcc.dg/tree-ssa/pr21031.c scan-tree-dump-times forwprop1 "Replaced" 2
>>> FAIL: gcc.dg/tree-ssa/pr30978.c scan-tree-dump optimized "e_. = a_..D. > 0;"
>>> FAIL: gcc.dg/tree-ssa/ssa-fre-6.c scan-tree-dump-times fre1 "Replaced " 5
>>> FAIL: gcc.dg/tree-ssa/vrp47.c scan-tree-dump-times dom1 "x[^ ]* & y" 1
>>> FAIL: gcc.dg/tree-ssa/vrp47.c scan-tree-dump-times vrp1 "x[^ ]* \^ 1" 1
>>> FAIL: gcc.dg/vect/pr49038.c execution test
>>> FAIL: gcc.dg/vect/pr49038.c -flto execution test
>>> FAIL: gcc.target/i386/andor-2.c scan-assembler-not sete
>>>
>>> The Ada, Obj-C, Obj-C++, C++, Fortran, and Java testsuite don't show any new regressions by this.
>>>
>>> Some failing testcases are simply caused by different folding behavior and producing simplier code. The binop-xor tests 1 and 3 might be better removed for now, or marked as being expect to fail. The cause for their failing is in doing tree-analysis via fold-const on gimplified trees, which now don't allow folding here to look through.
>>> To illustrate required changes for other tests, I attach here some required changes for testsuite
>>>
>>> Index: gcc.dg/tree-ssa/pr30978.c
>>> ===================================================================
>>> --- gcc.dg/tree-ssa/pr30978.c (revision 174264)
>>> +++ gcc.dg/tree-ssa/pr30978.c (working copy)
>>> @@ -10,5 +10,5 @@
>>> return e;
>>> }
>>>
>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;\n[^\n]*e_. = \\\(int\\\)" "optimized" } } */
>>> /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>
>>> Index: gcc.dg/tree-ssa/builtin-expect-5.c
>>> ===================================================================
>>> --- gcc.dg/tree-ssa/builtin-expect-5.c (revision 174264)
>>> +++ gcc.dg/tree-ssa/builtin-expect-5.c (working copy)
>>> @@ -11,5 +11,5 @@
>>>
>>> /* { 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 {builtin_expect[^\n]*, 1\);} "forwprop1"} } */
>>> /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>
>>> Index: gcc.dg/tree-ssa/pr21031.c
>>> ===================================================================
>>> --- gcc.dg/tree-ssa/pr21031.c (revision 174264)
>>> +++ gcc.dg/tree-ssa/pr21031.c (working copy)
>>> @@ -16,5 +16,5 @@
>>> 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.dg/tree-ssa/ssa-fre-6.c
>>> ===================================================================
>>> --- gcc.dg/tree-ssa/ssa-fre-6.c (revision 174264)
>>> +++ gcc.dg/tree-ssa/ssa-fre-6.c (working copy)
>>> @@ -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" } } */
>>>
>>> Regards,
>>> Kai
>>>
>
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 14+ messages in thread