Re: [PATCH] Introduce -fprofile-update=maybe-atomic

["Martin Liška" <mliska@suse.cz>]

On 11/10/2016 06:31 PM, Nathan Sidwell wrote:
> On 11/10/2016 08:24 AM, Martin Liška wrote:
>> On 11/10/2016 05:17 PM, David Edelsohn wrote:
>>> Maybe instead of adding "maybe", we need to change the severity of the
>>> warning so that the warning is not emitted by default.
>>
>> Adding the warning option to -Wextra can be solution. Is it acceptable
>> approach?
> 
> I don't think that's good.  Now I understand the -pthreads thing, we have different use cases.
> 
> 1) user explicitly said -fprofile-update=FOO.  They shouldn't have to enable something else to get a diagnostic that FOO doesn't work.
> 
> 2) driver implicitly said -fprofile-update=FOO, because the user said -pthreads but the driver doesn't know if FOO is acceptable.  We want to silently fallback to the old behaviour.
> 
> The proposed solution addresses #2 by having the driver say -fprofile-update=META-FOO.  My dislike is that we're exposing this to the user and they're going to start using it.  That strikes me as undesirable.
> 
> How hard is it to implement the fprofile-update option value as a list. I.e. '-fprofile-update=atomic,single', with semantics of 'pick the first one you can do'? If that's straightforwards, then that seems to me as a better solution for #2. [flyby-thought, have 'atomic,single' as an acceptable single option value?]

Hello.

We use lists like for -fsanitize=address,undefined, however as -fprofile-update has only 3 (and passing 'single,atomic' does not make sense), I would prefer
to s/maybe-atomic/prefer-atomic. I guess handling the option list in gcc.c and doing substitutions would be very inconvenient.

Thanks,
MArtin

> 
> Failing that, Martin's solution is probably the sanest available solution, but I'd like to rename 'maybe-atomic' to the more meaningful 'prefer-atomic'.  With 'maybe-atomic', I'm left wondering if it looks at the phase of the moon.
> 
> nathan
>