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