On Wed, May 11, 2011 at 5:46 PM, Kai Tietz <[email protected]> wrote:
> 2011/5/11 Richard Guenther <[email protected]>:
>> The most important thing is to get predicate types sane - that affects
>> tcc_comparison codes and the TRUTH_* codes. After that, the TRUTH_*
>> codes are redundant with the BIT_* ones which already are always
>> validly typed. As fold already converts some TRUTH_* to BIT_* variants
>> we usually have a mix of both which is not handled very well by tree
>> optimizers.
>
> Well, to boolify comparisions isn't that hard at all, but I don't see
> here much improvement, as it will lead necessarily for uses without
> boolean-type always in gimple as '(type) comparison_tcc-ssa', which
> seems to me like trying to put the cart before the horse.
Well, it's of course a matter of consistency.
> So updated patch inlined (and attached). The type-casting for
> TRUTH_ANDIF/ORIF is still necessary (without it I get bootstrap
> failures due perfectly working gimple_cond_expr function, which is
> producing here happily "iftmp" variables assigning later on wrong
> type.
Hm, I see ...
> Regards,
> Kai
>
> Index: gcc/gcc/gimplify.c
> ===================================================================
> --- gcc.orig/gcc/gimplify.c 2011-05-11 17:03:24.853377700 +0200
> +++ gcc/gcc/gimplify.c 2011-05-11 17:11:02.018281300 +0200
> @@ -2824,9 +2824,6 @@ gimple_boolify (tree expr)
> }
> }
>
> - if (TREE_CODE (type) == BOOLEAN_TYPE)
> - return expr;
> -
> switch (TREE_CODE (expr))
> {
> case TRUTH_AND_EXPR:
> @@ -2851,6 +2848,8 @@ gimple_boolify (tree expr)
> default:
> /* Other expressions that get here must have boolean values, but
> might need to be converted to the appropriate mode. */
> + if (TREE_CODE (type) == BOOLEAN_TYPE)
> + return expr;
> return fold_convert_loc (loc, boolean_type_node, expr);
> }
> }
> @@ -6762,6 +6761,21 @@ gimplify_expr (tree *expr_p, gimple_seq
>
> case TRUTH_ANDIF_EXPR:
> case TRUTH_ORIF_EXPR:
> + {
> + tree org_type = TREE_TYPE (*expr_p);
> +
> + *expr_p = gimple_boolify (*expr_p);
> +
> + /* This shouldn't happen, but due fold-const (and here especially
> + fold_truth_not_expr) happily uses operand type and doesn't
> + automatically uses boolean_type as result, this happens. */
> + if (TREE_CODE (org_type) != BOOLEAN_TYPE)
> + {
> + *expr_p = fold_convert (org_type, *expr_p);
> + ret = GS_OK;
> + break;
> + }
I suppose it makes sense to be consistent with the other TRUTH_*
exprs. But when looking at the further context - we call into
gimplify_boolean_expr - the conversion back to the original type
is not necessary. So I would prefer to inline gimplify_boolean_expr
at this single caller location and simply gimple_boolify (*expr_p)
without doing the conversion back. Thus,
@@ -6762,9 +6761,22 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
- /* Pass the source location of the outer expression. */
- ret = gimplify_boolean_expr (expr_p, saved_location);
- break;
+ {
+ /* Preserve the original type of the expression and the
+ source location of the outer expression. */
+ tree org_type = TREE_TYPE (*expr_p);
+ *expr_p = gimple_boolify (*expr_p);
+ *expr_p = build3_loc (saved_location, COND_EXPR,
+ org_type, *expr_p,
+ fold_convert_loc
+ (saved_location,
+ org_type, boolean_true_node),
+ fold_convert_loc
+ (saved_location,
+ org_type, boolean_false_node));
+ ret = GS_OK;
+ break;
+ }
case TRUTH_NOT_EXPR:
if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE)
and remove the then unused function.
Ok with that change if it passes bootstrap and regtest for all languages.
Thanks,
Richard.
> + }
> /* Pass the source location of the outer expression. */
> ret = gimplify_boolean_expr (expr_p, saved_location);
> break;
> @@ -7203,6 +7217,22 @@ gimplify_expr (tree *expr_p, gimple_seq
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> + {
> + tree org_type = TREE_TYPE (*expr_p);
> +
> + *expr_p = gimple_boolify (*expr_p);
> +
> + /* This shouldn't happen, but due fold-const (and here especially
> + fold_truth_not_expr) happily uses operand type and doesn't
> + automatically uses boolean_type as result, this happens. */
> + if (TREE_CODE (org_type) != BOOLEAN_TYPE)
> + {
> + *expr_p = fold_convert (org_type, *expr_p);
> + ret = GS_OK;
> + break;
> + }
> + }
> +
> /* Classified as tcc_expression. */
> goto expr_2;
>
> Index: gcc/gcc/tree-cfg.c
> ===================================================================
> --- gcc.orig/gcc/tree-cfg.c 2011-05-11 17:03:24.863377700 +0200
> +++ gcc/gcc/tree-cfg.c 2011-05-11 17:04:32.293292500 +0200
> @@ -3541,10 +3541,10 @@ do_pointer_plus_expr_check:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> {
> - /* We allow any kind of integral typed argument and result. */
> - if (!INTEGRAL_TYPE_P (rhs1_type)
> - || !INTEGRAL_TYPE_P (rhs2_type)
> - || !INTEGRAL_TYPE_P (lhs_type))
> + /* We allow only boolean typed argument and result. */
> + if (TREE_CODE (rhs1_type) != BOOLEAN_TYPE
> + || TREE_CODE (rhs2_type) != BOOLEAN_TYPE
> + || TREE_CODE (lhs_type) != BOOLEAN_TYPE)
> {
> error ("type mismatch in binary truth expression");
> debug_generic_expr (lhs_type);
>
