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]

RFA: Fix PR41963 (miscompilation of 177.mesa)


Hi,

my recent change to activate rsqrt already under -ffast-math exposed a bug 
in the actual transformation pass.  It replaces this sequence:

  len = sqrtf (x);
  x /= len;
  someotheruseof (len);

with
  len = rsqrtf (x);
  x *= len;
  someotheruseof (len);

I.e. changed the value of len despite there being other non-transformable 
uses of it.  The fix is trivial, we need to check if all uses of the to-be 
reciprocal are actually divisions we can transform (checking for a 
single-use would also be okay, but then we'd miss the transformation in 
the "x/=len,y/=len,z/=len" case).

I checked that 177.mesa doesn't miscompare anymore, otherwise regstrapping 
on x86_64-linux in progress.  Okay for trunk?


Ciao,
Michael.
-- 
	PR middle-end/41963
	* tree-ssa-math-opts.c (execute_cse_reciprocals): Check all uses
	of a potential reciprocal to really be reciprocals.

testsuite/
	* gcc.dg/pr41963.c: New test.

Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c	(revision 153935)
+++ tree-ssa-math-opts.c	(working copy)
@@ -531,7 +531,9 @@ execute_cse_reciprocals (void)
 		      || DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD))
 		{
 		  enum built_in_function code;
-		  bool md_code;
+		  bool md_code, fail;
+		  imm_use_iterator ui;
+		  use_operand_p use_p;
 
 		  code = DECL_FUNCTION_CODE (fndecl);
 		  md_code = DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD;
@@ -540,12 +542,36 @@ execute_cse_reciprocals (void)
 		  if (!fndecl)
 		    continue;
 
+		  /* Check that all uses of the SSA name are divisions,
+		     otherwise replacing the defining statement will do
+		     the wrong thing.  */
+		  fail = false;
+		  FOR_EACH_IMM_USE_FAST (use_p, ui, arg1)
+		    {
+		      gimple stmt2 = USE_STMT (use_p);
+		      if (is_gimple_debug (stmt2))
+			continue;
+		      if (!is_gimple_assign (stmt2)
+			  || gimple_assign_rhs_code (stmt2) != RDIV_EXPR
+			  || gimple_assign_rhs1 (stmt2) == arg1
+			  || gimple_assign_rhs2 (stmt2) != arg1)
+			{
+			  fail = true;
+			  break;
+			}
+		    }
+		  if (fail)
+		    continue;
+
 		  gimple_call_set_fndecl (stmt1, fndecl);
 		  update_stmt (stmt1);
 
-		  gimple_assign_set_rhs_code (stmt, MULT_EXPR);
-		  fold_stmt_inplace (stmt);
-		  update_stmt (stmt);
+		  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
+		    {
+		      gimple_assign_set_rhs_code (stmt, MULT_EXPR);
+		      fold_stmt_inplace (stmt);
+		      update_stmt (stmt);
+		    }
 		}
 	    }
 	}
Index: testsuite/gcc.dg/pr41963.c
===================================================================
--- testsuite/gcc.dg/pr41963.c	(revision 0)
+++ testsuite/gcc.dg/pr41963.c	(revision 0)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -ffast-math" } */
+#include <math.h>
+
+static __attribute__((noinline)) void f (float *dst, float *src)
+{
+  int i, j;
+  for (i = 0; i < 2; i++)
+    {
+      float len;
+      dst[0] = src[0];
+      dst[1] = src[1];
+      len = sqrtf (dst[0] * dst[0] + dst[1] * dst[1]);
+      if (len > 0.5f)
+	{
+	  len = 1.0f / len;
+	  dst[0] *= len;
+	  dst[1] *= len;
+	}
+    }
+}
+
+extern void abort (void);
+
+int main()
+{
+  float dst[2], src[2];
+  src[0] = 2.0f;
+  src[1] = 5.0f;
+  f (dst, src);
+  if (fabsf (dst[0] * dst[0] + dst[1] * dst[1] - 1.0f) > 0.01f)
+    abort ();
+  return 0;
+}


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