2011/5/26 Richard Guenther <richard.guent...@gmail.com>:
> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>> 2011/5/26 Richard Guenther <richard.guent...@gmail.com>:
>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther
>>> <richard.guent...@gmail.com> wrote:
>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz <kti...@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
