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]

[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" } } */


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