This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR/18308: ice-on-valid because of non-GIMPLE (take 2)


> I'm happy to consider ICEing in dojump.c a regression for GCC 4.0;
We've never ICE'd on a COND_EXPR in do_jump in any previous release.

Please bootstrap and regression test an updated patch, and if/when
it completes without any new failures, repost it to gcc-patches.

Here it is, bootstrapped/regtested on powerpc-darwin.


I've chosen not to reinstate the full logic of COND_EXPR expansion, but only a subset. The original code included this snippet:

      /* Do (a ? 1 : 0) and (a ? 0 : 1) as special cases.  */
      if (integer_onep (TREE_OPERAND (exp, 1))
          && integer_zerop (TREE_OPERAND (exp, 2)))
        do_jump (TREE_OPERAND (exp, 0), if_false_label, if_true_label);

      else if (integer_zerop (TREE_OPERAND (exp, 1))
               && integer_onep (TREE_OPERAND (exp, 2)))
        do_jump (TREE_OPERAND (exp, 0), if_true_label, if_false_label);

This code is obviously optimizing less than it could: for example none of (a ? 2 : 0) or (a ? 2 : 1) is short-circuited. So, I considered either using integer_nonzerop instead of integer_onep, or writing more complex code that would obtain optimal jump threading out of the expander. The former still would not have caught (a ? 2 : 1), which I guess was actually quite common without gimplification; and the latter would be inappropriate at this point, also because COND_EXPRs are only produced by loop-level if-conversion so it is pretty hard to test.

So, I decided just to remove these two cases for the sake of clarity. Unlike the previous take of this patch, this "suboptimality" can be fixed even with a single cfg cleanup, which will be able to merge the empty basic blocks.

Ok for HEAD?

Paolo
2004-12-28  Paolo Bonzini  <bonzini@gnu.org>
	    Devang Patel  <dpatel@apple.com>

	PR tree-optimization/18308

	* tree-if-conv.c (add_to_dst_predicate_list): Gimplify
	the operands before creating a new expression.
	* dojump.c (do_jump): Make drop_through_label available
	for all cases.  Add expansion of COND_EXPR.

2004-12-28  Paolo Bonzini  <bonzini@gnu.org>

	* gcc.dg/vect/pr18308.c: New testcase.

Index: gcc/dojump.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/dojump.c,v
retrieving revision 1.35
diff -u -r1.35 dojump.c
--- gcc/dojump.c	23 Dec 2004 13:02:26 -0000	1.35
+++ gcc/dojump.c	4 Jan 2005 10:12:56 -0000
@@ -1,6 +1,6 @@
 /* Convert tree expression to rtl instructions, for GNU compiler.
    Copyright (C) 1988, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
-   2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
+   2000, 2001, 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -164,6 +164,7 @@
   int i;
   tree type;
   enum machine_mode mode;
+  rtx drop_through_label = 0;
 
   switch (code)
     {
@@ -293,10 +294,29 @@
       do_jump (TREE_OPERAND (exp, 0), if_true_label, if_false_label);
       break;
 
+    case COND_EXPR:
+      {
+	rtx label1 = gen_label_rtx ();
+	if (!if_true_label || !if_false_label)
+	  {
+	    drop_through_label = gen_label_rtx ();
+	    if (!if_true_label)
+	      if_true_label = drop_through_label;
+	    if (!if_false_label)
+	      if_false_label = drop_through_label;
+	  }
+
+        do_pending_stack_adjust ();
+        do_jump (TREE_OPERAND (exp, 0), label1, NULL_RTX);
+        do_jump (TREE_OPERAND (exp, 1), if_false_label, if_true_label);
+        emit_label (label1);
+        do_jump (TREE_OPERAND (exp, 2), if_false_label, if_true_label);
+	break;
+      }
+
     case TRUTH_ANDIF_EXPR:
     case TRUTH_ORIF_EXPR:
     case COMPOUND_EXPR:
-    case COND_EXPR:
       /* Lowered by gimplify.c.  */
       gcc_unreachable ();
 
@@ -478,7 +498,6 @@
             tree op0 = save_expr (TREE_OPERAND (exp, 0));
             tree op1 = save_expr (TREE_OPERAND (exp, 1));
             tree cmp0, cmp1;
-	    rtx drop_through_label = 0;
 
             /* If the target doesn't support combined unordered
                compares, decompose into two comparisons.  */
@@ -489,12 +508,6 @@
             cmp1 = fold (build2 (tcode2, TREE_TYPE (exp), op0, op1));
 	    do_jump (cmp0, 0, if_true_label);
 	    do_jump (cmp1, if_false_label, if_true_label);
-
-	    if (drop_through_label)
-	      {
-		do_pending_stack_adjust ();
-		emit_label (drop_through_label);
-	      }
           }
       }
       break;
@@ -568,6 +581,12 @@
 				   if_false_label, if_true_label);
 	}
     }
+
+  if (drop_through_label)
+    {
+      do_pending_stack_adjust ();
+      emit_label (drop_through_label);
+    }
 }
 
 /* Given a comparison expression EXP for values too wide to be compared
Index: gcc/tree-if-conv.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-if-conv.c,v
retrieving revision 2.24
diff -u -r2.24 tree-if-conv.c
--- gcc/tree-if-conv.c	29 Nov 2004 17:53:46 -0000	2.24
+++ gcc/tree-if-conv.c	4 Jan 2005 09:50:40 -0000
@@ -1,5 +1,5 @@
 /* If-conversion for vectorizer.
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005 Free Software Foundation, Inc.
    Contributed by Devang Patel <dpatel@apple.com>
 
 This file is part of GCC.
@@ -639,7 +639,19 @@
     new_cond = unshare_expr (cond);
   else
     {
-      tree tmp_stmt;
+      tree tmp_stmt = NULL_TREE;
+      tree tmp_stmts1 = NULL_TREE;
+      tree tmp_stmts2 = NULL_TREE;
+      prev_cond = force_gimple_operand (unshare_expr (prev_cond),
+					&tmp_stmts1, true, NULL);
+      if (tmp_stmts1)
+        bsi_insert_before (bsi, tmp_stmts1, BSI_SAME_STMT);
+
+      cond = force_gimple_operand (unshare_expr (cond),
+				   &tmp_stmts2, true, NULL);
+      if (tmp_stmts2)
+        bsi_insert_before (bsi, tmp_stmts2, BSI_SAME_STMT);
+
       /* new_cond == prev_cond AND cond */
       tree tmp = build (TRUTH_AND_EXPR, boolean_type_node,
 			unshare_expr (prev_cond), cond);



/* { dg-do compile } */
/* { dg-options "-O -ftree-vectorize -funroll-loops" } */
void foo();

void bar(int j)
{
  int i, k=0;
  for (i = 0; i < 2; ++i)
    if (j) k = 2;

  if (k) foo();
}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]