This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[patch] Call fold_expr_cond during final_cleanup (PR 20139)
- From: Kazu Hirata <kazu at codesourcery dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: law at redhat dot com
- Cc: dnovillo at redhat dot com
- Date: Fri, 8 Jul 2005 07:12:06 -0700
- Subject: [patch] Call fold_expr_cond during final_cleanup (PR 20139)
Hi,
Attached is a patch to call fold_expr_cond during final_cleanup.
Consider:
void
foo (double x)
{
double p, q;
p = fabs (x);
q = 0.0;
if (p < q)
link_error ();
}
p < q is always true even when x is NaN (see
gcc.c-torture/execute/20020720-1.c), so we could remove the "if"
statement along with a call to link_error.
The problem is that I accidentally killed this optimization at tree
level, which made the testcase's fate depend on whether RTL
optimizations can optimize away the "if" statement.
The patch fixes this problem by folding all of COND_EXPR_COND after
out-of-ssa. Here are some justifications for doing so.
- PR tree-optimization/20139 is a regression.
- We could try to combine "p = fabs (x);" and "if (p < 0.0)" to fold
the "if" statement, but I don't think we will have a full fledged tree
combiner before stage2 ends.
- We could propagate the values of p and q in VRP and fold the "if"
statement, but I don't think it's worth extending VRP to handle
floating point numbers. Even if it's worth the effort, we are not
going to have it before stage2 ends.
- Performance impact is within an error, at least last time I tried
this a couple of months ago.
There is one latent bug we have to fix. remove_bb calls release_defs
regardless of whether we are in SSA or not. If we are not in SSA, the
operand cache is invalid, so calling release_defs does not make sense.
(Actually, it's even dangerous to call release_defs because operand
cache infrastructure manages its memory on their own within GC, so if
we try to access operand cache after out-of-SSA, we may dereference a
pointer like 0xa5a5a5a5, causing a segfault.) The patch fixes this
problem by checking in_ssa_p before calling release_defs.
Tested on x86_64-pc-linux-gnu on top of
http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00566.html
OK to apply?
p.s.
After this patch goes in, I'm going to post a patch to remove
gcc.c-torture/execute/20020720-1.x as we can expect the optimization
to happen at tree level.
Kazu Hirata
2005-07-08 Kazu Hirata <kazu@codesourcery.com>
PR tree-optimization/20139
* tree-cfg.c (remove_bb): Check in_ssa_p before calling
release_defs.
* tree-optimize.c (execute_cleanup_cfg_post_optimizing): Call
fold_cond_expr_cond.
* tree-ssanames.c (release_defs): Assert in_ssa_p.
* tree.c (upper_bound_in_type, lower_bound_in_type): Rewrite.
2005-07-08 Kazu Hirata <kazu@codesourcery.com>
PR tree-optimization/20139
* gcc.dg/tree-ssa/pr20139.c: New.
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.208
diff -c -d -p -r2.208 tree-cfg.c
*** tree-cfg.c 3 Jul 2005 21:08:01 -0000 2.208
--- tree-cfg.c 8 Jul 2005 01:49:35 -0000
*************** remove_bb (basic_block bb)
*** 1981,1987 ****
}
else
{
! release_defs (stmt);
bsi_remove (&i);
}
--- 1981,1992 ----
}
else
{
! /* Release SSA definitions if we are in SSA. Note that we
! may be called when not in SSA. For example,
! final_cleanup calls this function via
! cleanup_tree_cfg. */
! if (in_ssa_p)
! release_defs (stmt);
bsi_remove (&i);
}
Index: tree-optimize.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-optimize.c,v
retrieving revision 2.118
diff -c -d -p -r2.118 tree-optimize.c
*** tree-optimize.c 5 Jul 2005 16:20:24 -0000 2.118
--- tree-optimize.c 8 Jul 2005 01:49:35 -0000
*************** struct tree_opt_pass pass_cleanup_cfg =
*** 132,137 ****
--- 132,138 ----
static void
execute_cleanup_cfg_post_optimizing (void)
{
+ fold_cond_expr_cond ();
cleanup_tree_cfg ();
cleanup_dead_labels ();
group_case_labels ();
Index: tree-ssanames.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssanames.c,v
retrieving revision 2.27
diff -c -d -p -r2.27 tree-ssanames.c
*** tree-ssanames.c 25 Jun 2005 02:01:49 -0000 2.27
--- tree-ssanames.c 8 Jul 2005 01:49:35 -0000
*************** release_defs (tree stmt)
*** 289,294 ****
--- 289,298 ----
tree def;
ssa_op_iter iter;
+ /* Make sure that we are in SSA. Otherwise, operand cache may point
+ to garbage. */
+ gcc_assert (in_ssa_p);
+
FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
if (TREE_CODE (def) == SSA_NAME)
release_ssa_name (def);
--- /dev/null 2004-06-24 11:05:26.000000000 -0700
+++ testsuite/gcc.dg/tree-ssa/pr20139.c 2005-07-04 20:54:15.000000000 -0700
@@ -0,0 +1,23 @@
+/* PR tree-optimization/20139
+ This testcase is derived from gcc.dg/20020720-1.c. Here we check
+ that the optimization happens at tree level. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-final_cleanup" } */
+
+extern double fabs (double);
+extern void link_error (void);
+
+void
+foo (double x)
+{
+ double p, q;
+
+ p = fabs (x);
+ q = 0.0;
+ if (p < q)
+ link_error ();
+}
+
+/* { dg-final { scan-tree-dump-times "link_error" 0 "final_cleanup" } } */
+/* { dg-final { cleanup-tree-dump "final_cleanup" } } */