2011/5/10 Richard Guenther <richard.guent...@gmail.com>:
> On Tue, May 10, 2011 at 3:56 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
>> Hello,
>>
>> this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
>> expressions
>> on gimplification to their binary form. Additionally it takes care
>> that conditions
>> are getting boolified for operation.
>>
>> ChangeLog
>>
>> 2011-05-10 Kai Tietz
>>
>> * gimplify.c (gimplify_exit_expr): Boolify conditional
>> expression part.
>> (shortcut_cond_r): Likewise.
>> (shortcut_cond_expr): Likewise.
>> (gimplify_cond_expr): Likewise.
>> (gimplify_modify_expr_rhs): Likewise.
>> (gimplify_boolean_expr): Likewise.
>> (gimple_boolify): Boolify operands for BOOLEAN typed
>> base expressions.
>> (gimplify_expr): Boolify TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR,
>> TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR. Additionally
>> move TRUTH_AND|OR|XOR_EXPR to its binary form.
>>
>> Tested for x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
>
> Comments inline (the attachment makes my comments harder to spot,
> look close).
>
> Index: gcc/gcc/gimplify.c
> ===================================================================
> --- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.000000000 +0200
> +++ gcc/gcc/gimplify.c 2011-05-10 15:46:58.365473600 +0200
> @@ -1664,10 +1664,12 @@ build_and_jump (tree *label_p)
> static enum gimplify_status
> gimplify_exit_expr (tree *expr_p)
> {
> - tree cond = TREE_OPERAND (*expr_p, 0);
> - tree expr;
> + tree cond, expr;
>
> + TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
> + cond = TREE_OPERAND (*expr_p, 0);
>
> Why do you need to boolify things at all when we build a COND_EXPR
> that gets re-gimplified anyway? I'm confused by this (similar to other
> places you do that). I would expect that you at most would do this
> when gimplifying COND_EXPRs, not when you build them.
>
> expr = build_and_jump (&gimplify_ctxp->exit_label);
> +
> expr = build3 (COND_EXPR, void_type_node, cond, expr, NULL_TREE);
> *expr_p = expr;
>
> @@ -2586,6 +2588,7 @@ shortcut_cond_r (tree pred, tree *true_l
> /* Keep the original source location on the first 'if'. Set the source
> location of the ? on the second 'if'. */
> new_locus = EXPR_HAS_LOCATION (pred) ? EXPR_LOCATION (pred) : locus;
> + TREE_OPERAND (pred, 0) = gimple_boolify (TREE_OPERAND (pred, 0));
> expr = build3 (COND_EXPR, void_type_node, TREE_OPERAND (pred, 0),
> shortcut_cond_r (TREE_OPERAND (pred, 1), true_label_p,
> false_label_p, locus),
> @@ -2594,7 +2597,7 @@ shortcut_cond_r (tree pred, tree *true_l
> }
> else
> {
> - expr = build3 (COND_EXPR, void_type_node, pred,
> + expr = build3 (COND_EXPR, void_type_node, gimple_boolify (pred),
> build_and_jump (true_label_p),
> build_and_jump (false_label_p));
> SET_EXPR_LOCATION (expr, locus);
> @@ -2625,7 +2628,8 @@ shortcut_cond_expr (tree expr)
> bool emit_end, emit_false, jump_over_else;
> bool then_se = then_ && TREE_SIDE_EFFECTS (then_);
> bool else_se = else_ && TREE_SIDE_EFFECTS (else_);
> -
> +
> + pred = TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
> /* First do simple transformations. */
> if (!else_se)
> {
> @@ -2665,7 +2669,7 @@ shortcut_cond_expr (tree expr)
> SET_EXPR_LOCATION (expr, EXPR_LOCATION (pred));
> else_ = shortcut_cond_expr (expr);
> else_se = else_ && TREE_SIDE_EFFECTS (else_);
> - pred = TREE_OPERAND (pred, 0);
> + pred = gimple_boolify (TREE_OPERAND (pred, 0));
> expr = build3 (COND_EXPR, void_type_node, pred, NULL_TREE, else_);
> SET_EXPR_LOCATION (expr, locus);
> }
> @@ -2824,9 +2828,6 @@ gimple_boolify (tree expr)
> }
> }
>
> - if (TREE_CODE (type) == BOOLEAN_TYPE)
> - return expr;
> -
> switch (TREE_CODE (expr))
> {
> case TRUTH_AND_EXPR:
> @@ -2851,6 +2852,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;
>
> Why do you need to move this?
>
> return fold_convert_loc (loc, boolean_type_node, expr);
> }
> }
> @@ -2865,7 +2868,7 @@ gimplify_pure_cond_expr (tree *expr_p, g
> enum gimplify_status ret, tret;
> enum tree_code code;
>
> - cond = gimple_boolify (COND_EXPR_COND (expr));
> + cond = COND_EXPR_COND (expr) = gimple_boolify (COND_EXPR_COND (expr));
>
> why change COND_EXPR_COND?
>
> /* We need to handle && and || specially, as their gimplification
> creates pure cond_expr, thus leading to an infinite cycle otherwise. */
> @@ -2937,6 +2940,7 @@ gimplify_cond_expr (tree *expr_p, gimple
> enum tree_code pred_code;
> gimple_seq seq = NULL;
>
> + TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
>
> So you are doing it here - why do it everywhere else? I'd like to see
> this change at exactly _one_ place.
>
> /* If this COND_EXPR has a value, copy the values into a temporary within
> the arms. */
> if (!VOID_TYPE_P (type))
> @@ -4276,6 +4280,7 @@ gimplify_modify_expr_rhs (tree *expr_p,
> false);
>
> case COND_EXPR:
> +
> /* If we're assigning to a non-register type, push the assignment
> down into the branches. This is mandatory for ADDRESSABLE types,
> since we cannot generate temporaries for such, but it saves a
> @@ -4287,6 +4292,7 @@ gimplify_modify_expr_rhs (tree *expr_p,
> tree cond = *from_p;
> tree result = *to_p;
>
> + TREE_OPERAND (cond, 0) = gimple_boolify (TREE_OPERAND (cond,
> 0));
> ret = gimplify_expr (&result, pre_p, post_p,
> is_gimple_lvalue, fb_lvalue);
> if (ret != GS_ERROR)
> @@ -4710,6 +4716,7 @@ gimplify_boolean_expr (tree *expr_p, loc
> {
> /* Preserve the original type of the expression. */
> tree type = TREE_TYPE (*expr_p);
> + *expr_p = gimple_boolify (*expr_p);
>
> *expr_p = build3 (COND_EXPR, type, *expr_p,
> fold_convert_loc (locus, type, boolean_true_node),
> @@ -6762,6 +6769,13 @@ gimplify_expr (tree *expr_p, gimple_seq
>
> case TRUTH_ANDIF_EXPR:
> case TRUTH_ORIF_EXPR:
> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE)
> + {
> + tree type = TREE_TYPE (*expr_p);
> + *expr_p = fold_convert (type, gimple_boolify (*expr_p));
> + ret = GS_OK;
> + break;
> + }
>
> Huh, that doesn't make sense. Why convert back to the original
> type?
>
> I know COND_EXPRs don't have a boolean value type-wise, but
> they do have one semantically. It looks like you are just papering
> over the inconsistencies at a lot of places instead of trying to fix them
> during gimplification.
>
> /* Pass the source location of the outer expression. */
> ret = gimplify_boolean_expr (expr_p, saved_location);
> break;
> @@ -7203,6 +7217,30 @@ gimplify_expr (tree *expr_p, gimple_seq
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE)
> + {
> + tree type = TREE_TYPE (*expr_p);
> + *expr_p = fold_convert (type, gimple_boolify (*expr_p));
> + ret = GS_OK;
> + break;
> + }
> + /* Call it to make sure that operands are boolified, too. */
> + *expr_p = gimple_boolify (*expr_p);
> + switch (TREE_CODE (*expr_p))
> + {
> + case TRUTH_AND_EXPR:
> + TREE_SET_CODE (*expr_p, BIT_AND_EXPR);
> + break;
> + case TRUTH_OR_EXPR:
> + TREE_SET_CODE (*expr_p, BIT_IOR_EXPR);
> + break;
> + case TRUTH_XOR_EXPR:
> + TREE_SET_CODE (*expr_p, BIT_XOR_EXPR);
> + break;
> + default:
> + break;
> + }
> +
> /* Classified as tcc_expression. */
> goto expr_2;
>
> Please also change verify_gimple_assign_binary to barf on
> the TRUTH_* codes. We don't want other passes re-introducing them.
>
> Richard.
ChangeLog
2011-05-10 Kai Tietz
* tree-cfg.c (verify_gimple_assign_binary): Barf on
TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR.
* gimplify.c (gimplify_exit_expr): Boolify conditional
expression part.
(shortcut_cond_r): Likewise.
(shortcut_cond_expr): Likewise.
(gimplify_cond_expr): Likewise.
(gimplify_modify_expr_rhs): Likewise.
(gimplify_boolean_expr): Likewise.
(gimple_boolify): Boolify operands for BOOLEAN typed
base expressions.
(gimplify_expr): Boolify TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR,
TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR. Additionally
move TRUTH_AND|OR|XOR_EXPR to its binary form.
The issue about this gimplification is just in case of freshly
generated COND_EXPR and making sure its generated conditional is
boolified before creating. As late we might run exactly in those
retype-cast for conditional-part being result. The real pain are the
comparison expressions, which getting generated in some cases with
integer types, as they are often used as an implicit type cast.
The re-type type-casts I introduced after seeing exactly for
ANDIF/ORIF and TRUTH AND/OR/XOR the appearance of resulting types not
boolean.
Regards,
Kai
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.000000000 +0200
+++ gcc/gcc/gimplify.c 2011-05-10 15:46:58.365473600 +0200
@@ -1664,10 +1664,12 @@ build_and_jump (tree *label_p)
static enum gimplify_status
gimplify_exit_expr (tree *expr_p)
{
- tree cond = TREE_OPERAND (*expr_p, 0);
- tree expr;
+ tree cond, expr;
+ TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
+ cond = TREE_OPERAND (*expr_p, 0);
expr = build_and_jump (&gimplify_ctxp->exit_label);
+
expr = build3 (COND_EXPR, void_type_node, cond, expr, NULL_TREE);
*expr_p = expr;
@@ -2586,6 +2588,7 @@ shortcut_cond_r (tree pred, tree *true_l
/* Keep the original source location on the first 'if'. Set the source
location of the ? on the second 'if'. */
new_locus = EXPR_HAS_LOCATION (pred) ? EXPR_LOCATION (pred) : locus;
+ TREE_OPERAND (pred, 0) = gimple_boolify (TREE_OPERAND (pred, 0));
expr = build3 (COND_EXPR, void_type_node, TREE_OPERAND (pred, 0),
shortcut_cond_r (TREE_OPERAND (pred, 1), true_label_p,
false_label_p, locus),
@@ -2594,7 +2597,7 @@ shortcut_cond_r (tree pred, tree *true_l
}
else
{
- expr = build3 (COND_EXPR, void_type_node, pred,
+ expr = build3 (COND_EXPR, void_type_node, gimple_boolify (pred),
build_and_jump (true_label_p),
build_and_jump (false_label_p));
SET_EXPR_LOCATION (expr, locus);
@@ -2625,7 +2628,8 @@ shortcut_cond_expr (tree expr)
bool emit_end, emit_false, jump_over_else;
bool then_se = then_ && TREE_SIDE_EFFECTS (then_);
bool else_se = else_ && TREE_SIDE_EFFECTS (else_);
-
+
+ pred = TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
/* First do simple transformations. */
if (!else_se)
{
@@ -2665,7 +2669,7 @@ shortcut_cond_expr (tree expr)
SET_EXPR_LOCATION (expr, EXPR_LOCATION (pred));
else_ = shortcut_cond_expr (expr);
else_se = else_ && TREE_SIDE_EFFECTS (else_);
- pred = TREE_OPERAND (pred, 0);
+ pred = gimple_boolify (TREE_OPERAND (pred, 0));
expr = build3 (COND_EXPR, void_type_node, pred, NULL_TREE, else_);
SET_EXPR_LOCATION (expr, locus);
}
@@ -2824,9 +2828,6 @@ gimple_boolify (tree expr)
}
}
- if (TREE_CODE (type) == BOOLEAN_TYPE)
- return expr;
-
switch (TREE_CODE (expr))
{
case TRUTH_AND_EXPR:
@@ -2851,6 +2852,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);
}
}
@@ -2865,7 +2868,7 @@ gimplify_pure_cond_expr (tree *expr_p, g
enum gimplify_status ret, tret;
enum tree_code code;
- cond = gimple_boolify (COND_EXPR_COND (expr));
+ cond = COND_EXPR_COND (expr) = gimple_boolify (COND_EXPR_COND (expr));
/* We need to handle && and || specially, as their gimplification
creates pure cond_expr, thus leading to an infinite cycle otherwise. */
@@ -2937,6 +2940,7 @@ gimplify_cond_expr (tree *expr_p, gimple
enum tree_code pred_code;
gimple_seq seq = NULL;
+ TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
/* If this COND_EXPR has a value, copy the values into a temporary within
the arms. */
if (!VOID_TYPE_P (type))
@@ -4276,6 +4280,7 @@ gimplify_modify_expr_rhs (tree *expr_p,
false);
case COND_EXPR:
+
/* If we're assigning to a non-register type, push the assignment
down into the branches. This is mandatory for ADDRESSABLE types,
since we cannot generate temporaries for such, but it saves a
@@ -4287,6 +4292,7 @@ gimplify_modify_expr_rhs (tree *expr_p,
tree cond = *from_p;
tree result = *to_p;
+ TREE_OPERAND (cond, 0) = gimple_boolify (TREE_OPERAND (cond, 0));
ret = gimplify_expr (&result, pre_p, post_p,
is_gimple_lvalue, fb_lvalue);
if (ret != GS_ERROR)
@@ -4710,6 +4716,7 @@ gimplify_boolean_expr (tree *expr_p, loc
{
/* Preserve the original type of the expression. */
tree type = TREE_TYPE (*expr_p);
+ *expr_p = gimple_boolify (*expr_p);
*expr_p = build3 (COND_EXPR, type, *expr_p,
fold_convert_loc (locus, type, boolean_true_node),
@@ -6762,6 +6769,13 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
+ if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE)
+ {
+ tree type = TREE_TYPE (*expr_p);
+ *expr_p = fold_convert (type, gimple_boolify (*expr_p));
+ ret = GS_OK;
+ break;
+ }
/* Pass the source location of the outer expression. */
ret = gimplify_boolean_expr (expr_p, saved_location);
break;
@@ -7203,6 +7217,30 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
+ if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE)
+ {
+ tree type = TREE_TYPE (*expr_p);
+ *expr_p = fold_convert (type, gimple_boolify (*expr_p));
+ ret = GS_OK;
+ break;
+ }
+ /* Call it to make sure that operands are boolified, too. */
+ *expr_p = gimple_boolify (*expr_p);
+ switch (TREE_CODE (*expr_p))
+ {
+ case TRUTH_AND_EXPR:
+ TREE_SET_CODE (*expr_p, BIT_AND_EXPR);
+ break;
+ case TRUTH_OR_EXPR:
+ TREE_SET_CODE (*expr_p, BIT_IOR_EXPR);
+ break;
+ case TRUTH_XOR_EXPR:
+ TREE_SET_CODE (*expr_p, BIT_XOR_EXPR);
+ break;
+ default:
+ break;
+ }
+
/* Classified as tcc_expression. */
goto expr_2;
Index: gcc/gcc/tree-cfg.c
===================================================================
--- gcc.orig/gcc/tree-cfg.c 2011-05-04 15:59:07.000000000 +0200
+++ gcc/gcc/tree-cfg.c 2011-05-10 16:57:31.628029600 +0200
@@ -3540,21 +3540,7 @@ do_pointer_plus_expr_check:
case TRUTH_AND_EXPR:
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))
- {
- error ("type mismatch in binary truth expression");
- debug_generic_expr (lhs_type);
- debug_generic_expr (rhs1_type);
- debug_generic_expr (rhs2_type);
- return true;
- }
-
- return false;
- }
+ gcc_unreachable ();
case LT_EXPR:
case LE_EXPR:
