* [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
@ 2011-05-10 14:25 Kai Tietz
2011-05-10 15:24 ` Richard Guenther
2011-05-11 10:46 ` Eric Botcazou
0 siblings, 2 replies; 26+ messages in thread
From: Kai Tietz @ 2011-05-10 14:25 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther
[-- Attachment #1: Type: text/plain, Size: 822 bytes --]
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?
Regards,
Kai
[-- Attachment #2: truth_op_gimplify.txt --]
[-- Type: text/plain, Size: 5746 bytes --]
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;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 14:25 [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary Kai Tietz
@ 2011-05-10 15:24 ` Richard Guenther
2011-05-10 15:34 ` Kai Tietz
2011-05-11 10:46 ` Eric Botcazou
1 sibling, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-10 15:24 UTC (permalink / raw)
To: Kai Tietz; +Cc: GCC Patches
On Tue, May 10, 2011 at 3:56 PM, Kai Tietz <ktietz70@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
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 15:24 ` Richard Guenther
@ 2011-05-10 15:34 ` Kai Tietz
2011-05-10 15:38 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-10 15:34 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 10002 bytes --]
2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, May 10, 2011 at 3:56 PM, Kai Tietz <ktietz70@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
[-- Attachment #2: truth_op_gimplify.txt --]
[-- Type: text/plain, Size: 6573 bytes --]
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:
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 15:34 ` Kai Tietz
@ 2011-05-10 15:38 ` Richard Guenther
2011-05-10 15:45 ` Paolo Bonzini
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-10 15:38 UTC (permalink / raw)
To: Kai Tietz; +Cc: GCC Patches
On Tue, May 10, 2011 at 5:08 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
>> On Tue, May 10, 2011 at 3:56 PM, Kai Tietz <ktietz70@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.
But you know they are boolean in the sense that they are
truth_value_p, that is they result from an operation that has
a boolean result. Take advantage of that (thus, a truncation
is never necessary).
This doesn't answer all my remarks. I know that comparisons are
badly typed (fold happily folds integer conversions into the comparison
type, but likely the FEs also create those directly). So, the gimplifier
already has to deal with this case. Thus, you shouldn't need to sprinkle
explicit handling all over the place.
I suppose you have testcases for all the cases you looked at, please
add some that cover these corner cases.
Also I see middle-end pieces that still call (fold_)build2 (TRUTH_ ...
eventually re-gimplifying the stuff, so it might not show up with
testing. Those should be changed to build the BIT_ variant.
Richard.
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 15:38 ` Richard Guenther
@ 2011-05-10 15:45 ` Paolo Bonzini
2011-05-10 15:52 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2011-05-10 15:45 UTC (permalink / raw)
To: Richard Guenther; +Cc: Kai Tietz, GCC Patches
On 05/10/2011 05:23 PM, Richard Guenther wrote:
> I suppose you have testcases for all the cases you looked at, please
> add some that cover these corner cases.
Also, there is quite some tree-vrp.c dead code with these changes.
Removing the TRUTH_*_CODE handling in VRP will help finding more places
where the middle-end is building boolean operations. There should be
testcases covering these parts of VRP.
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 15:45 ` Paolo Bonzini
@ 2011-05-10 15:52 ` Richard Guenther
2011-05-10 17:28 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-10 15:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Kai Tietz, GCC Patches
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>
>> I suppose you have testcases for all the cases you looked at, please
>> add some that cover these corner cases.
>
> Also, there is quite some tree-vrp.c dead code with these changes. Removing
> the TRUTH_*_CODE handling in VRP will help finding more places where the
> middle-end is building boolean operations. There should be testcases
> covering these parts of VRP.
Btw, you can split the patch into two pieces - first, make TRUTH_*
expressions correctly typed (take boolean typed operands and procude
a boolean typed result) and verify that in verify_gimple_assign_binary.
A second patch than can do the s/TRUTH_/BIT_/ substitution during
gimplification. That way the first (and more difficult) part doesn't get
too big with unrelated changes.
Richard.
> Paolo
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 15:52 ` Richard Guenther
@ 2011-05-10 17:28 ` Kai Tietz
2011-05-10 20:49 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-10 17:28 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>>
>>> I suppose you have testcases for all the cases you looked at, please
>>> add some that cover these corner cases.
>>
>> Also, there is quite some tree-vrp.c dead code with these changes. Removing
>> the TRUTH_*_CODE handling in VRP will help finding more places where the
>> middle-end is building boolean operations. There should be testcases
>> covering these parts of VRP.
>
> Btw, you can split the patch into two pieces - first, make TRUTH_*
> expressions correctly typed (take boolean typed operands and procude
> a boolean typed result) and verify that in verify_gimple_assign_binary.
> A second patch than can do the s/TRUTH_/BIT_/ substitution during
> gimplification. That way the first (and more difficult) part doesn't get
> too big with unrelated changes.
>
> Richard.
>
>> Paolo
>>
>
Well, I think I found one big issue here about booified expression of
condition. The gimple_boolify needs to handle COND_EXPR in more
detail. As if a conditional expression has to be boolified, it means
its condition and its other operands need to be boolified, too. And
this is for sure one cause, why I see for ANDIF/ORIF and the truth
AND|OR|XOR none boolean types.
I will continue on that.
To split this seems to make sense, as I have to touch much more areas
for the TRUTH to BIT conversion.
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 17:28 ` Kai Tietz
@ 2011-05-10 20:49 ` Kai Tietz
2011-05-11 9:35 ` Richard Guenther
2011-05-11 10:00 ` Kai Tietz
0 siblings, 2 replies; 26+ messages in thread
From: Kai Tietz @ 2011-05-10 20:49 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]
2011/5/10 Kai Tietz <ktietz70@googlemail.com>:
> 2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
>> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>>>
>>>> I suppose you have testcases for all the cases you looked at, please
>>>> add some that cover these corner cases.
>>>
>>> Also, there is quite some tree-vrp.c dead code with these changes. Removing
>>> the TRUTH_*_CODE handling in VRP will help finding more places where the
>>> middle-end is building boolean operations. There should be testcases
>>> covering these parts of VRP.
>>
>> Btw, you can split the patch into two pieces - first, make TRUTH_*
>> expressions correctly typed (take boolean typed operands and procude
>> a boolean typed result) and verify that in verify_gimple_assign_binary.
>> A second patch than can do the s/TRUTH_/BIT_/ substitution during
>> gimplification. That way the first (and more difficult) part doesn't get
>> too big with unrelated changes.
>>
>> Richard.
>>
>>> Paolo
>>>
>>
>
> Well, I think I found one big issue here about booified expression of
> condition. The gimple_boolify needs to handle COND_EXPR in more
> detail. As if a conditional expression has to be boolified, it means
> its condition and its other operands need to be boolified, too. And
> this is for sure one cause, why I see for ANDIF/ORIF and the truth
> AND|OR|XOR none boolean types.
>
> I will continue on that.
>
> To split this seems to make sense, as I have to touch much more areas
> for the TRUTH to BIT conversion.
>
> Regards,
> Kai
>
So I use this thread for first part of the series of patches. This one
improves boolean type-logic
during gimplification.
To gimple_boolify the handling for COND_EXPR are added, and in general
it is tried to do
boolean operation just on boolean type. As sadly fold-const (and here
in special fold_truth_not_expr (), which doesn't provide by default
boolean type and uses instead operand-type, which is IMHO a major flaw
here. But well, there are some comments indicating that this behavior
is required due some other quirks. But this is for sure something to
be cleaned up) produces truth operations with wrong type, which are in
calculations not necessarily identifyable as truth ops anymore, this
patch makes sure that for truth AND|OR|XOR original type remains.
2011-05-10 Kai Tietz
* gimplify.c (gimple_boolify): Handle COND_EXPR
and make sure that even if type is BOOLEAN for
TRUTH-opcodes the operands getting boolified.
(gimplify_expr): Boolify operand condition for
COND_EXPR.
Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
take care that we keep expression's type.
Ok for apply?
Regards,
Kai
[-- Attachment #2: thruth_conditions_gimple.txt --]
[-- Type: text/plain, Size: 3135 bytes --]
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200
+++ gcc/gcc/gimplify.c 2011-05-10 21:14:49.106340400 +0200
@@ -2824,11 +2824,20 @@ gimple_boolify (tree expr)
}
}
- if (TREE_CODE (type) == BOOLEAN_TYPE)
- return expr;
-
switch (TREE_CODE (expr))
{
+ case COND_EXPR:
+ /* If we have a conditional expression, which shall be
+ boolean, take care we boolify also their left and right arm. */
+ if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2))))
+ TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2));
+ if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1))))
+ TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1));
+ TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
+ if (TREE_CODE (type) == BOOLEAN_TYPE)
+ return expr;
+ return fold_convert_loc (loc, boolean_type_node, expr);
+
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
@@ -2851,6 +2860,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);
}
}
@@ -6714,6 +6725,7 @@ gimplify_expr (tree *expr_p, gimple_seq
break;
case COND_EXPR:
+ TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
ret = gimplify_cond_expr (expr_p, pre_p, fallback);
/* C99 code may assign to an array in a structure value of a
@@ -6762,6 +6774,17 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
/* Pass the source location of the outer expression. */
ret = gimplify_boolean_expr (expr_p, saved_location);
break;
@@ -7203,6 +7226,17 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
/* Classified as tcc_expression. */
goto expr_2;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 20:49 ` Kai Tietz
@ 2011-05-11 9:35 ` Richard Guenther
2011-05-11 10:06 ` Kai Tietz
2011-05-11 10:51 ` Kai Tietz
2011-05-11 10:00 ` Kai Tietz
1 sibling, 2 replies; 26+ messages in thread
From: Richard Guenther @ 2011-05-11 9:35 UTC (permalink / raw)
To: Kai Tietz; +Cc: Paolo Bonzini, GCC Patches
On Tue, May 10, 2011 at 9:26 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/10 Kai Tietz <ktietz70@googlemail.com>:
>> 2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
>>> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>>>>
>>>>> I suppose you have testcases for all the cases you looked at, please
>>>>> add some that cover these corner cases.
>>>>
>>>> Also, there is quite some tree-vrp.c dead code with these changes. Removing
>>>> the TRUTH_*_CODE handling in VRP will help finding more places where the
>>>> middle-end is building boolean operations. There should be testcases
>>>> covering these parts of VRP.
>>>
>>> Btw, you can split the patch into two pieces - first, make TRUTH_*
>>> expressions correctly typed (take boolean typed operands and procude
>>> a boolean typed result) and verify that in verify_gimple_assign_binary.
>>> A second patch than can do the s/TRUTH_/BIT_/ substitution during
>>> gimplification. That way the first (and more difficult) part doesn't get
>>> too big with unrelated changes.
>>>
>>> Richard.
>>>
>>>> Paolo
>>>>
>>>
>>
>> Well, I think I found one big issue here about booified expression of
>> condition. The gimple_boolify needs to handle COND_EXPR in more
>> detail. As if a conditional expression has to be boolified, it means
>> its condition and its other operands need to be boolified, too. And
>> this is for sure one cause, why I see for ANDIF/ORIF and the truth
>> AND|OR|XOR none boolean types.
>>
>> I will continue on that.
>>
>> To split this seems to make sense, as I have to touch much more areas
>> for the TRUTH to BIT conversion.
>>
>> Regards,
>> Kai
>>
>
> So I use this thread for first part of the series of patches. This one
> improves boolean type-logic
> during gimplification.
> To gimple_boolify the handling for COND_EXPR are added, and in general
> it is tried to do
> boolean operation just on boolean type. As sadly fold-const (and here
> in special fold_truth_not_expr (), which doesn't provide by default
> boolean type and uses instead operand-type, which is IMHO a major flaw
> here. But well, there are some comments indicating that this behavior
> is required due some other quirks. But this is for sure something to
> be cleaned up) produces truth operations with wrong type, which are in
> calculations not necessarily identifyable as truth ops anymore, this
> patch makes sure that for truth AND|OR|XOR original type remains.
>
> 2011-05-10 Kai Tietz
>
> * gimplify.c (gimple_boolify): Handle COND_EXPR
> and make sure that even if type is BOOLEAN for
> TRUTH-opcodes the operands getting boolified.
> (gimplify_expr): Boolify operand condition for
> COND_EXPR.
> Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
> take care that we keep expression's type.
>
> Ok for apply?
Posting patches inline makes it easier to put in review comments, so
please consider doing that.
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200
+++ gcc/gcc/gimplify.c 2011-05-10 21:14:49.106340400 +0200
@@ -2824,11 +2824,20 @@ gimple_boolify (tree expr)
}
}
- if (TREE_CODE (type) == BOOLEAN_TYPE)
- return expr;
-
switch (TREE_CODE (expr))
{
+ case COND_EXPR:
+ /* If we have a conditional expression, which shall be
+ boolean, take care we boolify also their left and right arm. */
+ if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P
(TREE_TYPE (TREE_OPERAND (expr, 2))))
+ TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2));
+ if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P
(TREE_TYPE (TREE_OPERAND (expr, 1))))
+ TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1));
+ TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
+ if (TREE_CODE (type) == BOOLEAN_TYPE)
+ return expr;
+ return fold_convert_loc (loc, boolean_type_node, expr);
+
That looks like premature optimization. Why isn't it enough to
fold-convert the COND_EXPR itself? Thus, I don't see why the
existing gimple_boolify isn't enough.
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
@@ -2851,6 +2860,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);
}
}
@@ -6714,6 +6725,7 @@ gimplify_expr (tree *expr_p, gimple_seq
break;
case COND_EXPR:
+ TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
ret = gimplify_cond_expr (expr_p, pre_p, fallback);
This boolification should be done by gimplify_cond_expr as I said
in the previous review. It does already handle this perfectly fine.
/* C99 code may assign to an array in a structure value of a
@@ -6762,6 +6774,17 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
This shouldn't be necessary at all. Instead gimplify_boolean_expr
should deal with it - which it magically does by building a COND_EXPR
which should already deal with all cases just fine.
/* Pass the source location of the outer expression. */
ret = gimplify_boolean_expr (expr_p, saved_location);
break;
@@ -7203,6 +7226,17 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
Now this is the only place where we need to do something. And it
nearly looks ok but IMHO should simply do
tree orig_type = TREE_TYPE (*expr_p);
*expr_p = gimple_boolify (*expr_p);
if (TREE_CODE (orig_type) != BOOLEAN_TYPE)
{
*expr_p = fold_convert (orig_type, *expr_p);
ret = GS_OK;
break;
}
/* Classified as tcc_expression. */
goto expr_2;
You lack a tree-cfg.c hunk to verify that the TRUTH_* exprs have
correct types. Sth like
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c (revision 173611)
+++ gcc/tree-cfg.c (working copy)
@@ -3542,9 +3615,9 @@ do_pointer_plus_expr_check:
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))
+ 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);
Otherwise you wouldn't know whether your patch was complete (how
did you verify that?).
Richard.
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 20:49 ` Kai Tietz
2011-05-11 9:35 ` Richard Guenther
@ 2011-05-11 10:00 ` Kai Tietz
2011-05-11 10:03 ` Richard Guenther
1 sibling, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 10:00 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]
Hi,
By investigating the conditional expression handling I found some
causes, why TRUTH operations AND, ANDIF, OR, XOR, and ORIF are
appearing withing conditional folding during gimplification.
The reason for this can be that the truth expression is simply used as
result of an assignment or return statement, which then leads to the
issue that expression has lhs type. By this reason it is still
necessary to have in TRUTH operations type-cast after boolifying it
for later operation, if type isn't of kind boolean. Therefore it is
necessary for conditional to check if their arms might be TRUTH
results and therefore doing boolification of the arms, too.
2011-05-11 Kai Tietz
* gimplify.c (gimple_boolify): Handle COND_EXPR
and make sure that even if type is BOOLEAN for
TRUTH-opcodes the operands getting boolified.
(gimple_has_cond_boolean_arms): Helper function to
detect if condition is a TRUTH operation in arms.
(gimple_is_truth_op): Checks if operand is of BOOLEAN
kind.
(gimplify_expr): Boolify operand condition for
COND_EXPR and try to see if condition might be an TRUTH operation.
Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
take care that we keep expression's type.
Tested on x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
Regards,
Kai
[-- Attachment #2: thruth_conditions_gimple.txt --]
[-- Type: text/plain, Size: 6050 bytes --]
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200
+++ gcc/gcc/gimplify.c 2011-05-11 10:20:30.413964700 +0200
@@ -102,6 +102,7 @@ typedef struct gimple_temp_hash_elt
/* Forward declaration. */
static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool);
+static bool gimple_has_cond_boolean_arms (tree);
/* Mark X addressable. Unlike the langhook we expect X to be in gimple
form and we don't do any syntax checking. */
@@ -2824,11 +2825,26 @@ gimple_boolify (tree expr)
}
}
- if (TREE_CODE (type) == BOOLEAN_TYPE)
- return expr;
-
switch (TREE_CODE (expr))
{
+ case COND_EXPR:
+ /* If we have a conditional expression, which shall be
+ boolean, take care we boolify also their left and right arm. */
+ if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2))))
+ TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2));
+ if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1))))
+ TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1));
+ TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
+ if (TREE_CODE (type) == BOOLEAN_TYPE)
+ return expr;
+ if (!VOID_TYPE_P (TREE_TYPE (expr)))
+ {
+ /* These expressions always produce boolean results. */
+ TREE_TYPE (expr) = boolean_type_node;
+ }
+ else
+ return fold_convert_loc (loc, boolean_type_node, expr);
+
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
@@ -2851,6 +2867,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);
}
}
@@ -2909,6 +2927,66 @@ generic_expr_could_trap_p (tree expr)
return false;
}
+/* This function checks if OP is of TRUTH kind expression. It special-case
+ COND_EXPR, as here we need to look on results, too. See function
+ gimple_has_cond_boolean_arms () for more details.
+ If OP is of possible kind TRUTH expression, TRUE is returned,
+ otherwise FALSE is returned.
+ This functions assumes that all TRUTH operations, COND_EXPR with
+ boolean arms, INTEGER_CST with value of one or zero, and any
+ comparision has boolean result. */
+
+static bool
+gimple_is_truth_op (tree op)
+{
+ if (VOID_TYPE_P (TREE_TYPE (op)))
+ return false;
+ if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
+ return true;
+ switch (TREE_CODE (op))
+ {
+ case TRUTH_AND_EXPR:
+ case TRUTH_OR_EXPR:
+ case TRUTH_XOR_EXPR:
+ case TRUTH_ANDIF_EXPR:
+ case TRUTH_ORIF_EXPR:
+ case TRUTH_NOT_EXPR:
+ return true;
+ case COND_EXPR:
+ return gimple_has_cond_boolean_arms (op);
+ case INTEGER_CST:
+ if (integer_zerop (op) || integer_onep (op))
+ return true;
+ return false;
+ default:
+ if (TREE_CODE_CLASS (TREE_CODE (op)) == tcc_comparison)
+ return true;
+ break;
+ }
+ return false;
+}
+
+/* This function checks if all arms of the condition expression EXPR
+ are of kind TRUTH. If so, it returns TRUE, otherwise FALSE. */
+
+static bool
+gimple_has_cond_boolean_arms (tree expr)
+{
+ tree type = TREE_TYPE (expr);
+ tree arm1 = TREE_OPERAND (expr, 0);
+ tree arm2 = TREE_OPERAND (expr, 1);
+
+ if (VOID_TYPE_P (type))
+ return false;
+ if (!arm1 && !arm2)
+ return false;
+ if (arm1 && !gimple_is_truth_op (arm1))
+ return false;
+ if (arm2 && !gimple_is_truth_op (arm2))
+ return false;
+ return true;
+}
+
/* Convert the conditional expression pointed to by EXPR_P '(p) ? a : b;'
into
@@ -6714,7 +6792,25 @@ gimplify_expr (tree *expr_p, gimple_seq
break;
case COND_EXPR:
- ret = gimplify_cond_expr (expr_p, pre_p, fallback);
+ {
+ tree type = TREE_TYPE (*expr_p);
+ tree nexpr = NULL_TREE;
+ if (TREE_CODE (type) == BOOLEAN_TYPE
+ || !gimple_has_cond_boolean_arms (*expr_p))
+ type = NULL;
+ if (type)
+ nexpr = gimple_boolify (*expr_p);
+
+ TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
+
+ ret = gimplify_cond_expr (expr_p, pre_p, fallback);
+
+ if (nexpr)
+ {
+ *expr_p = fold_convert (type, *expr_p);
+ ret = GS_OK;
+ }
+ }
/* C99 code may assign to an array in a structure value of a
conditional expression, and this has undefined behavior
@@ -6762,6 +6858,18 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
/* Pass the source location of the outer expression. */
ret = gimplify_boolean_expr (expr_p, saved_location);
break;
@@ -7203,6 +7311,17 @@ gimplify_expr (tree *expr_p, gimple_seq
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
case TRUTH_XOR_EXPR:
+ /* 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 (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;
+ }
+ *expr_p = gimple_boolify (*expr_p);
/* Classified as tcc_expression. */
goto expr_2;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 10:00 ` Kai Tietz
@ 2011-05-11 10:03 ` Richard Guenther
2011-05-11 10:12 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-11 10:03 UTC (permalink / raw)
To: Kai Tietz; +Cc: Paolo Bonzini, GCC Patches
On Wed, May 11, 2011 at 11:00 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Hi,
>
> By investigating the conditional expression handling I found some
> causes, why TRUTH operations AND, ANDIF, OR, XOR, and ORIF are
> appearing withing conditional folding during gimplification.
> The reason for this can be that the truth expression is simply used as
> result of an assignment or return statement, which then leads to the
> issue that expression has lhs type. By this reason it is still
> necessary to have in TRUTH operations type-cast after boolifying it
> for later operation, if type isn't of kind boolean. Therefore it is
> necessary for conditional to check if their arms might be TRUTH
> results and therefore doing boolification of the arms, too.
You are not making much sense - gimple_boolify already boolifies
both arms. It assumes that if the expr is bool already the arms are
as well - I'm not sure that always holds, so defering that check as
your patch did probably makes sense.
Richard.
> 2011-05-11 Kai Tietz
>
> * gimplify.c (gimple_boolify): Handle COND_EXPR
> and make sure that even if type is BOOLEAN for
> TRUTH-opcodes the operands getting boolified.
> (gimple_has_cond_boolean_arms): Helper function to
> detect if condition is a TRUTH operation in arms.
> (gimple_is_truth_op): Checks if operand is of BOOLEAN
> kind.
> (gimplify_expr): Boolify operand condition for
> COND_EXPR and try to see if condition might be an TRUTH operation.
> Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
> take care that we keep expression's type.
>
> Tested on x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
>
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 9:35 ` Richard Guenther
@ 2011-05-11 10:06 ` Kai Tietz
2011-05-11 10:51 ` Kai Tietz
1 sibling, 0 replies; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 10:06 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
2011/5/11 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, May 10, 2011 at 9:26 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/10 Kai Tietz <ktietz70@googlemail.com>:
>>> 2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>>>>>
>>>>>> I suppose you have testcases for all the cases you looked at, please
>>>>>> add some that cover these corner cases.
>>>>>
>>>>> Also, there is quite some tree-vrp.c dead code with these changes. Removing
>>>>> the TRUTH_*_CODE handling in VRP will help finding more places where the
>>>>> middle-end is building boolean operations. There should be testcases
>>>>> covering these parts of VRP.
>>>>
>>>> Btw, you can split the patch into two pieces - first, make TRUTH_*
>>>> expressions correctly typed (take boolean typed operands and procude
>>>> a boolean typed result) and verify that in verify_gimple_assign_binary.
>>>> A second patch than can do the s/TRUTH_/BIT_/ substitution during
>>>> gimplification. That way the first (and more difficult) part doesn't get
>>>> too big with unrelated changes.
>>>>
>>>> Richard.
>>>>
>>>>> Paolo
>>>>>
>>>>
>>>
>>> Well, I think I found one big issue here about booified expression of
>>> condition. The gimple_boolify needs to handle COND_EXPR in more
>>> detail. As if a conditional expression has to be boolified, it means
>>> its condition and its other operands need to be boolified, too. And
>>> this is for sure one cause, why I see for ANDIF/ORIF and the truth
>>> AND|OR|XOR none boolean types.
>>>
>>> I will continue on that.
>>>
>>> To split this seems to make sense, as I have to touch much more areas
>>> for the TRUTH to BIT conversion.
>>>
>>> Regards,
>>> Kai
>>>
>>
>> So I use this thread for first part of the series of patches. This one
>> improves boolean type-logic
>> during gimplification.
>> To gimple_boolify the handling for COND_EXPR are added, and in general
>> it is tried to do
>> boolean operation just on boolean type. As sadly fold-const (and here
>> in special fold_truth_not_expr (), which doesn't provide by default
>> boolean type and uses instead operand-type, which is IMHO a major flaw
>> here. But well, there are some comments indicating that this behavior
>> is required due some other quirks. But this is for sure something to
>> be cleaned up) produces truth operations with wrong type, which are in
>> calculations not necessarily identifyable as truth ops anymore, this
>> patch makes sure that for truth AND|OR|XOR original type remains.
>>
>> 2011-05-10 Kai Tietz
>>
>> * gimplify.c (gimple_boolify): Handle COND_EXPR
>> and make sure that even if type is BOOLEAN for
>> TRUTH-opcodes the operands getting boolified.
>> (gimplify_expr): Boolify operand condition for
>> COND_EXPR.
>> Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
>> take care that we keep expression's type.
>>
>> Ok for apply?
>
> Posting patches inline makes it easier to put in review comments, so
> please consider doing that.
Ok, I think about this. Not sure how well, google-mail handles this.
I'll try next time.
> Index: gcc/gcc/gimplify.c
> ===================================================================
> --- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200
> +++ gcc/gcc/gimplify.c 2011-05-10 21:14:49.106340400 +0200
> @@ -2824,11 +2824,20 @@ gimple_boolify (tree expr)
> }
> }
>
> - if (TREE_CODE (type) == BOOLEAN_TYPE)
> - return expr;
> -
> switch (TREE_CODE (expr))
> {
> + case COND_EXPR:
> + /* If we have a conditional expression, which shall be
> + boolean, take care we boolify also their left and right arm. */
> + if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P
> (TREE_TYPE (TREE_OPERAND (expr, 2))))
> + TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2));
> + if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P
> (TREE_TYPE (TREE_OPERAND (expr, 1))))
> + TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1));
> + TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
> + if (TREE_CODE (type) == BOOLEAN_TYPE)
> + return expr;
> + return fold_convert_loc (loc, boolean_type_node, expr);
> +
>
> That looks like premature optimization. Why isn't it enough to
> fold-convert the COND_EXPR itself? Thus, I don't see why the
> existing gimple_boolify isn't enough.
See description of recent update patch. It isn't enough.
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> @@ -2851,6 +2860,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);
> }
> }
> @@ -6714,6 +6725,7 @@ gimplify_expr (tree *expr_p, gimple_seq
> break;
>
> case COND_EXPR:
> + TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
> ret = gimplify_cond_expr (expr_p, pre_p, fallback);
>
> This boolification should be done by gimplify_cond_expr as I said
> in the previous review. It does already handle this perfectly fine.
No, but this variant is a bit too weak (see again updated patch). As
the COND itself can be TRUTH operation, which otherwise will later on
introduces none-boolified inner ANDIF/ORIF/AND/OR/XOR truth
expressions.
> /* C99 code may assign to an array in a structure value of a
> @@ -6762,6 +6774,17 @@ gimplify_expr (tree *expr_p, gimple_seq
>
> case TRUTH_ANDIF_EXPR:
> case TRUTH_ORIF_EXPR:
> + /* 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 (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;
> + }
> + *expr_p = gimple_boolify (*expr_p);
>
> This shouldn't be necessary at all. Instead gimplify_boolean_expr
> should deal with it - which it magically does by building a COND_EXPR
> which should already deal with all cases just fine.
>
> /* Pass the source location of the outer expression. */
> ret = gimplify_boolean_expr (expr_p, saved_location);
> break;
> @@ -7203,6 +7226,17 @@ gimplify_expr (tree *expr_p, gimple_seq
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> + /* 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 (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;
> + }
> + *expr_p = gimple_boolify (*expr_p);
>
> Now this is the only place where we need to do something. And it
> nearly looks ok but IMHO should simply do
>
> tree orig_type = TREE_TYPE (*expr_p);
> *expr_p = gimple_boolify (*expr_p);
> if (TREE_CODE (orig_type) != BOOLEAN_TYPE)
> {
> *expr_p = fold_convert (orig_type, *expr_p);
> ret = GS_OK;
> break;
> }
>
> /* Classified as tcc_expression. */
> goto expr_2;
>
> You lack a tree-cfg.c hunk to verify that the TRUTH_* exprs have
> correct types. Sth like
Ok, I will add this. With conditional expression checking, this
should be doable to check here for XOR/AND/OR truth.
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c (revision 173611)
> +++ gcc/tree-cfg.c (working copy)
> @@ -3542,9 +3615,9 @@ do_pointer_plus_expr_check:
> 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))
> + 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);
>
>
> Otherwise you wouldn't know whether your patch was complete (how
> did you verify that?).
>
> Richard.
>
>
>> Regards,
>> Kai
>>
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 10:03 ` Richard Guenther
@ 2011-05-11 10:12 ` Kai Tietz
2011-05-11 10:31 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 10:12 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
2011/5/11 Richard Guenther <richard.guenther@gmail.com>:
> On Wed, May 11, 2011 at 11:00 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hi,
>>
>> By investigating the conditional expression handling I found some
>> causes, why TRUTH operations AND, ANDIF, OR, XOR, and ORIF are
>> appearing withing conditional folding during gimplification.
>> The reason for this can be that the truth expression is simply used as
>> result of an assignment or return statement, which then leads to the
>> issue that expression has lhs type. By this reason it is still
>> necessary to have in TRUTH operations type-cast after boolifying it
>> for later operation, if type isn't of kind boolean. Therefore it is
>> necessary for conditional to check if their arms might be TRUTH
>> results and therefore doing boolification of the arms, too.
>
> You are not making much sense - gimple_boolify already boolifies
> both arms. It assumes that if the expr is bool already the arms are
> as well - I'm not sure that always holds, so defering that check as
> your patch did probably makes sense.
It makes absolutely sense. Simply try the following example:
int
foo (int a, int b, int c)
{
int e = (a && b);
return e ? (c && !a) : (c && a && b);
}
You will see that by this you have TRUTH AND operations with none
boolean type, due the fact that for a conditional only the cond part
was converted. This patch checks that inner arms getting boolified, if
result of condition on both arms is a TRUTH result.
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 10:12 ` Kai Tietz
@ 2011-05-11 10:31 ` Kai Tietz
2011-05-11 12:08 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 10:31 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
For more detail what happens and why this conditional handling is necessary:
The sample code is:
int
foo (int a, int b)
{
return (a ? b != 3 : 0);
}
leads for variant without condition boolifying of arms to:
;; Function foo (foo)
foo (int a, int b)
{
int D.1991;
int D.1990;
int D.1989;
<bb 2>:
D.1990_2 = a_1(D) != 0;
D.1991_4 = b_3(D) != 3;
D.1989_5 = D.1991_4 & D.1990_2;
return D.1989_5;
}
with this code we see
;; Function foo (foo)
foo (int a, int b)
{
_Bool D.1992;
_Bool D.1991;
_Bool D.1990;
int D.1989;
<bb 2>:
D.1990_2 = a_1(D) != 0;
D.1991_4 = b_3(D) != 3;
D.1992_5 = D.1991_4 & D.1990_2;
D.1989_6 = (int) D.1992_5;
return D.1989_6;
}
So you see that by this, the SSA variable having _Bool type, as to be
wished, and not being handled as int.
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-10 14:25 [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary Kai Tietz
2011-05-10 15:24 ` Richard Guenther
@ 2011-05-11 10:46 ` Eric Botcazou
2011-05-11 11:12 ` Kai Tietz
1 sibling, 1 reply; 26+ messages in thread
From: Eric Botcazou @ 2011-05-11 10:46 UTC (permalink / raw)
To: Kai Tietz; +Cc: gcc-patches, Richard Guenther
> this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
> expressions on gimplification to their binary form.
What is it for? This will redirect the compilation stream from proven paths to
others so there must be a good reason to do it. What's the effect on the code?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 9:35 ` Richard Guenther
2011-05-11 10:06 ` Kai Tietz
@ 2011-05-11 10:51 ` Kai Tietz
1 sibling, 0 replies; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 10:51 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, GCC Patches
2011/5/11 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, May 10, 2011 at 9:26 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/10 Kai Tietz <ktietz70@googlemail.com>:
>>> 2011/5/10 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> On 05/10/2011 05:23 PM, Richard Guenther wrote:
>>>>>>
>>>>>> I suppose you have testcases for all the cases you looked at, please
>>>>>> add some that cover these corner cases.
>>>>>
>>>>> Also, there is quite some tree-vrp.c dead code with these changes. Removing
>>>>> the TRUTH_*_CODE handling in VRP will help finding more places where the
>>>>> middle-end is building boolean operations. There should be testcases
>>>>> covering these parts of VRP.
>>>>
>>>> Btw, you can split the patch into two pieces - first, make TRUTH_*
>>>> expressions correctly typed (take boolean typed operands and procude
>>>> a boolean typed result) and verify that in verify_gimple_assign_binary.
>>>> A second patch than can do the s/TRUTH_/BIT_/ substitution during
>>>> gimplification. That way the first (and more difficult) part doesn't get
>>>> too big with unrelated changes.
>>>>
>>>> Richard.
>>>>
>>>>> Paolo
>>>>>
>>>>
>>>
>>> Well, I think I found one big issue here about booified expression of
>>> condition. The gimple_boolify needs to handle COND_EXPR in more
>>> detail. As if a conditional expression has to be boolified, it means
>>> its condition and its other operands need to be boolified, too. And
>>> this is for sure one cause, why I see for ANDIF/ORIF and the truth
>>> AND|OR|XOR none boolean types.
>>>
>>> I will continue on that.
>>>
>>> To split this seems to make sense, as I have to touch much more areas
>>> for the TRUTH to BIT conversion.
>>>
>>> Regards,
>>> Kai
>>>
>>
>> So I use this thread for first part of the series of patches. This one
>> improves boolean type-logic
>> during gimplification.
>> To gimple_boolify the handling for COND_EXPR are added, and in general
>> it is tried to do
>> boolean operation just on boolean type. As sadly fold-const (and here
>> in special fold_truth_not_expr (), which doesn't provide by default
>> boolean type and uses instead operand-type, which is IMHO a major flaw
>> here. But well, there are some comments indicating that this behavior
>> is required due some other quirks. But this is for sure something to
>> be cleaned up) produces truth operations with wrong type, which are in
>> calculations not necessarily identifyable as truth ops anymore, this
>> patch makes sure that for truth AND|OR|XOR original type remains.
>>
>> 2011-05-10 Kai Tietz
>>
>> * gimplify.c (gimple_boolify): Handle COND_EXPR
>> and make sure that even if type is BOOLEAN for
>> TRUTH-opcodes the operands getting boolified.
>> (gimplify_expr): Boolify operand condition for
>> COND_EXPR.
>> Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional
>> take care that we keep expression's type.
>>
>> Ok for apply?
>
> Posting patches inline makes it easier to put in review comments, so
> please consider doing that.
>
>
> Index: gcc/gcc/gimplify.c
> ===================================================================
> --- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200
> +++ gcc/gcc/gimplify.c 2011-05-10 21:14:49.106340400 +0200
> @@ -2824,11 +2824,20 @@ gimple_boolify (tree expr)
> }
> }
>
> - if (TREE_CODE (type) == BOOLEAN_TYPE)
> - return expr;
> -
> switch (TREE_CODE (expr))
> {
> + case COND_EXPR:
> + /* If we have a conditional expression, which shall be
> + boolean, take care we boolify also their left and right arm. */
> + if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P
> (TREE_TYPE (TREE_OPERAND (expr, 2))))
> + TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2));
> + if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P
> (TREE_TYPE (TREE_OPERAND (expr, 1))))
> + TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1));
> + TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
> + if (TREE_CODE (type) == BOOLEAN_TYPE)
> + return expr;
> + return fold_convert_loc (loc, boolean_type_node, expr);
> +
>
> That looks like premature optimization. Why isn't it enough to
> fold-convert the COND_EXPR itself? Thus, I don't see why the
> existing gimple_boolify isn't enough.
Old code simple casted condition expression to boolean type, but
didn't converted inner arms. By this the arms are remaining by their
old type, which later then gets converted. This is for a condition of
kind COND_EXPR pretty bad, as exactly this leads us to useless type
conversion in inner arms.
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> @@ -2851,6 +2860,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);
> }
> }
> @@ -6714,6 +6725,7 @@ gimplify_expr (tree *expr_p, gimple_seq
> break;
>
> case COND_EXPR:
> + TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0));
> ret = gimplify_cond_expr (expr_p, pre_p, fallback);
>
> This boolification should be done by gimplify_cond_expr as I said
> in the previous review. It does already handle this perfectly fine.
>
> /* C99 code may assign to an array in a structure value of a
> @@ -6762,6 +6774,17 @@ gimplify_expr (tree *expr_p, gimple_seq
>
> case TRUTH_ANDIF_EXPR:
> case TRUTH_ORIF_EXPR:
> + /* 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 (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;
> + }
> + *expr_p = gimple_boolify (*expr_p);
>
> This shouldn't be necessary at all. Instead gimplify_boolean_expr
> should deal with it - which it magically does by building a COND_EXPR
> which should already deal with all cases just fine.
No, see above the comment about COND_EXPR in gimple_boolify. This can
happen in cases an ANDIF/ORIF expression (which later gets and AND/OR)
would be on an arm of a COND_EXPR, which simply gets converted to
boolean by casting.
> /* Pass the source location of the outer expression. */
> ret = gimplify_boolean_expr (expr_p, saved_location);
> break;
> @@ -7203,6 +7226,17 @@ gimplify_expr (tree *expr_p, gimple_seq
> case TRUTH_AND_EXPR:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> + /* 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 (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;
> + }
> + *expr_p = gimple_boolify (*expr_p);
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 10:46 ` Eric Botcazou
@ 2011-05-11 11:12 ` Kai Tietz
2011-05-11 12:16 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 11:12 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther
2011/5/11 Eric Botcazou <ebotcazou@adacore.com>:
>> this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
>> expressions on gimplification to their binary form.
>
> What is it for? This will redirect the compilation stream from proven paths to
> others so there must be a good reason to do it. What's the effect on the code?
>
> --
> Eric Botcazou
Well, it would have some effects. First we don't need to handle TRUTH
and BINARY variants of AND, OR, XOR any longer special. Second cause
is that on BINARY trees the reassociation pass can operate, which
leads to better optimized boolean logic.
Regards,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 10:31 ` Kai Tietz
@ 2011-05-11 12:08 ` Richard Guenther
0 siblings, 0 replies; 26+ messages in thread
From: Richard Guenther @ 2011-05-11 12:08 UTC (permalink / raw)
To: Kai Tietz; +Cc: Paolo Bonzini, GCC Patches
On Wed, May 11, 2011 at 11:35 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> For more detail what happens and why this conditional handling is necessary:
>
> The sample code is:
>
> int
> foo (int a, int b)
> {
> return (a ? b != 3 : 0);
> }
>
> leads for variant without condition boolifying of arms to:
>
> ;; Function foo (foo)
>
> foo (int a, int b)
> {
> int D.1991;
> int D.1990;
> int D.1989;
>
> <bb 2>:
> D.1990_2 = a_1(D) != 0;
> D.1991_4 = b_3(D) != 3;
> D.1989_5 = D.1991_4 & D.1990_2;
> return D.1989_5;
>
> }
There is no TRUTH_* expr. The patch should fix TRUTH_* expr types,
not everything else (I know tcc_comparison exprs have the same issue).
If you want to fix tcc_comparison results as well do so in gimplify_expr
(and adjust the type verifier similar to the TRUTH_ case).
Richard.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 11:12 ` Kai Tietz
@ 2011-05-11 12:16 ` Richard Guenther
2011-05-11 19:39 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-11 12:16 UTC (permalink / raw)
To: Kai Tietz; +Cc: Eric Botcazou, gcc-patches
On Wed, May 11, 2011 at 12:03 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/11 Eric Botcazou <ebotcazou@adacore.com>:
>>> this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
>>> expressions on gimplification to their binary form.
>>
>> What is it for? This will redirect the compilation stream from proven paths to
>> others so there must be a good reason to do it. What's the effect on the code?
>>
>> --
>> Eric Botcazou
>
> Well, it would have some effects. First we don't need to handle TRUTH
> and BINARY variants of AND, OR, XOR any longer special. Second cause
> is that on BINARY trees the reassociation pass can operate, which
> leads to better optimized boolean logic.
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.
Richard.
> Regards,
> Kai
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 12:16 ` Richard Guenther
@ 2011-05-11 19:39 ` Kai Tietz
2011-05-12 10:32 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-11 19:39 UTC (permalink / raw)
To: Richard Guenther; +Cc: Eric Botcazou, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]
2011/5/11 Richard Guenther <richard.guenther@gmail.com>:
> 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.
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.
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;
+ }
+ }
/* 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);
[-- Attachment #2: truth_op_gimplify.txt --]
[-- Type: text/plain, Size: 2665 bytes --]
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
@@ -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:
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-11 19:39 ` Kai Tietz
@ 2011-05-12 10:32 ` Richard Guenther
2011-05-12 14:57 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-12 10:32 UTC (permalink / raw)
To: Kai Tietz; +Cc: Eric Botcazou, gcc-patches
On Wed, May 11, 2011 at 5:46 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/11 Richard Guenther <richard.guenther@gmail.com>:
>> 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);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-12 10:32 ` Richard Guenther
@ 2011-05-12 14:57 ` Kai Tietz
2011-05-12 15:41 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-12 14:57 UTC (permalink / raw)
To: Richard Guenther; +Cc: Eric Botcazou, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 5019 bytes --]
While testing some other issues with C++'s __java_boolean type
occurred. So I adjusted check in test-cfg.c as you suggested.
Additionally due the fact that we are now boolifying conditions for
even BOOLEAN_TYPE'ed cases (for making sure inner arms are boolified,
too), we possibly would alter here truth-type provided by FE. To
restore original type (for types != boolean-type), we do type
conversion always back to FE's used type for truth-AND/OR/XOR/etc as
result.
Patch bootstrapped with all languages on x86_64-pc-linux-gnu
(multilib). Ok for apply?
Regards,
Kai
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
+++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
+ && type == boolean_type_node)
+ return expr;
return fold_convert_loc (loc, boolean_type_node, expr);
}
}
@@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
return GS_OK;
}
-/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
- points to the expression to gimplify.
-
- Expressions of the form 'a && b' are gimplified to:
-
- a && b ? true : false
-
- LOCUS is the source location to be put on the generated COND_EXPR.
- gimplify_cond_expr will do the rest. */
-
-static enum gimplify_status
-gimplify_boolean_expr (tree *expr_p, location_t locus)
-{
- /* Preserve the original type of the expression. */
- tree type = TREE_TYPE (*expr_p);
-
- *expr_p = build3 (COND_EXPR, type, *expr_p,
- fold_convert_loc (locus, type, boolean_true_node),
- fold_convert_loc (locus, type, boolean_false_node));
-
- SET_EXPR_LOCATION (*expr_p, locus);
-
- return GS_OK;
-}
-
/* Gimplify an expression sequence. This function gimplifies each
expression and rewrites the original expression with the last
expression of the sequence in GIMPLE form.
@@ -6762,12 +6737,26 @@ 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)
+ if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
+ || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
{
tree type = TREE_TYPE (*expr_p);
*expr_p = fold_convert (type, gimple_boolify (*expr_p));
@@ -7203,6 +7192,24 @@ 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, we need to keep
+ orignal type. */
+ if (TREE_CODE (org_type) != BOOLEAN_TYPE
+ || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
+ {
+ *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-12 09:02:58.989243000 +0200
+++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
+ if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
+ || !useless_type_conversion_p (boolean_type_node, rhs2_type)
+ || !useless_type_conversion_p (boolean_type_node, lhs_type))
{
error ("type mismatch in binary truth expression");
debug_generic_expr (lhs_type);
[-- Attachment #2: thruth_conditions_gimple.txt --]
[-- Type: text/plain, Size: 4460 bytes --]
Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
+++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
+ && type == boolean_type_node)
+ return expr;
return fold_convert_loc (loc, boolean_type_node, expr);
}
}
@@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
return GS_OK;
}
-/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
- points to the expression to gimplify.
-
- Expressions of the form 'a && b' are gimplified to:
-
- a && b ? true : false
-
- LOCUS is the source location to be put on the generated COND_EXPR.
- gimplify_cond_expr will do the rest. */
-
-static enum gimplify_status
-gimplify_boolean_expr (tree *expr_p, location_t locus)
-{
- /* Preserve the original type of the expression. */
- tree type = TREE_TYPE (*expr_p);
-
- *expr_p = build3 (COND_EXPR, type, *expr_p,
- fold_convert_loc (locus, type, boolean_true_node),
- fold_convert_loc (locus, type, boolean_false_node));
-
- SET_EXPR_LOCATION (*expr_p, locus);
-
- return GS_OK;
-}
-
/* Gimplify an expression sequence. This function gimplifies each
expression and rewrites the original expression with the last
expression of the sequence in GIMPLE form.
@@ -6762,12 +6737,26 @@ 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)
+ if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
+ || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
{
tree type = TREE_TYPE (*expr_p);
*expr_p = fold_convert (type, gimple_boolify (*expr_p));
@@ -7203,6 +7192,24 @@ 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, we need to keep
+ orignal type. */
+ if (TREE_CODE (org_type) != BOOLEAN_TYPE
+ || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
+ {
+ *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-12 09:02:58.989243000 +0200
+++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
+ if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
+ || !useless_type_conversion_p (boolean_type_node, rhs2_type)
+ || !useless_type_conversion_p (boolean_type_node, lhs_type))
{
error ("type mismatch in binary truth expression");
debug_generic_expr (lhs_type);
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-12 14:57 ` Kai Tietz
@ 2011-05-12 15:41 ` Richard Guenther
2011-05-12 21:33 ` Kai Tietz
0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-12 15:41 UTC (permalink / raw)
To: Kai Tietz; +Cc: Eric Botcazou, gcc-patches
On Thu, May 12, 2011 at 3:29 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> While testing some other issues with C++'s __java_boolean type
> occurred. So I adjusted check in test-cfg.c as you suggested.
> Additionally due the fact that we are now boolifying conditions for
> even BOOLEAN_TYPE'ed cases (for making sure inner arms are boolified,
> too), we possibly would alter here truth-type provided by FE. To
> restore original type (for types != boolean-type), we do type
> conversion always back to FE's used type for truth-AND/OR/XOR/etc as
> result.
boolean_type_node is the only BOOLEAN_TYPE node we have,
so please remove the !=/== boolean_type_node checks again, or,
if you want more visual consistency with the adjustment gimple_boolify
makes replace them with !=/== boolean_type_node comparisons
completely.
Ok with either change.
Thanks,
Richard.
> Patch bootstrapped with all languages on x86_64-pc-linux-gnu
> (multilib). Ok for apply?
>
> Regards,
> Kai
>
> Index: gcc/gcc/gimplify.c
> ===================================================================
> --- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
> +++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
> + && type == boolean_type_node)
> + return expr;
> return fold_convert_loc (loc, boolean_type_node, expr);
> }
> }
> @@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
> return GS_OK;
> }
>
> -/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
> - points to the expression to gimplify.
> -
> - Expressions of the form 'a && b' are gimplified to:
> -
> - a && b ? true : false
> -
> - LOCUS is the source location to be put on the generated COND_EXPR.
> - gimplify_cond_expr will do the rest. */
> -
> -static enum gimplify_status
> -gimplify_boolean_expr (tree *expr_p, location_t locus)
> -{
> - /* Preserve the original type of the expression. */
> - tree type = TREE_TYPE (*expr_p);
> -
> - *expr_p = build3 (COND_EXPR, type, *expr_p,
> - fold_convert_loc (locus, type, boolean_true_node),
> - fold_convert_loc (locus, type, boolean_false_node));
> -
> - SET_EXPR_LOCATION (*expr_p, locus);
> -
> - return GS_OK;
> -}
> -
> /* Gimplify an expression sequence. This function gimplifies each
> expression and rewrites the original expression with the last
> expression of the sequence in GIMPLE form.
> @@ -6762,12 +6737,26 @@ 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)
> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
> + || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
> {
> tree type = TREE_TYPE (*expr_p);
> *expr_p = fold_convert (type, gimple_boolify (*expr_p));
> @@ -7203,6 +7192,24 @@ 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, we need to keep
> + orignal type. */
> + if (TREE_CODE (org_type) != BOOLEAN_TYPE
> + || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
> + {
> + *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-12 09:02:58.989243000 +0200
> +++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
> + || !useless_type_conversion_p (boolean_type_node, rhs2_type)
> + || !useless_type_conversion_p (boolean_type_node, lhs_type))
> {
> error ("type mismatch in binary truth expression");
> debug_generic_expr (lhs_type);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-12 15:41 ` Richard Guenther
@ 2011-05-12 21:33 ` Kai Tietz
2011-05-12 22:48 ` H.J. Lu
0 siblings, 1 reply; 26+ messages in thread
From: Kai Tietz @ 2011-05-12 21:33 UTC (permalink / raw)
To: Richard Guenther; +Cc: Eric Botcazou, gcc-patches
2011/5/12 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, May 12, 2011 at 3:29 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> While testing some other issues with C++'s __java_boolean type
>> occurred. So I adjusted check in test-cfg.c as you suggested.
>> Additionally due the fact that we are now boolifying conditions for
>> even BOOLEAN_TYPE'ed cases (for making sure inner arms are boolified,
>> too), we possibly would alter here truth-type provided by FE. To
>> restore original type (for types != boolean-type), we do type
>> conversion always back to FE's used type for truth-AND/OR/XOR/etc as
>> result.
>
> boolean_type_node is the only BOOLEAN_TYPE node we have,
> so please remove the !=/== boolean_type_node checks again, or,
> if you want more visual consistency with the adjustment gimple_boolify
> makes replace them with !=/== boolean_type_node comparisons
> completely.
>
> Ok with either change.
>
> Thanks,
> Richard.
>
>> Patch bootstrapped with all languages on x86_64-pc-linux-gnu
>> (multilib). Ok for apply?
>>
>> Regards,
>> Kai
>>
>> Index: gcc/gcc/gimplify.c
>> ===================================================================
>> --- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
>> +++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
>> + && type == boolean_type_node)
>> + return expr;
>> return fold_convert_loc (loc, boolean_type_node, expr);
>> }
>> }
>> @@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
>> return GS_OK;
>> }
>>
>> -/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
>> - points to the expression to gimplify.
>> -
>> - Expressions of the form 'a && b' are gimplified to:
>> -
>> - a && b ? true : false
>> -
>> - LOCUS is the source location to be put on the generated COND_EXPR.
>> - gimplify_cond_expr will do the rest. */
>> -
>> -static enum gimplify_status
>> -gimplify_boolean_expr (tree *expr_p, location_t locus)
>> -{
>> - /* Preserve the original type of the expression. */
>> - tree type = TREE_TYPE (*expr_p);
>> -
>> - *expr_p = build3 (COND_EXPR, type, *expr_p,
>> - fold_convert_loc (locus, type, boolean_true_node),
>> - fold_convert_loc (locus, type, boolean_false_node));
>> -
>> - SET_EXPR_LOCATION (*expr_p, locus);
>> -
>> - return GS_OK;
>> -}
>> -
>> /* Gimplify an expression sequence. This function gimplifies each
>> expression and rewrites the original expression with the last
>> expression of the sequence in GIMPLE form.
>> @@ -6762,12 +6737,26 @@ 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)
>> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
>> + || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
>> {
>> tree type = TREE_TYPE (*expr_p);
>> *expr_p = fold_convert (type, gimple_boolify (*expr_p));
>> @@ -7203,6 +7192,24 @@ 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, we need to keep
>> + orignal type. */
>> + if (TREE_CODE (org_type) != BOOLEAN_TYPE
>> + || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
>> + {
>> + *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-12 09:02:58.989243000 +0200
>> +++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
>> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
>> + || !useless_type_conversion_p (boolean_type_node, rhs2_type)
>> + || !useless_type_conversion_p (boolean_type_node, lhs_type))
>> {
>> error ("type mismatch in binary truth expression");
>> debug_generic_expr (lhs_type);
>>
>
Committed at revision 173711 with removing check for !=/== boolean_type_node.
Thanks,
Kai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-12 21:33 ` Kai Tietz
@ 2011-05-12 22:48 ` H.J. Lu
2011-05-13 10:07 ` Richard Guenther
0 siblings, 1 reply; 26+ messages in thread
From: H.J. Lu @ 2011-05-12 22:48 UTC (permalink / raw)
To: Kai Tietz; +Cc: Richard Guenther, Eric Botcazou, gcc-patches
On Thu, May 12, 2011 at 11:19 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/5/12 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, May 12, 2011 at 3:29 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> While testing some other issues with C++'s __java_boolean type
>>> occurred. So I adjusted check in test-cfg.c as you suggested.
>>> Additionally due the fact that we are now boolifying conditions for
>>> even BOOLEAN_TYPE'ed cases (for making sure inner arms are boolified,
>>> too), we possibly would alter here truth-type provided by FE. To
>>> restore original type (for types != boolean-type), we do type
>>> conversion always back to FE's used type for truth-AND/OR/XOR/etc as
>>> result.
>>
>> boolean_type_node is the only BOOLEAN_TYPE node we have,
>> so please remove the !=/== boolean_type_node checks again, or,
>> if you want more visual consistency with the adjustment gimple_boolify
>> makes replace them with !=/== boolean_type_node comparisons
>> completely.
>>
>> Ok with either change.
>>
>> Thanks,
>> Richard.
>>
>>> Patch bootstrapped with all languages on x86_64-pc-linux-gnu
>>> (multilib). Ok for apply?
>>>
>>> Regards,
>>> Kai
>>>
>>> Index: gcc/gcc/gimplify.c
>>> ===================================================================
>>> --- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
>>> +++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
>>> + && type == boolean_type_node)
>>> + return expr;
>>> return fold_convert_loc (loc, boolean_type_node, expr);
>>> }
>>> }
>>> @@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
>>> return GS_OK;
>>> }
>>>
>>> -/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
>>> - points to the expression to gimplify.
>>> -
>>> - Expressions of the form 'a && b' are gimplified to:
>>> -
>>> - a && b ? true : false
>>> -
>>> - LOCUS is the source location to be put on the generated COND_EXPR.
>>> - gimplify_cond_expr will do the rest. */
>>> -
>>> -static enum gimplify_status
>>> -gimplify_boolean_expr (tree *expr_p, location_t locus)
>>> -{
>>> - /* Preserve the original type of the expression. */
>>> - tree type = TREE_TYPE (*expr_p);
>>> -
>>> - *expr_p = build3 (COND_EXPR, type, *expr_p,
>>> - fold_convert_loc (locus, type, boolean_true_node),
>>> - fold_convert_loc (locus, type, boolean_false_node));
>>> -
>>> - SET_EXPR_LOCATION (*expr_p, locus);
>>> -
>>> - return GS_OK;
>>> -}
>>> -
>>> /* Gimplify an expression sequence. This function gimplifies each
>>> expression and rewrites the original expression with the last
>>> expression of the sequence in GIMPLE form.
>>> @@ -6762,12 +6737,26 @@ 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)
>>> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
>>> + || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
>>> {
>>> tree type = TREE_TYPE (*expr_p);
>>> *expr_p = fold_convert (type, gimple_boolify (*expr_p));
>>> @@ -7203,6 +7192,24 @@ 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, we need to keep
>>> + orignal type. */
>>> + if (TREE_CODE (org_type) != BOOLEAN_TYPE
>>> + || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
>>> + {
>>> + *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-12 09:02:58.989243000 +0200
>>> +++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
>>> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
>>> + || !useless_type_conversion_p (boolean_type_node, rhs2_type)
>>> + || !useless_type_conversion_p (boolean_type_node, lhs_type))
>>> {
>>> error ("type mismatch in binary truth expression");
>>> debug_generic_expr (lhs_type);
>>>
>>
>
> Committed at revision 173711 with removing check for !=/== boolean_type_node.
>
This caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48984
--
H.J.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011-05-12 22:48 ` H.J. Lu
@ 2011-05-13 10:07 ` Richard Guenther
0 siblings, 0 replies; 26+ messages in thread
From: Richard Guenther @ 2011-05-13 10:07 UTC (permalink / raw)
To: H.J. Lu; +Cc: Kai Tietz, Eric Botcazou, gcc-patches
On Thu, May 12, 2011 at 9:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 12, 2011 at 11:19 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/12 Richard Guenther <richard.guenther@gmail.com>:
>>> On Thu, May 12, 2011 at 3:29 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> While testing some other issues with C++'s __java_boolean type
>>>> occurred. So I adjusted check in test-cfg.c as you suggested.
>>>> Additionally due the fact that we are now boolifying conditions for
>>>> even BOOLEAN_TYPE'ed cases (for making sure inner arms are boolified,
>>>> too), we possibly would alter here truth-type provided by FE. To
>>>> restore original type (for types != boolean-type), we do type
>>>> conversion always back to FE's used type for truth-AND/OR/XOR/etc as
>>>> result.
>>>
>>> boolean_type_node is the only BOOLEAN_TYPE node we have,
>>> so please remove the !=/== boolean_type_node checks again, or,
>>> if you want more visual consistency with the adjustment gimple_boolify
>>> makes replace them with !=/== boolean_type_node comparisons
>>> completely.
>>>
>>> Ok with either change.
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Patch bootstrapped with all languages on x86_64-pc-linux-gnu
>>>> (multilib). Ok for apply?
>>>>
>>>> Regards,
>>>> Kai
>>>>
>>>> Index: gcc/gcc/gimplify.c
>>>> ===================================================================
>>>> --- gcc.orig/gcc/gimplify.c 2011-05-12 09:02:58.946243000 +0200
>>>> +++ gcc/gcc/gimplify.c 2011-05-12 15:13:59.534550700 +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,9 @@ 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
>>>> + && type == boolean_type_node)
>>>> + return expr;
>>>> return fold_convert_loc (loc, boolean_type_node, expr);
>>>> }
>>>> }
>>>> @@ -4695,31 +4695,6 @@ gimplify_scalar_mode_aggregate_compare (
>>>> return GS_OK;
>>>> }
>>>>
>>>> -/* Gimplify TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR expressions. EXPR_P
>>>> - points to the expression to gimplify.
>>>> -
>>>> - Expressions of the form 'a && b' are gimplified to:
>>>> -
>>>> - a && b ? true : false
>>>> -
>>>> - LOCUS is the source location to be put on the generated COND_EXPR.
>>>> - gimplify_cond_expr will do the rest. */
>>>> -
>>>> -static enum gimplify_status
>>>> -gimplify_boolean_expr (tree *expr_p, location_t locus)
>>>> -{
>>>> - /* Preserve the original type of the expression. */
>>>> - tree type = TREE_TYPE (*expr_p);
>>>> -
>>>> - *expr_p = build3 (COND_EXPR, type, *expr_p,
>>>> - fold_convert_loc (locus, type, boolean_true_node),
>>>> - fold_convert_loc (locus, type, boolean_false_node));
>>>> -
>>>> - SET_EXPR_LOCATION (*expr_p, locus);
>>>> -
>>>> - return GS_OK;
>>>> -}
>>>> -
>>>> /* Gimplify an expression sequence. This function gimplifies each
>>>> expression and rewrites the original expression with the last
>>>> expression of the sequence in GIMPLE form.
>>>> @@ -6762,12 +6737,26 @@ 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)
>>>> + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE
>>>> + || TREE_CODE (TREE_TYPE (*expr_p)) != boolean_type_node)
>>>> {
>>>> tree type = TREE_TYPE (*expr_p);
>>>> *expr_p = fold_convert (type, gimple_boolify (*expr_p));
>>>> @@ -7203,6 +7192,24 @@ 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, we need to keep
>>>> + orignal type. */
>>>> + if (TREE_CODE (org_type) != BOOLEAN_TYPE
>>>> + || TREE_CODE (TREE_TYPE (org_type)) != boolean_type_node)
>>>> + {
>>>> + *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-12 09:02:58.989243000 +0200
>>>> +++ gcc/gcc/tree-cfg.c 2011-05-12 14:50:19.656249100 +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 or compatible argument and result. */
>>>> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
>>>> + || !useless_type_conversion_p (boolean_type_node, rhs2_type)
>>>> + || !useless_type_conversion_p (boolean_type_node, lhs_type))
>>>> {
>>>> error ("type mismatch in binary truth expression");
>>>> debug_generic_expr (lhs_type);
>>>>
>>>
>>
>> Committed at revision 173711 with removing check for !=/== boolean_type_node.
>>
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48984
Appearantly a check for boolean_type_node would have been better
then. It seems the Fortran FE has multiple BOOLEAN_TYPE
types.
Richard.
>
> --
> H.J.
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2011-05-13 9:05 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-10 14:25 [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary Kai Tietz
2011-05-10 15:24 ` Richard Guenther
2011-05-10 15:34 ` Kai Tietz
2011-05-10 15:38 ` Richard Guenther
2011-05-10 15:45 ` Paolo Bonzini
2011-05-10 15:52 ` Richard Guenther
2011-05-10 17:28 ` Kai Tietz
2011-05-10 20:49 ` Kai Tietz
2011-05-11 9:35 ` Richard Guenther
2011-05-11 10:06 ` Kai Tietz
2011-05-11 10:51 ` Kai Tietz
2011-05-11 10:00 ` Kai Tietz
2011-05-11 10:03 ` Richard Guenther
2011-05-11 10:12 ` Kai Tietz
2011-05-11 10:31 ` Kai Tietz
2011-05-11 12:08 ` Richard Guenther
2011-05-11 10:46 ` Eric Botcazou
2011-05-11 11:12 ` Kai Tietz
2011-05-11 12:16 ` Richard Guenther
2011-05-11 19:39 ` Kai Tietz
2011-05-12 10:32 ` Richard Guenther
2011-05-12 14:57 ` Kai Tietz
2011-05-12 15:41 ` Richard Guenther
2011-05-12 21:33 ` Kai Tietz
2011-05-12 22:48 ` H.J. Lu
2011-05-13 10:07 ` Richard Guenther
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).