Patchwork [gimplifier] : Boolify more strict conditional expressions and transform simple form to binary

login
register
mail settings
Submitter Kai Tietz
Date May 11, 2011, 3:46 p.m.
Message ID <BANLkTi=QeAe4ZMjoq_YdOp06FyHD9K-zvQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/95238/
State New
Headers show

Comments

Kai Tietz - May 11, 2011, 3:46 p.m.
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-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:
Richard Guenther - May 12, 2011, 8:40 a.m.
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);
>

Patch

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);