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