On Mon, Jun 23, 2014 at 03:35:01PM +0200, Bernd Edlinger wrote:
> Hi Martin,
>
> >>
> >> Well actually, I am not sure if we ever wanted to have a race condition
> >> here.
> >> Have you seen any impact of --param allow-store-data-races on any
> >> benchmark?
> >
> > It's trivially to write one. The only pass that checks the param is
> > tree loop invariant motion and it does that when it applies store-motion.
> > Register pressure increase is increased by a factor of two.
> >
> > So I'd agree that we might want to disable this again for -Ofast.
> >
> > As nothing tests for the PACKED variants nor for the LOAD variant
> > I'd rather remove those. Claiming we don't create races for those
> > when you disable it via the param is simply not true.
> >
> > Thanks,
> > Richard.
> >
>
> OK, please go ahead with your patch.
Perhaps not unsurprisingly, the patch is very similar. Bootstrapped
and tested on x86_64-linux. OK for trunk?
Thanks,
Martin
2014-06-24 Martin Jambor <mjam...@suse.cz>
* params.def (PARAM_ALLOW_LOAD_DATA_RACES)
(PARAM_ALLOW_PACKED_LOAD_DATA_RACES)
(PARAM_ALLOW_PACKED_STORE_DATA_RACES): Removed.
(PARAM_ALLOW_STORE_DATA_RACES): Set default to zero.
* opts.c (default_options_optimization): Set
PARAM_ALLOW_STORE_DATA_RACES to one at -Ofast.
* doc/invoke.texi (allow-load-data-races)
(allow-packed-load-data-races, allow-packed-store-data-races):
Removed.
(allow-store-data-races): Document the new default.
testsuite/
* g++.dg/simulate-thread/bitfields-2.C: Remove allow-load-data-races
parameter.
* g++.dg/simulate-thread/bitfields.C: Likewise.
* gcc.dg/simulate-thread/strict-align-global.c: Remove
allow-packed-store-data-races parameter.
* gcc.dg/simulate-thread/subfields.c: Likewise.
* gcc.dg/tree-ssa/20050314-1.c: Set parameter allow-store-data-races
to one.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 0d4bd00..027b6fb 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10176,25 +10176,10 @@ The maximum number of conditional stores paires that
can be sunk. Set to 0
if either vectorization (@option{-ftree-vectorize}) or if-conversion
(@option{-ftree-loop-if-convert}) is disabled. The default is 2.
-@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.
+at optimization level @option{-Ofast}.
@item case-values-threshold
The smallest number of different values for which it is best to use a
diff --git a/gcc/opts.c b/gcc/opts.c
index 3ab06c6..19203dc 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -620,6 +620,13 @@ default_options_optimization (struct gcc_options *opts,
opt2 ? default_param_value (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP) : 1000,
opts->x_param_values, opts_set->x_param_values);
+ /* At -Ofast, allow store motion to introduce potential race conditions. */
+ maybe_set_param_value
+ (PARAM_ALLOW_STORE_DATA_RACES,
+ opts->x_optimize_fast ? 1
+ : default_param_value (PARAM_ALLOW_STORE_DATA_RACES),
+ opts->x_param_values, opts_set->x_param_values);
+
if (opts->x_optimize_size)
/* We want to crossjump as much as possible. */
maybe_set_param_value (PARAM_MIN_CROSSJUMP_INSNS, 1,
diff --git a/gcc/params.def b/gcc/params.def
index 28ef79a..aa1e88d 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1002,25 +1002,10 @@ DEFPARAM (PARAM_CASE_VALUES_THRESHOLD,
0, 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)
+ 0, 0, 1)
/* Reassociation width to be used by tree reassoc optimization. */
DEFPARAM (PARAM_TREE_REASSOC_WIDTH,
diff --git a/gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C
b/gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C
index 077514a..be5d1ea 100644
--- a/gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C
+++ b/gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C
@@ -1,5 +1,5 @@
/* { dg-do link } */
-/* { dg-options "--param allow-load-data-races=0 --param
allow-store-data-races=0" } */
+/* { dg-options "--param allow-store-data-races=0" } */
/* { dg-final { simulate-thread } } */
/* Test that setting <var.a> does not touch either <var.b> or <var.c>.
diff --git a/gcc/testsuite/g++.dg/simulate-thread/bitfields.C
b/gcc/testsuite/g++.dg/simulate-thread/bitfields.C
index 3acf21f..b829587 100644
--- a/gcc/testsuite/g++.dg/simulate-thread/bitfields.C
+++ b/gcc/testsuite/g++.dg/simulate-thread/bitfields.C
@@ -1,5 +1,5 @@
/* { dg-do link } */
-/* { dg-options "--param allow-load-data-races=0 --param
allow-store-data-races=0" } */
+/* { dg-options "--param allow-store-data-races=0" } */
/* { dg-final { simulate-thread } } */
/* Test that setting <var.a> does not touch either <var.b> or <var.c>.
diff --git a/gcc/testsuite/gcc.dg/simulate-thread/strict-align-global.c
b/gcc/testsuite/gcc.dg/simulate-thread/strict-align-global.c
index fdcd7f4..f8b88ad 100644
--- a/gcc/testsuite/gcc.dg/simulate-thread/strict-align-global.c
+++ b/gcc/testsuite/gcc.dg/simulate-thread/strict-align-global.c
@@ -1,5 +1,4 @@
/* { dg-do link } */
-/* { dg-options "--param allow-packed-store-data-races=0" } */
/* { dg-final { simulate-thread } } */
#include <stdio.h>
diff --git a/gcc/testsuite/gcc.dg/simulate-thread/subfields.c
b/gcc/testsuite/gcc.dg/simulate-thread/subfields.c
index 2d93117..70e38a1 100644
--- a/gcc/testsuite/gcc.dg/simulate-thread/subfields.c
+++ b/gcc/testsuite/gcc.dg/simulate-thread/subfields.c
@@ -1,5 +1,4 @@
/* { dg-do link } */
-/* { dg-options "--param allow-packed-store-data-races=0" } */
/* { dg-final { simulate-thread } } */
#include <stdio.h>
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c
b/gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c
index 1082973..8f07781 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-lim1-details" } */
+/* { dg-options "-O1 -fdump-tree-lim1-details --param
allow-store-data-races=1" } */
float a[100];
