Patchwork [cxx-mem-model] options for controlling data races

login
register
mail settings
Submitter Aldy Hernandez
Date March 14, 2011, 1:20 p.m.
Message ID <4D7E1611.801@redhat.com>
Download mbox | patch
Permalink /patch/86735/
State New
Headers show

Comments

Aldy Hernandez - March 14, 2011, 1:20 p.m.
The following patch adds the --param(eters) needed to tweak the 
optimizers into allowing or disallowing data races in cross-thread 
visible data.

There will be additional flags added later to make these parameters 
available to the common user.  It has been suggested to use 
-fmemory-model=xxx, but I will implement this later.

The next step will be to find out where we introduce the data races in 
the testsuite, and fix the optimizers accordingly.

Committed to branch.
* params.h (ALLOW_LOAD_DATA_RACES): New.
	(ALLOW_STORE_DATA_RACES): New.
	(ALLOW_PACKED_LOAD_DATA_RACES): New.
	(ALLOW_PACKED_STORE_DATA_RACES): New.
	* params.def (PARAM_ALLOW_LOAD_DATA_RACES): New.
	(PARAM_ALLOW_STORE_DATA_RACES): New.
	(PARAM_ALLOW_PACKED_LOAD_DATA_RACES): New.
	(PARAM_ALLOW_PACKED_STORE_DATA_RACES): New.
	* doc/invoke.texi (Optimize Options): Document above parameters.

Patch

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 170745)
+++ doc/invoke.texi	(working copy)
@@ -8897,6 +8897,26 @@  partitions.
 The maximum number of namespaces to consult for suggestions when C++
 name lookup fails for an identifier.  The default is 1000.
 
+@item allow-load-data-races
+Allow optimizers to introduce new data races on loads.
+Set to 1 to allow, otherwise to 0.  This option is enabled by default
+unless implicitly set by the @option{-fmemory-model=} option.
+
+@item allow-store-data-races
+Allow optimizers to introduce new data races on stores.
+Set to 1 to allow, otherwise to 0.  This option is enabled by default
+unless implicitly set by the @option{-fmemory-model=} option.
+
+@item allow-packed-load-data-races
+Allow optimizers to introduce new data races on packed data loads.
+Set to 1 to allow, otherwise to 0.  This option is enabled by default
+unless implicitly set by the @option{-fmemory-model=} option.
+
+@item allow-packed-store-data-races
+Allow optimizers to introduce new data races on packed data stores.
+Set to 1 to allow, otherwise to 0.  This option is enabled by default
+unless implicitly set by the @option{-fmemory-model=} option.
+
 @end table
 @end table
 
Index: params.h
===================================================================
--- params.h	(revision 170745)
+++ params.h	(working copy)
@@ -206,4 +206,13 @@  extern void init_param_values (int *para
   PARAM_VALUE (PARAM_PREFETCH_MIN_INSN_TO_MEM_RATIO)
 #define MIN_NONDEBUG_INSN_UID \
   PARAM_VALUE (PARAM_MIN_NONDEBUG_INSN_UID)
+#define ALLOW_LOAD_DATA_RACES \
+  PARAM_VALUE (PARAM_ALLOW_LOAD_DATA_RACES)
+#define ALLOW_STORE_DATA_RACES \
+  PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES)
+#define ALLOW_PACKED_LOAD_DATA_RACES \
+  PARAM_VALUE (PARAM_ALLOW_PACKED_LOAD_DATA_RACES)
+#define ALLOW_PACKED_STORE_DATA_RACES \
+  PARAM_VALUE (PARAM_ALLOW_PACKED_STORE_DATA_RACES)
+
 #endif /* ! GCC_PARAMS_H */
Index: testsuite/gcc.dg/memmodel/speculative-store.c
===================================================================
--- testsuite/gcc.dg/memmodel/speculative-store.c	(revision 170852)
+++ testsuite/gcc.dg/memmodel/speculative-store.c	(working copy)
@@ -1,14 +1,12 @@ 
 /* { dg-do link } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 --param allow-store-data-races=0" } */
 /* { dg-final { memmodel-gdb-test } } */
 
