Patchwork Fix up stdarg pass (PR tree-optimization/56205)

login
register
mail settings
Submitter Jakub Jelinek
Date Feb. 5, 2013, 3:34 p.m.
Message ID <20130205153444.GL4385@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/218280/
State New
Headers show

Comments

Jakub Jelinek - Feb. 5, 2013, 3:34 p.m.
Hi!

The following testcase is miscompiled on ppc64, because the stdarg pass
didn't figure out that va_list was escaping.
The problem was that we've processed the
# z.2_31 = PHI <z.2_27(11), z.2_24(12)>
stmt was processed before processing z.2_27 and z.2_24 assignments,
so neither of them was in va_list_escape_vars at that point and so
z.2_31 wasn't added.  As we don't iterate in the first pass, we need
to just give up if we see this during check_all_va_list_escapes.
I don't think it happens often enough to warrant the need to iterate
instead.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2013-02-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56205
	* tree-stdarg.c (check_all_va_list_escapes): Return true if
	there are any PHI nodes that set non-va_list_escape_vars SSA_NAME
	and some va_list_escape_vars SSA_NAME appears in some PHI argument.

	* gcc.dg/tree-ssa/stdarg-6.c: New test.
	* gcc.c-torture/execute/pr56205.c: New test.


	Jakub
