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: [PATCH] Fix ICE in expand_cse_reciprocals (PR tree-optimization/42078)


On Nov 18, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

> IMHO, the problem is that arg1 is being reused to hold its own
> reciprocal.  Should a different assignment of a newly-created SSA_NAME
> be created instead, debug bind stmts would be automagically fixed upon
> removal or replacement of the original assignment.

I offer two patches below.

They're built upon the understanding that reusing an SSA name's DEF for
a different value is not kosher: it fails to update debug stmts and it
might wreak havoc in any additional side information.

gsi_replace will refuse to replace a stmt with one that sets a different
SSA name, for similar reasons.

The first patch reimplements the code that modified the call stmt in
place, copying it instead, modifying the copy to set a different
variable (just a different SSA name (*), in this case) to the modified
value, inserting the copy before the original, and then removing the
original stmt.

Debug stmts that refer to the removed DEF are reset with the patch
above.  In order to enable the automatic propagation of debug stmts (and
any other code that might be attempting to log or make sense of the
transformations for any other purpose), we have to tell the compiler how
the old DEF relates with the new one before removing the old one.  This
is what the second patch implements.

Is the 1st ok to install?  How about the 2nd, on top of the 1st?


(*) using the same SSA base variable is a bit of a hack.  It might be
easier on the eyes of someone looking at the dumps to actually create a
tmp_var named after ârecipâ or somesuch.  I couldn't convince myself
this would do us significant good, and this code was slightly shorter,
so I went ahead with it.

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/42078
	* tree-ssa-math-opts.c (execute_cse_reciprocals): Reset debug
	stmts that refer to the original DEFs.

for  gcc/testsuite/ChangeLog
from  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/42078
	* gcc.dg/pr42078.c: New test.

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c.orig	2009-11-18 22:25:06.000000000 -0200
+++ gcc/tree-ssa-math-opts.c	2009-11-19 00:28:05.000000000 -0200
@@ -534,6 +534,9 @@ execute_cse_reciprocals (void)
 		  bool md_code, fail;
 		  imm_use_iterator ui;
 		  use_operand_p use_p;
+		  gimple_stmt_iterator gsi1;
+		  gimple rstmt1;
+		  tree rarg1;
 
 		  code = DECL_FUNCTION_CODE (fndecl);
 		  md_code = DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD;
@@ -563,12 +566,22 @@ execute_cse_reciprocals (void)
 		  if (fail)
 		    continue;
 
-		  gimple_call_set_fndecl (stmt1, fndecl);
-		  update_stmt (stmt1);
+		  rstmt1 = gimple_copy (stmt1);
+		  rarg1 = duplicate_ssa_name (arg1, rstmt1);
+		  gimple_call_set_lhs (rstmt1, rarg1);
+		  gimple_call_set_fndecl (rstmt1, fndecl);
+
+		  gsi1 = gsi_for_stmt (stmt1);
+		  gsi_insert_before (&gsi1, rstmt1, true);
+		  /* This will reset debug stmts that refer to arg1,
+		     so we don't have to check for them in the loop
+		     below.  */
+		  gsi_remove (&gsi1, true);
 
 		  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
 		    {
 		      gimple_assign_set_rhs_code (stmt, MULT_EXPR);
+		      gimple_assign_set_rhs2 (stmt, rarg1);
 		      fold_stmt_inplace (stmt);
 		      update_stmt (stmt);
 		    }
Index: gcc/testsuite/gcc.dg/pr42078.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/pr42078.c	2009-11-19 00:27:11.000000000 -0200
@@ -0,0 +1,22 @@
+/* PR tree-optimization/42078 */
+/* { dg-do compile } */
+/* { dg-options "-g -O -ffast-math" } */
+
+double sqrt (double x);
+
+float
+foo (float x)
+{
+  float y = sqrt (x);
+  return x / y;
+}
+
+inline float
+bar (float x)
+{
+  float y = sqrt (x);
+  float a = y;
+  float b = y;
+  float c = y;
+  return x / y;
+}
for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/42078
	* tree-ssa-math-opts.c (execute_cse_reciprocals): Propagate
	reciprocal-of-reciprocal DEFs into debug stmts.

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c.orig	2009-11-19 00:32:12.000000000 -0200
+++ gcc/tree-ssa-math-opts.c	2009-11-19 00:36:56.000000000 -0200
@@ -573,9 +573,19 @@ execute_cse_reciprocals (void)
 
 		  gsi1 = gsi_for_stmt (stmt1);
 		  gsi_insert_before (&gsi1, rstmt1, true);
-		  /* This will reset debug stmts that refer to arg1,
-		     so we don't have to check for them in the loop
-		     below.  */
+
+		  /* This gets debug stmts that refer to arg1 updated
+		     in terms of rarg1.  */
+		  if (MAY_HAVE_DEBUG_STMTS)
+		    {
+		      tree one = build_one_cst (TREE_TYPE (arg1));
+
+		      stmt1 = gimple_build_assign_with_ops (RDIV_EXPR,
+							    arg1,
+							    one,
+							    rarg1);
+		      gsi_replace (&gsi1, stmt1, true);
+		    }
 		  gsi_remove (&gsi1, true);
 
 		  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

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