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.
> Regards,
> Kai
>
