This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR/18308: ice-on-valid because of non-GIMPLE (take 2)
- From: Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>, Roger Sayle <roger at eyesopen dot com>
- Date: Tue, 04 Jan 2005 14:40:56 +0100
- Subject: Re: PR/18308: ice-on-valid because of non-GIMPLE (take 2)
- References: <41D27969.3030902@lu.unisi.ch> <Pine.LNX.4.44.0412290258140.13835-100000@www.eyesopen.com>
> 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();
}