-/* FIXME: I can't get this to fail.  */
-
 #include <stdio.h>
 #include "memmodel.h"
 
 /* This file tests that speculative store movement out of a loop doesn't 
-   happen.  This is disallowed when -fno-allow-store-data-races is on.  */
+   happen.  This is disallowed when --param allow-store-data-races is 0.  */
 
 int global = 100;
 
Index: testsuite/gcc.dg/memmodel/subfields.c
===================================================================
--- testsuite/gcc.dg/memmodel/subfields.c	(revision 170852)
+++ testsuite/gcc.dg/memmodel/subfields.c	(working copy)
@@ -1,9 +1,7 @@ 
 /* { dg-do link } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 --param allow-packed-store-data-races=0" } */
 /* { dg-final { memmodel-gdb-test } } */
 
-/* FIXME: I can't get this to fail.  */
-
 #include <stdio.h>
 #include "memmodel.h"
 
Index: testsuite/gcc.dg/memmodel/global-hoist.c
===================================================================
--- testsuite/gcc.dg/memmodel/global-hoist.c	(revision 170852)
+++ testsuite/gcc.dg/memmodel/global-hoist.c	(working copy)
@@ -1,10 +1,10 @@ 
 /* { dg-do link } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 --param allow-load-data-races=0" } */
 /* { dg-final { memmodel-gdb-test } } */
 
 /* Verify that a load of a global is not hoisted out of a loop.  This
    can introduce a data race, and is dissallowed if
-   -fno-allow-load-data-races is enabled. */
+   --param allow-load-data-races is 0. */
 
 #include <stdio.h>
 #include <stdlib.h>
Index: testsuite/gcc.dg/memmodel/d2.c
===================================================================
--- testsuite/gcc.dg/memmodel/d2.c	(revision 170852)
+++ testsuite/gcc.dg/memmodel/d2.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do link } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 --param allow-load-data-races=0" } */
 /* { dg-final { memmodel-gdb-test } } */
 
 #include <stdio.h>
@@ -8,7 +8,7 @@ 
 /* This is a variation on global-hoist.c where instead of a loop, we
    store global into different elements of an array in straightline
    code with an if condition.  This will catch cases where commoning
-   should be disabled when -fno-allow-load-data-races is on.  */
+   should be disabled when --param allow-load-data-races is 0.  */
 
 /* Test the FALSE path in test.  */
 
Index: testsuite/gcc.dg/memmodel/d3.c
===================================================================
--- testsuite/gcc.dg/memmodel/d3.c	(revision 170852)
+++ testsuite/gcc.dg/memmodel/d3.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* { dg-do link } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 --param allow-load-data-races=0" } */
 /* { dg-final { memmodel-gdb-test } } */
 
 #include <stdio.h>
@@ -8,7 +8,7 @@ 
 /* A variation on global-hoist.c where instead of a loop, we store
    global into different elements of an array in straightline code
    with an if condition.  This will catch cases where commoning should
-   be disabled when -fno-allow-load-data-races is on.  */
+   be disabled when --param allow-load-data-races is 0.  */
 
 /* Test the TRUE path in test.  */
 
Index: params.def
===================================================================
--- params.def	(revision 170745)
+++ params.def	(working copy)
@@ -877,6 +877,28 @@  DEFPARAM (CXX_MAX_NAMESPACES_FOR_DIAGNOS
 	  "name lookup fails",
 	  1000, 0, 0)
 
+ /* Data race flags for C++0x memory model compliance.  */
+ 
+ DEFPARAM (PARAM_ALLOW_LOAD_DATA_RACES,
+	   "allow-load-data-races",
+	   "Allow new data races on loads to be introduced",
+	   1, 0, 1)
+ 
+ DEFPARAM (PARAM_ALLOW_STORE_DATA_RACES,
+	   "allow-store-data-races",
+	   "Allow new data races on stores to be introduced",
+	   1, 0, 1)
+ 
+ DEFPARAM (PARAM_ALLOW_PACKED_LOAD_DATA_RACES,
+	   "allow-packed-load-data-races",
+	   "Allow new data races on packed data loads to be introduced",
+	   1, 0, 1)
+ 
+ DEFPARAM (PARAM_ALLOW_PACKED_STORE_DATA_RACES,
+	   "allow-packed-store-data-races",
+	   "Allow new data races on packed data stores to be introduced",
+	   1, 0, 1)
+
 /*
 Local variables:
 mode:c