Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Thu, May 12, 2011 at 9:59 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Thu, May 12, 2011 at 11:19 AM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/12 Richard Guenther richard.guent...@gmail.com:
On Thu, May 12, 2011 at 3:29 PM, Kai Tietz ktiet...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Wed, May 11, 2011 at 5:46 PM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/11 Richard Guenther richard.guent...@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 =
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
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:
{
-
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Thu, May 12, 2011 at 3:29 PM, Kai Tietz ktiet...@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;
+
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/12 Richard Guenther richard.guent...@gmail.com:
On Thu, May 12, 2011 at 3:29 PM, Kai Tietz ktiet...@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 =
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Thu, May 12, 2011 at 11:19 AM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/12 Richard Guenther richard.guent...@gmail.com:
On Thu, May 12, 2011 at 3:29 PM, Kai Tietz ktiet...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Tue, May 10, 2011 at 9:26 PM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/10 Kai Tietz ktiet...@googlemail.com:
2011/5/10 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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.0 +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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
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
Index: gcc/gcc/gimplify.c
===
--- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.0 +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 =
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Wed, May 11, 2011 at 11:00 AM, Kai Tietz ktiet...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/11 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 9:26 PM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/10 Kai Tietz ktiet...@googlemail.com:
2011/5/10 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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.0 +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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/11 Richard Guenther richard.guent...@gmail.com:
On Wed, May 11, 2011 at 11:00 AM, Kai Tietz ktiet...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/11 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 9:26 PM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/10 Kai Tietz ktiet...@googlemail.com:
2011/5/10 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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.0 +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,
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/11 Eric Botcazou ebotca...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Wed, May 11, 2011 at 11:35 AM, Kai Tietz ktiet...@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.
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Wed, May 11, 2011 at 12:03 PM, Kai Tietz ktiet...@googlemail.com wrote: 2011/5/11 Eric Botcazou ebotca...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/11 Richard Guenther richard.guent...@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);
Index: gcc/gcc/gimplify.c
===
--- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.0 +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);
+
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Tue, May 10, 2011 at 3:56 PM, Kai Tietz ktiet...@googlemail.com wrote:
Hello,
this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
expressions
on gimplification to their binary form. Additionally it takes care
that conditions
are getting boolified for operation.
ChangeLog
2011-05-10 Kai Tietz
* gimplify.c (gimplify_exit_expr): Boolify conditional
expression part.
(shortcut_cond_r): Likewise.
(shortcut_cond_expr): Likewise.
(gimplify_cond_expr): Likewise.
(gimplify_modify_expr_rhs): Likewise.
(gimplify_boolean_expr): Likewise.
(gimple_boolify): Boolify operands for BOOLEAN typed
base expressions.
(gimplify_expr): Boolify TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR,
TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR. Additionally
move TRUTH_AND|OR|XOR_EXPR to its binary form.
Tested for x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
Comments inline (the attachment makes my comments harder to spot,
look close).
Index: gcc/gcc/gimplify.c
===
--- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.0 +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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Tue, May 10, 2011 at 5:08 PM, Kai Tietz ktiet...@googlemail.com wrote:
2011/5/10 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 3:56 PM, Kai Tietz ktiet...@googlemail.com wrote:
Hello,
this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR
expressions
on gimplification to their binary form. Additionally it takes care
that conditions
are getting boolified for operation.
ChangeLog
2011-05-10 Kai Tietz
* gimplify.c (gimplify_exit_expr): Boolify conditional
expression part.
(shortcut_cond_r): Likewise.
(shortcut_cond_expr): Likewise.
(gimplify_cond_expr): Likewise.
(gimplify_modify_expr_rhs): Likewise.
(gimplify_boolean_expr): Likewise.
(gimple_boolify): Boolify operands for BOOLEAN typed
base expressions.
(gimplify_expr): Boolify TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR,
TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR. Additionally
move TRUTH_AND|OR|XOR_EXPR to its binary form.
Tested for x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
Comments inline (the attachment makes my comments harder to spot,
look close).
Index: gcc/gcc/gimplify.c
===
--- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.0 +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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/10 Richard Guenther richard.guent...@gmail.com: On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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
Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary
2011/5/10 Kai Tietz ktiet...@googlemail.com:
2011/5/10 Richard Guenther richard.guent...@gmail.com:
On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini bonz...@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
Index: gcc/gcc/gimplify.c
===
--- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.0 +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 =
