* RFA: Fix PR41963 (miscompilation of 177.mesa)
@ 2009-11-06 14:10 Michael Matz
2009-11-06 15:00 ` Richard Guenther
0 siblings, 1 reply; 2+ messages in thread
From: Michael Matz @ 2009-11-06 14:10 UTC (permalink / raw)
To: gcc-patches
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;
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: RFA: Fix PR41963 (miscompilation of 177.mesa)
2009-11-06 14:10 RFA: Fix PR41963 (miscompilation of 177.mesa) Michael Matz
@ 2009-11-06 15:00 ` Richard Guenther
0 siblings, 0 replies; 2+ messages in thread
From: Richard Guenther @ 2009-11-06 15:00 UTC (permalink / raw)
To: Michael Matz; +Cc: gcc-patches
On Fri, Nov 6, 2009 at 2:54 PM, Michael Matz <matz@suse.de> wrote:
> 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?
Ok.
Thanks,
Richard.
>
> 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;
> +}
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-11-06 14:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-06 14:10 RFA: Fix PR41963 (miscompilation of 177.mesa) Michael Matz
2009-11-06 15:00 ` Richard Guenther
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).