Richard Guenther - Feb. 5, 2013, 3:41 p.m.
On Tue, 5 Feb 2013, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled on ppc64, because the stdarg pass
> didn't figure out that va_list was escaping.
> The problem was that we've processed the
> # z.2_31 = PHI <z.2_27(11), z.2_24(12)>
> stmt was processed before processing z.2_27 and z.2_24 assignments,
> so neither of them was in va_list_escape_vars at that point and so
> z.2_31 wasn't added.  As we don't iterate in the first pass, we need
> to just give up if we see this during check_all_va_list_escapes.
> I don't think it happens often enough to warrant the need to iterate
> instead.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2013-02-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/56205
> 	* tree-stdarg.c (check_all_va_list_escapes): Return true if
> 	there are any PHI nodes that set non-va_list_escape_vars SSA_NAME
> 	and some va_list_escape_vars SSA_NAME appears in some PHI argument.
> 
> 	* gcc.dg/tree-ssa/stdarg-6.c: New test.
> 	* gcc.c-torture/execute/pr56205.c: New test.
> 
> --- gcc/tree-stdarg.c.jj	2013-01-11 09:02:48.000000000 +0100
> +++ gcc/tree-stdarg.c	2013-02-05 10:47:13.292700397 +0100
> @@ -526,6 +526,37 @@ check_all_va_list_escapes (struct stdarg
>      {
>        gimple_stmt_iterator i;
>  
> +      for (i = gsi_start_phis (bb); !gsi_end_p (i); gsi_next (&i))
> +	{
> +	  tree lhs;
> +	  use_operand_p uop;
> +	  ssa_op_iter soi;
> +	  gimple phi = gsi_stmt (i);
> +
> +	  lhs = PHI_RESULT (phi);
> +	  if (virtual_operand_p (lhs)
> +	      || bitmap_bit_p (si->va_list_escape_vars,
> +			       SSA_NAME_VERSION (lhs)))
> +	    continue;
> +
> +	  FOR_EACH_PHI_ARG (uop, phi, soi, SSA_OP_USE)
> +	    {
> +	      tree rhs = USE_FROM_PTR (uop);
> +	      if (TREE_CODE (rhs) == SSA_NAME
> +		  && bitmap_bit_p (si->va_list_escape_vars,
> +				SSA_NAME_VERSION (rhs)))
> +		{
> +		  if (dump_file && (dump_flags & TDF_DETAILS))
> +		    {
> +		      fputs ("va_list escapes in ", dump_file);
> +		      print_gimple_stmt (dump_file, phi, 0, dump_flags);
> +		      fputc ('\n', dump_file);
> +		    }
> +		  return true;
> +		}
> +	    }
> +	}
> +
>        for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
>  	{
>  	  gimple stmt = gsi_stmt (i);
> --- gcc/testsuite/gcc.dg/tree-ssa/stdarg-6.c.jj	2013-02-05 11:01:37.518935765 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/stdarg-6.c	2013-02-05 11:15:01.511610919 +0100
> @@ -0,0 +1,35 @@
> +/* PR tree-optimization/56205 */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fdump-tree-stdarg" } */
> +
> +#include <stdarg.h>
> +
> +int a, b;
> +char c[128];
> +
> +static inline void
> +foo (int x, char const *y, va_list z)
> +{
> +  __builtin_printf ("%s %d %s", x ? "" : "foo", ++a, (y && *y) ? "bar" : "");
> +  if (y && *y)
> +    __builtin_vprintf (y, z);
> +}
> +
> +void
> +bar (int x, char const *y, ...)
> +{
> +  va_list z;
> +  va_start (z, y);
> +  if (!x && *c == '\0')
> +    ++b;
> +  foo (x, y, z);
> +  va_end (z);
> +}
> +
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { powerpc*-*-linux* && ilp32 } } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target alpha*-*-linux* } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target s390*-*-linux* } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && ia32 } } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target ia64-*-* } } } */
> +/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target { powerpc*-*-* && lp64 } } } } */
> --- gcc/testsuite/gcc.c-torture/execute/pr56205.c.jj	2013-02-05 11:36:30.693183702 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr56205.c	2013-02-05 11:34:34.000000000 +0100
> @@ -0,0 +1,61 @@
> +/* PR tree-optimization/56205 */
> +
> +#include <stdarg.h>
> +
> +int a, b;
> +char c[128];
> +
> +__attribute__((noinline, noclone)) static void
> +f1 (const char *fmt, ...)
> +{
> +  va_list ap;
> +  asm volatile ("" : : : "memory");
> +  if (__builtin_strcmp (fmt, "%s %d %s") != 0)
> +    __builtin_abort ();
> +  va_start (ap, fmt);
> +  if (__builtin_strcmp (va_arg (ap, const char *), "foo") != 0
> +      || va_arg (ap, int) != 1
> +      || __builtin_strcmp (va_arg (ap, const char *), "bar") != 0)
> +    __builtin_abort ();
> +  va_end (ap);
> +}
> +
> +__attribute__((noinline, noclone)) static void
> +f2 (const char *fmt, va_list ap)
> +{
> +  asm volatile ("" : : : "memory");
> +  if (__builtin_strcmp (fmt, "baz") != 0
> +      || __builtin_strcmp (va_arg (ap, const char *), "foo") != 0
> +      || va_arg (ap, double) != 12.0
> +      || va_arg (ap, int) != 26)
> +    __builtin_abort ();
> +}
> +
> +static void
> +f3 (int x, char const *y, va_list z)
> +{
> +  f1 ("%s %d %s", x ? "" : "foo", ++a, (y && *y) ? "bar" : "");
> +  if (y && *y)
> +    f2 (y, z);
> +}
> +
> +__attribute__((noinline, noclone)) void
> +f4 (int x, char const *y, ...)
> +{
> +  va_list z;
> +  va_start (z, y);
> +  if (!x && *c == '\0')
> +    ++b;
> +  f3 (x, y, z);
> +  va_end (z);
> +}
> +
> +int
> +main ()
> +{
> +  asm volatile ("" : : : "memory");
> +  f4 (0, "baz", "foo", 12.0, 26);
> +  if (a != 1 || b != 1)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>

Patch

--- gcc/tree-stdarg.c.jj	2013-01-11 09:02:48.000000000 +0100
+++ gcc/tree-stdarg.c	2013-02-05 10:47:13.292700397 +0100
@@ -526,6 +526,37 @@  check_all_va_list_escapes (struct stdarg
     {
       gimple_stmt_iterator i;
 
+      for (i = gsi_start_phis (bb); !gsi_end_p (i); gsi_next (&i))
+	{
+	  tree lhs;
+	  use_operand_p uop;
+	  ssa_op_iter soi;
+	  gimple phi = gsi_stmt (i);
+
+	  lhs = PHI_RESULT (phi);
+	  if (virtual_operand_p (lhs)
+	      || bitmap_bit_p (si->va_list_escape_vars,
+			       SSA_NAME_VERSION (lhs)))
+	    continue;
+
+	  FOR_EACH_PHI_ARG (uop, phi, soi, SSA_OP_USE)
+	    {
+	      tree rhs = USE_FROM_PTR (uop);
+	      if (TREE_CODE (rhs) == SSA_NAME
+		  && bitmap_bit_p (si->va_list_escape_vars,
+				SSA_NAME_VERSION (rhs)))
+		{
+		  if (dump_file && (dump_flags & TDF_DETAILS))
+		    {
+		      fputs ("va_list escapes in ", dump_file);
+		      print_gimple_stmt (dump_file, phi, 0, dump_flags);
+		      fputc ('\n', dump_file);
+		    }
+		  return true;
+		}
+	    }
+	}
+
       for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
 	{
 	  gimple stmt = gsi_stmt (i);
--- gcc/testsuite/gcc.dg/tree-ssa/stdarg-6.c.jj	2013-02-05 11:01:37.518935765 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/stdarg-6.c	2013-02-05 11:15:01.511610919 +0100
@@ -0,0 +1,35 @@ 
+/* PR tree-optimization/56205 */
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-tree-stdarg" } */
+
+#include <stdarg.h>
+
+int a, b;
+char c[128];
+
+static inline void
+foo (int x, char const *y, va_list z)
+{
+  __builtin_printf ("%s %d %s", x ? "" : "foo", ++a, (y && *y) ? "bar" : "");
+  if (y && *y)
+    __builtin_vprintf (y, z);
+}
+
+void
+bar (int x, char const *y, ...)
+{
+  va_list z;
+  va_start (z, y);
+  if (!x && *c == '\0')
+    ++b;
+  foo (x, y, z);
+  va_end (z);
+}
+
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { powerpc*-*-linux* && ilp32 } } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target alpha*-*-linux* } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target s390*-*-linux* } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && ia32 } } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target ia64-*-* } } } */
+/* { dg-final { scan-tree-dump "bar: va_list escapes 1, needs to save all GPR units" "stdarg" { target { powerpc*-*-* && lp64 } } } } */
--- gcc/testsuite/gcc.c-torture/execute/pr56205.c.jj	2013-02-05 11:36:30.693183702 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr56205.c	2013-02-05 11:34:34.000000000 +0100
@@ -0,0 +1,61 @@ 
+/* PR tree-optimization/56205 */
+
+#include <stdarg.h>
+
+int a, b;
+char c[128];
+
+__attribute__((noinline, noclone)) static void
+f1 (const char *fmt, ...)
+{
+  va_list ap;
+  asm volatile ("" : : : "memory");
+  if (__builtin_strcmp (fmt, "%s %d %s") != 0)
+    __builtin_abort ();
+  va_start (ap, fmt);
+  if (__builtin_strcmp (va_arg (ap, const char *), "foo") != 0
+      || va_arg (ap, int) != 1
+      || __builtin_strcmp (va_arg (ap, const char *), "bar") != 0)
+    __builtin_abort ();
+  va_end (ap);
+}
+
+__attribute__((noinline, noclone)) static void
+f2 (const char *fmt, va_list ap)
+{
+  asm volatile ("" : : : "memory");
+  if (__builtin_strcmp (fmt, "baz") != 0
+      || __builtin_strcmp (va_arg (ap, const char *), "foo") != 0
+      || va_arg (ap, double) != 12.0
+      || va_arg (ap, int) != 26)
+    __builtin_abort ();
+}
+
+static void
+f3 (int x, char const *y, va_list z)
+{
+  f1 ("%s %d %s", x ? "" : "foo", ++a, (y && *y) ? "bar" : "");
+  if (y && *y)
+    f2 (y, z);
+}
+
+__attribute__((noinline, noclone)) void
+f4 (int x, char const *y, ...)
+{
+  va_list z;
+  va_start (z, y);
+  if (!x && *c == '\0')
+    ++b;
+  f3 (x, y, z);
+  va_end (z);
+}
+
+int
+main ()
+{
+  asm volatile ("" : : : "memory");
+  f4 (0, "baz", "foo", 12.0, 26);
+  if (a != 1 || b != 1)
+    __builtin_abort ();
+  return 0;
+}