Hi, On Thu, 26 Jun 2014 08:43:46, Richard Biener wrote: > > On June 26, 2014 12:03:21 AM CEST, Martin Jambor <mjam...@suse.cz> wrote: >>Hi, >> >>On Wed, Jun 25, 2014 at 03:14:31PM -0600, Jeff Law wrote: >>> On 06/24/14 14:19, Martin Jambor wrote: >>>>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. >>> Don't we want to deprecate, not remove the dead options? >>> >> >>Is there a mechanism for deprecating parameters (I could not quickly >>find any) or do you mean to leave them there and only document them as >>deprecated? >> >>I am not really concerned how we deal with the unused parameters, >>removing or any form of deprecating is fine with me. > > --params are not a stable interface, so we can just remove those. Of course > this would be the opportunity to introduce a real option for this task and > leave the param as an implementation detail. >
well, of course, given the fact that the --param allow-store-data-races=0 is actually used now by linux kernel makefiles we should keep this parameter. I'd agree with Richard about the other parameters. Note however that they are not really a secret any more: See https://gcc.gnu.org/wiki/Atomic/GCCMM/ExecutiveSummary where these --params are documented, should this page be adjusted too when we remove them? Bernd. > Richard. > >>Thanks, >> >>Martin > >
