Re: [patch] fix PR34005 - ICE due to rhs cond_expr (gimplifier patch)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Nov 15, 2007 at 02:19:21PM -0500, Diego Novillo wrote:
> Dorit Nuzman wrote:
> 
> >        PR tree-optimization/34005
> >        * tree-gimple.c (is_gimple_formal_tmp_rhs): Add a case for
> >COND_EXPR.
> >        * gimplify.c (gimplify_ctx): Add a new member allow_rhs_cond_expr.
> >        (gimplify_pure_cond_expr): New function.
> >        (expression_without_side_effects_p): New function.
> >        (gimplify_cond_expr): Call implify_pure_cond_expr.
>                                      gimplify_pure_cond_expr

Updated.

> >+ static bool
> >+ expression_without_side_effects_p (tree expr)
> 
> Hmm, why isn't TREE_SIDE_EFFECTS sufficient?

As clarified by Zdenek in bugzilla, this really is not about side-effects
in the TREE_SIDE_EFFECTS sense, but whether the expression (not gimplified
yet) could trap or not.  Transforming
x = (p == NULL) ? "" : *p;
into
a = "";
b = *p;
cond = (p == NULL);
x = cond ? a : b;
is wrong, even when !TREE_SIDE_EFFECTS.

> >+       if (gimplify_ctxp->allow_rhs_cond_expr
> >+           && !TREE_SIDE_EFFECTS (*expr_p)
> >+           && expression_without_side_effects_p (*expr_p))

The patch below just renames the function to expression_could_trap_p
to make it clearer what it does and also doesn't care whether COND_EXPR
condition has side effects or can trap (because it doesn't matter, all
we need to prevent is evaluating the COND_EXPR branches if they can
have any side effects).

Following was bootstrapped/regtested on x86_64-linux, ok for trunk?

2007-11-28  Zdenek Dvorak  <ook@ucw.cz>
	    Dorit Nuzman  <dorit@il.ibm.com>
	    Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/34005
	* tree-gimple.c (is_gimple_formal_tmp_rhs): Add a case for COND_EXPR.
	* gimplify.c (gimplify_ctx): Add a new member allow_rhs_cond_expr.
	(gimplify_pure_cond_expr): New function.
	(expression_could_trap_p): New function.
	(gimplify_cond_expr): Call gimplify_pure_cond_expr.
	(force_gimple_operand): Initialize new field allow_rhs_cond_expr.

2007-11-28  Martin Michlmayr <tbm@cyrius.com>
	    Dorit Nuzman  <dorit@il.ibm.com>

	PR tree-optimization/34005
	* gcc.dg/vect/pr34005.c: New test.

--- gcc/gimplify.c.jj	2007-11-26 11:02:25.000000000 +0100
+++ gcc/gimplify.c	2007-11-28 14:52:58.000000000 +0100
@@ -94,6 +94,7 @@ struct gimplify_ctx
   int conditions;
   bool save_stack;
   bool into_ssa;
+  bool allow_rhs_cond_expr;
 };
 
 static struct gimplify_ctx *gimplify_ctxp;
@@ -2546,6 +2547,59 @@ gimple_boolify (tree expr)
     }
 }
 
