On Tue, 15 Oct 2019, Kyrill Tkachov wrote:
> Hi Richard,
>
> On 10/15/19 8:17 AM, Richard Biener wrote:
> >
> > This makes allow-store-data-races adjustable per function by making it
> > a regular option rather than a --param.
>
>
> Note that the kernel has --param=allow-store-data-races=0 in its build flags.
>
> I guess that will break unless they rename it to
> -fno-allow-store-data-races?
Yes. Or simply drop the --param since unless they happen to use -Ofast
their setting is the default.
Richard.
> Thanks,
>
> Kyrill
>
>
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> >
> > Thanks,
> > Richard.
> >
> > 2019-10-15 Richard Biener <rguent...@suse.de>
> >
> > PR middle-end/92046
> > * common.opt (fallow-store-data-races): New.
> > * params.def (PARAM_ALLOW_STORE_DATA_RACES): Remove.
> > * params.h (ALLOW_STORE_DATA_RACES): Likewise.
> > * doc/invoke.texi (fallow-store-data-races): Document.
> > (--param allow-store-data-races): Remove docs.
> > * opts.c (default_options_table): Enable -fallow-store-data-races
> > at -Ofast.
> > (default_options_optimization): Do not enable --param
> > allow-store-data-races at -Ofast.
> > * tree-if-conv.c (ifcvt_memrefs_wont_trap): Use
> > flag_store_data_races
> > instead of PARAM_ALLOW_STORE_DATA_RACES.
> > * tree-ssa-loop-im.c (execute_sm): Likewise.
> >
> > * c-c++-common/cxxbitfields-3.c: Adjust.
> > * c-c++-common/cxxbitfields-6.c: Likewise.
> > * c-c++-common/simulate-thread/bitfields-1.c: Likewise.
> > * c-c++-common/simulate-thread/bitfields-2.c: Likewise.
> > * c-c++-common/simulate-thread/bitfields-3.c: Likewise.
> > * c-c++-common/simulate-thread/bitfields-4.c: Likewise.
> > * g++.dg/simulate-thread/bitfields-2.C: Likewise.
> > * g++.dg/simulate-thread/bitfields.C: Likewise.
> > * gcc.dg/lto/pr52097_0.c: Likewise.
> > * gcc.dg/simulate-thread/speculative-store-2.c: Likewise.
> > * gcc.dg/simulate-thread/speculative-store-3.c: Likewise.
> > * gcc.dg/simulate-thread/speculative-store-4.c: Likewise.
> > * gcc.dg/simulate-thread/speculative-store.c: Likewise.
> > * gcc.dg/tree-ssa/20050314-1.c: Likewise.
> >
> > Index: gcc/common.opt
> > ===================================================================
> > --- gcc/common.opt (revision 276983)
> > +++ gcc/common.opt (working copy)
> > @@ -993,6 +993,10 @@ Align the start of loops.
> > falign-loops=
> > Common RejectNegative Joined Var(str_align_loops) Optimization
> >
> > +fallow-store-data-races
> > +Common Report Var(flag_store_data_races) Optimization
> > +Allow the compiler to introduce new data races on stores.
> > +
> > fargument-alias
> > Common Ignore
> > Does nothing. Preserved for backward compatibility.
> > Index: gcc/doc/invoke.texi
> > ===================================================================
> > --- gcc/doc/invoke.texi (revision 276983)
> > +++ gcc/doc/invoke.texi (working copy)
> > @@ -406,6 +406,7 @@ Objective-C and Objective-C++ Dialects}.
> > -falign-jumps[=@var{n}[:@var{m}:[@var{n2}[:@var{m2}]]]] @gol
> > -falign-labels[=@var{n}[:@var{m}:[@var{n2}[:@var{m2}]]]] @gol
> > -falign-loops[=@var{n}[:@var{m}:[@var{n2}[:@var{m2}]]]] @gol
> > +-fallow-store-data-races @gol
> > -fassociative-math -fauto-profile -fauto-profile[=@var{path}] @gol
> > -fauto-inc-dec -fbranch-probabilities @gol
> > -fcaller-saves @gol
> > @@ -8463,9 +8464,9 @@ designed to reduce code size.
> > Disregard strict standards compliance. @option{-Ofast} enables all
> > @option{-O3} optimizations. It also enables optimizations that are not
> > valid for all standard-compliant programs.
> > -It turns on @option{-ffast-math} and the Fortran-specific
> > -@option{-fstack-arrays}, unless @option{-fmax-stack-var-size} is
> > -specified, and @option{-fno-protect-parens}.
> > +It turns on @option{-ffast-math}, @option{-fallow-store-data-races}
> > +and the Fortran-specific @option{-fstack-arrays}, unless
> > +@option{-fmax-stack-var-size} is specified, and
> > @option{-fno-protect-parens}.
> >
> > @item -Og
> > @opindex Og
> > @@ -10227,6 +10228,12 @@ The maximum allowed @var{n} option value
> >
> > Enabled at levels @option{-O2}, @option{-O3}.
> >
> > +@item -fallow-store-data-races
> > +@opindex fallow-store-data-races
> > +Allow the compiler to introduce new data races on stores.
> > +
> > +Enabled at level @option{-Ofast}.
> > +
> > @item -funit-at-a-time
> > @opindex funit-at-a-time
> > This option is left for compatibility reasons. @option{-funit-at-a-time}
> > @@ -12060,10 +12067,6 @@ The maximum number of conditional store
> > if either vectorization (@option{-ftree-vectorize}) or if-conversion
> > (@option{-ftree-loop-if-convert}) is disabled.
> >
> > -@item allow-store-data-races
> > -Allow optimizers to introduce new data races on stores.
> > -Set to 1 to allow, otherwise to 0.
> > -
> > @item case-values-threshold
> > The smallest number of different values for which it is best to use a
> > jump-table instead of a tree of conditional branches. If the value is
> > Index: gcc/opts.c
> > ===================================================================
> > --- gcc/opts.c (revision 276983)
> > +++ gcc/opts.c (working copy)
> > @@ -564,6 +564,7 @@ static const struct default_options defa
> >
> > /* -Ofast adds optimizations to -O3. */
> > { OPT_LEVELS_FAST, OPT_ffast_math, NULL, 1 },
> > + { OPT_LEVELS_FAST, OPT_fallow_store_data_races, NULL, 1 },
> >
> > { OPT_LEVELS_NONE, 0, NULL, 0 }
> > };
> > @@ -671,13 +672,6 @@ default_options_optimization (struct gcc
> > opt2 ? 100 : default_param_value
> > (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE),
> > 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,
> > Index: gcc/params.def
> > ===================================================================
> > --- gcc/params.def (revision 276983)
> > +++ gcc/params.def (working copy)
> > @@ -1225,12 +1225,6 @@ DEFPARAM (PARAM_JUMP_TABLE_MAX_GROWTH_RA
> > "optimizing for speed.",
> > 800, 0, 0)
> >
> > -/* Data race flags for C++0x memory model compliance. */
> > -DEFPARAM (PARAM_ALLOW_STORE_DATA_RACES,
> > - "allow-store-data-races",
> > - "Allow new data races on stores to be introduced.",
> > - 0, 0, 1)
> > -
> > /* Reassociation width to be used by tree reassoc optimization. */
> > DEFPARAM (PARAM_TREE_REASSOC_WIDTH,
> > "tree-reassoc-width",
> > Index: gcc/params.h
> > ===================================================================
> > --- gcc/params.h (revision 276983)
> > +++ gcc/params.h (working copy)
> > @@ -228,8 +228,6 @@ extern void init_param_values (int *para
> > PARAM_VALUE (PARAM_MAX_STORES_TO_SINK)
> > #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 \
> > Index: gcc/testsuite/c-c++-common/cxxbitfields-3.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/cxxbitfields-3.c (revision 276983)
> > +++ gcc/testsuite/c-c++-common/cxxbitfields-3.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
> > -/* { dg-options "-O2 --param allow-store-data-races=0" } */
> > +/* { dg-options "-O2 -fno-allow-store-data-races" } */
> >
> > /* Make sure we don't narrow down to a QI or HI to store into VAR.J,
> > but instead use an SI. */
> > Index: gcc/testsuite/c-c++-common/cxxbitfields-6.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/cxxbitfields-6.c (revision 276983)
> > +++ gcc/testsuite/c-c++-common/cxxbitfields-6.c (working copy)
> > @@ -1,6 +1,6 @@
> > /* PR middle-end/50141 */
> > /* { dg-do compile } */
> > -/* { dg-options "-O2 --param allow-store-data-races=0" } */
> > +/* { dg-options "-O2 -fno-allow-store-data-races" } */
> >
> > struct S
> > {
> > Index: gcc/testsuite/c-c++-common/simulate-thread/bitfields-1.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/simulate-thread/bitfields-1.c (revision
> > 276983)
> > +++ gcc/testsuite/c-c++-common/simulate-thread/bitfields-1.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/c-c++-common/simulate-thread/bitfields-2.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/simulate-thread/bitfields-2.c (revision
> > 276983)
> > +++ gcc/testsuite/c-c++-common/simulate-thread/bitfields-2.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link { target { ! int16 } } } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/c-c++-common/simulate-thread/bitfields-3.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/simulate-thread/bitfields-3.c (revision
> > 276983)
> > +++ gcc/testsuite/c-c++-common/simulate-thread/bitfields-3.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/c-c++-common/simulate-thread/bitfields-4.c
> > ===================================================================
> > --- gcc/testsuite/c-c++-common/simulate-thread/bitfields-4.c (revision
> > 276983)
> > +++ gcc/testsuite/c-c++-common/simulate-thread/bitfields-4.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C
> > ===================================================================
> > --- gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C (revision 276983)
> > +++ gcc/testsuite/g++.dg/simulate-thread/bitfields-2.C (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > /* Test that setting <var.a> does not touch either <var.b> or <var.c>.
> > Index: gcc/testsuite/g++.dg/simulate-thread/bitfields.C
> > ===================================================================
> > --- gcc/testsuite/g++.dg/simulate-thread/bitfields.C (revision 276983)
> > +++ gcc/testsuite/g++.dg/simulate-thread/bitfields.C (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > /* Test that setting <var.a> does not touch either <var.b> or <var.c>.
> > Index: gcc/testsuite/gcc.dg/lto/pr52097_0.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/lto/pr52097_0.c (revision 276983)
> > +++ gcc/testsuite/gcc.dg/lto/pr52097_0.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-lto-do link } */
> > -/* { dg-lto-options { { -O -flto -fexceptions -fnon-call-exceptions --param
> > allow-store-data-races=0 } } } */
> > +/* { dg-lto-options { { -O -flto -fexceptions -fnon-call-exceptions
> > -fno-allow-store-data-races } } } */
> > /* { dg-require-effective-target exceptions } */
> >
> > typedef struct { unsigned int e0 : 16; } s1;
> > Index: gcc/testsuite/gcc.dg/simulate-thread/speculative-store-2.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/simulate-thread/speculative-store-2.c (revision
> > 276983)
> > +++ gcc/testsuite/gcc.dg/simulate-thread/speculative-store-2.c (working
> > copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0 -O2" } */
> > +/* { dg-options "-fno-allow-store-data-races -O2" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c (revision
> > 276983)
> > +++ gcc/testsuite/gcc.dg/simulate-thread/speculative-store-3.c (working
> > copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0 -O2" } */
> > +/* { dg-options "-fno-allow-store-data-races -O2" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c (revision
> > 276983)
> > +++ gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c (working
> > copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > Index: gcc/testsuite/gcc.dg/simulate-thread/speculative-store.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/simulate-thread/speculative-store.c (revision
> > 276983)
> > +++ gcc/testsuite/gcc.dg/simulate-thread/speculative-store.c (working copy)
> > @@ -1,12 +1,12 @@
> > /* { dg-do link } */
> > -/* { dg-options "--param allow-store-data-races=0" } */
> > +/* { dg-options "-fno-allow-store-data-races" } */
> > /* { dg-final { simulate-thread } } */
> >
> > #include <stdio.h>
> > #include "simulate-thread.h"
> >
> > /* This file tests that speculative store movement out of a loop doesn't
> > - happen. This is disallowed when --param allow-store-data-races is 0.
> > */
> > + happen. This is disallowed when -fno-allow-store-data-races. */
> >
> > int global = 100;
> >
> > Index: gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c (revision 276983)
> > +++ gcc/testsuite/gcc.dg/tree-ssa/20050314-1.c (working copy)
> > @@ -1,5 +1,5 @@
> > /* { dg-do compile } */
> > -/* { dg-options "-O1 -fdump-tree-lim2-details --param
> > allow-store-data-races=1" } */
> > +/* { dg-options "-O1 -fdump-tree-lim2-details -fallow-store-data-races" }
> > */
> >
> > float a[100];
> >
> > Index: gcc/tree-if-conv.c
> > ===================================================================
> > --- gcc/tree-if-conv.c (revision 276983)
> > +++ gcc/tree-if-conv.c (working copy)
> > @@ -914,10 +914,10 @@ ifcvt_memrefs_wont_trap (gimple *stmt, v
> > to unconditionally. */
> > if (base_master_dr
> > && DR_BASE_W_UNCONDITIONALLY (*base_master_dr))
> > - return PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES);
> > + return flag_store_data_races;
> > /* or the base is known to be not readonly. */
> > else if (base_object_writable (DR_REF (a)))
> > - return PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES);
> > + return flag_store_data_races;
> > }
> >
> > return false;
> > Index: gcc/tree-ssa-loop-im.c
> > ===================================================================
> > --- gcc/tree-ssa-loop-im.c (revision 276983)
> > +++ gcc/tree-ssa-loop-im.c (working copy)
> > @@ -2100,7 +2100,7 @@ execute_sm (class loop *loop, vec<edge>
> > for_each_index (&ref->mem.ref, force_move_till, &fmt_data);
> >
> > if (bb_in_transaction (loop_preheader_edge (loop)->src)
> > - || (! PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES)
> > + || (! flag_store_data_races
> > && ! ref_always_accessed_p (loop, ref, true)))
> > multi_threaded_model_p = true;
> >
> >
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
