Patchwork rewrite another failing data race test (gcc.dg/pr52558-2.c)

login
register
mail settings
Submitter Aldy Hernandez
Date Sept. 7, 2012, 6:05 p.m.
Message ID <504A3772.40307@redhat.com>
Download mbox | patch
Permalink /patch/182431/
State New
Headers show

Comments

Aldy Hernandez - Sept. 7, 2012, 6:05 p.m.
This is the same thing as gcc.dg/pr52558-1.c, but in this case I had to 
tweak the testcase a bit because optimization passes after LIM are smart 
enough to remove the condition altogether, thus never triggering the 
test.  Interestingly, GCC can figure out what's going on when the 
condition is "l < 1234", but not when it is "l != 4".

Luckily, the original PR (PR52558) was testing "l != 4", so now the test 
looks exactly as the what the PR writer had.

Tested on x86-64 Linux by running with and without --param 
allow-store-data-races=0, and by visual inspection of the assembly.

OK?
testsuite/
	* gcc.dg/pr52558-2.c: Delete.
	* gcc.dg/simulate-thread/speculative-store-3.c: New.
Richard Guenther - Sept. 10, 2012, 9:36 a.m.
On Fri, Sep 7, 2012 at 8:05 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> This is the same thing as gcc.dg/pr52558-1.c, but in this case I had to
> tweak the testcase a bit because optimization passes after LIM are smart
> enough to remove the condition altogether, thus never triggering the test.
> Interestingly, GCC can figure out what's going on when the condition is "l <
> 1234", but not when it is "l != 4".
>
> Luckily, the original PR (PR52558) was testing "l != 4", so now the test
> looks exactly as the what the PR writer had.
>
> Tested on x86-64 Linux by running with and without --param
> allow-store-data-races=0, and by visual inspection of the assembly.
>
> OK?

Ok.

Thanks,
Richard.

Patch

diff --git a/gcc/testsuite/gcc.dg/pr52558-2.c b/gcc/testsuite/gcc.dg/pr52558-2.c
deleted file mode 100644
index 6d5f51c..0000000
--- a/gcc/testsuite/gcc.dg/pr52558-2.c
+++ /dev/null
@@ -1,23 +0,0 @@ 
-/* { dg-do compile } */
-/* { dg-options "--param allow-store-data-races=0 -O2 -fdump-tree-lim1" } */
-
-/* Test that g_2 is not written to unless !g_1.  */
-
-int g_1 = 1;
-int g_2 = 0;
-
-int func_1(void)
-{
- int l;
- for (l = 0; l < 1234; l++)
- {
-   if (g_1)
-     return l;
-   else
-     g_2 = 0;
- }
- return 999;
-}
-
-/* { dg-final { scan-tree-dump-times "MEM.*g_2_lsm_flag" 1 "lim1" } } */
-/* { dg-final { cleanup-tree-dump "lim1" } } */
diff --git a/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c b/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c
new file mode 100644
index 0000000..203c026
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c
@@ -0,0 +1,71 @@ 
+/* { dg-do link } */
+/* { dg-options "--param allow-store-data-races=0 -O2" } */
+/* { dg-final { simulate-thread } } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "simulate-thread.h"
+
+/* Test distilled from PR52558.  */
+
+int g_1 = 1;
+int g_2 = 0, insns = 0;
+int f;
+
+/* Test that g_2 is not written to unless !g_1.  */
+
+__attribute__((noinline))
+int funky()
+{
+  int l;
+  for (l = 0; l != 4; l++)
+    {
+      if (g_1)
+	{
+	  /* g_1 is globally true so we should always execute here,
+	     thus never writing to g_2 under any circumstance in this
+	     code path.  */
+	  return l;
+	}
+      for (g_2 = 0; g_2 >= 26; ++g_2)
+	;
+    }
+  return 999;
+}
+
+int simulate_thread_final_verify ()
+{
+  /* If g_2 != insns, someone must have cached `g_2' and stored a
+     racy value into it.  */
+  if (g_2 != insns)
+    {
+      printf("FAIL: g_2 was incorrectly cached\n");
+      return 1;
+    }
+  return 0;
+}
+
+void simulate_thread_other_threads ()
+{
+  ++insns;
+  ++g_2;
+}
+
+int simulate_thread_step_verify ()
+{
+  return 0;
+}
+
+__attribute__((noinline))
+void simulate_thread_main()
+{
+  f = funky();
+}
+
+int main()
+{
+  simulate_thread_main ();
+  simulate_thread_done ();
+  return 0;
+}