+/* Given a conditional expression *EXPR_P without side effects, gimplify
+   its operands.  New statements are inserted to PRE_P.  */
+
+static enum gimplify_status
+gimplify_pure_cond_expr (tree *expr_p, tree *pre_p)
+{
+  tree expr = *expr_p, cond;
+  enum gimplify_status ret, tret;
+  enum tree_code code;
+
+  cond = 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.  */
+  code = TREE_CODE (cond);
+  if (code == TRUTH_ANDIF_EXPR)
+    TREE_SET_CODE (cond, TRUTH_AND_EXPR);
+  else if (code == TRUTH_ORIF_EXPR)
+    TREE_SET_CODE (cond, TRUTH_OR_EXPR);
+  ret = gimplify_expr (&cond, pre_p, NULL,
+                              is_gimple_condexpr, fb_rvalue);
+  COND_EXPR_COND (*expr_p) = cond;
+
+  tret = gimplify_expr (&COND_EXPR_THEN (expr), pre_p, NULL,
+                                   is_gimple_val, fb_rvalue);
+  ret = MIN (ret, tret);
+  tret = gimplify_expr (&COND_EXPR_ELSE (expr), pre_p, NULL,
+                                   is_gimple_val, fb_rvalue);
+
+  return MIN (ret, tret);
+}
+
+/* Returns true if evaluating EXPR could trap.  */
+
+static bool
+expression_could_trap_p (tree expr)
+{
+  unsigned i, n;
+
+  if (!expr || is_gimple_val (expr))
+    return false;
+
+  if (!EXPR_P (expr) || tree_could_trap_p (expr))
+    return true;
+
+  n = TREE_OPERAND_LENGTH (expr);
+  for (i = 0; i < n; i++)
+    if (expression_could_trap_p (TREE_OPERAND (expr, i)))
+      return true;
+
+  return false;
+}
+
 /*  Convert the conditional expression pointed to by EXPR_P '(p) ? a : b;'
     into
 
@@ -2579,6 +2633,15 @@ gimplify_cond_expr (tree *expr_p, tree *
 
       if ((fallback & fb_lvalue) == 0)
 	{
+	  if (gimplify_ctxp->allow_rhs_cond_expr
+	      /* If either branch has side effects or could trap, it can't be
+		 evaluated unconditionally.  */
+	      && !TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p, 1))
+	      && !expression_could_trap_p (TREE_OPERAND (*expr_p, 1))
+	      && !TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p, 2))
+	      && !expression_could_trap_p (TREE_OPERAND (*expr_p, 2)))
+	    return gimplify_pure_cond_expr (expr_p, pre_p);
+
 	  result = tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
 	  ret = GS_ALL_DONE;
 	}
@@ -2593,7 +2656,7 @@ gimplify_cond_expr (tree *expr_p, tree *
 	  if (TREE_TYPE (TREE_OPERAND (expr, 2)) != void_type_node)
 	    TREE_OPERAND (expr, 2) =
 	      build_fold_addr_expr (TREE_OPERAND (expr, 2));
-	  
+
 	  tmp2 = tmp = create_tmp_var (type, "iftmp");
 
 	  expr = build3 (COND_EXPR, void_type_node, TREE_OPERAND (expr, 0),
@@ -6444,6 +6507,7 @@ force_gimple_operand (tree expr, tree *s
 
   push_gimplify_context ();
   gimplify_ctxp->into_ssa = gimple_in_ssa_p (cfun);
+  gimplify_ctxp->allow_rhs_cond_expr = true;
 
   if (var)
     expr = build_gimple_modify_stmt (var, expr);
--- gcc/tree-gimple.c.jj	2007-10-30 11:46:29.000000000 +0100
+++ gcc/tree-gimple.c	2007-11-28 14:47:37.000000000 +0100
@@ -61,6 +61,7 @@ is_gimple_formal_tmp_rhs (tree t)
     case TRUTH_AND_EXPR:
     case TRUTH_OR_EXPR:
     case TRUTH_XOR_EXPR:
+    case COND_EXPR:
     case ADDR_EXPR:
     case CALL_EXPR:
     case CONSTRUCTOR:
--- gcc/testsuite/gcc.dg/vect/pr34005.c.jj	2007-11-20 11:25:52.515489246 +0100
+++ gcc/testsuite/gcc.dg/vect/pr34005.c	2007-11-28 14:47:37.000000000 +0100
@@ -0,0 +1,15 @@
+/* PR tree-optimization/34005 */
+/* { dg-do compile } */
+
+/* Testcase by Martin Michlmayr <tbm@cyrius.com> */
+
+void XdmcpUnwrap (unsigned char *output, int k)
+{
+  int i;
+  unsigned char blocks[2][8];
+  k = (k == 0) ? 1 : 0;
+  for (i = 0; i < 32; i++)
+    output[i] = blocks[k][i];
+}
+
+/* { dg-final { cleanup-tree-dump "vect" } } */

	Jakub