Re: [PATCH] Do not set flag_complex_method to 2 for C++ by default.

[Andrew Pinski <pinskia at gmail dot com>]

On Thu, Nov 14, 2013 at 8:25 AM, Xinliang David Li <davidxl@google.com> wrote:
> Can we revisit the decision for this? Here are the reasons:
>
> 1) It seems that the motivation to make C++ consistent with c99 is to
> avoid confusing users who build the C source with both C and C++
> compilers. Why should C++'s default behavior be tuned for this niche
> case?

It is not a niche case.  It is confusing for people who write C++ code
to rewrite their code to C99 and find that C is much slower because of
correctness?  I think they have this backwards here.  C++ should be
consistent with C here.

> 2) It is very confusing for users who see huge performance difference
> between compiler generated code for Complex multiplication vs manually
> expanded code

I don't see why this is an issue if they understand how complex
multiplication works for correctness.  I am sorry but correctness over
speed is a good argument of why this should stay this way.

> 3) The default setting can also block potential vectorization
> opportunities for complex operations

Yes so again this is about a correctness issue over a speed issue.

> 4) GCC is about the only compiler which has this default -- very few
> user knows about GCC's strict default, and will think GCC performs
> poorly.


Correctness over speed is better.  I am sorry GCC is the only one
which gets it correct here.  If people don't like there is a flag to
disable it.

Thanks,
Andrew Pinski

>
> thanks,
>
> David
>
>
> On Wed, Nov 13, 2013 at 9:07 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Nov 13, 2013 at 5:26 PM, Cong Hou <congh@google.com> wrote:
>>> This patch is for PR58963.
>>>
>>> In the patch http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00560.html,
>>> the builtin function is used to perform complex multiplication and
>>> division. This is to comply with C99 standard, but I am wondering if
>>> C++ also needs this.
>>>
>>> There is no complex keyword in C++, and no content in C++ standard
>>> about the behavior of operations on complex types. The <complex>
>>> header file is all written in source code, including complex
>>> multiplication and division. GCC should not do too much for them by
>>> using builtin calls by default (although we can set -fcx-limited-range
>>> to prevent GCC doing this), which has a big impact on performance
>>> (there may exist vectorization opportunities).
>>>
>>> In this patch flag_complex_method will not be set to 2 for C++.
>>> Bootstraped and tested on an x86-64 machine.
>>
>> I think you need to look into this issue deeper as the original patch
>> only enabled it for C99:
>> http://gcc.gnu.org/ml/gcc-patches/2005-02/msg01483.html .
>>
>> Just a little deeper will find
>> http://gcc.gnu.org/ml/gcc/2007-07/msg00124.html which says yes C++
>> needs this.
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>>
>>> thanks,
>>> Cong
>>>
>>>
>>> Index: gcc/c-family/c-opts.c
>>> ===================================================================
>>> --- gcc/c-family/c-opts.c (revision 204712)
>>> +++ gcc/c-family/c-opts.c (working copy)
>>> @@ -198,8 +198,10 @@ c_common_init_options_struct (struct gcc
>>>    opts->x_warn_write_strings = c_dialect_cxx ();
>>>    opts->x_flag_warn_unused_result = true;
>>>
>>> -  /* By default, C99-like requirements for complex multiply and divide.  */
>>> -  opts->x_flag_complex_method = 2;
>>> +  /* By default, C99-like requirements for complex multiply and divide.
>>> +     But for C++ this should not be required.  */
>>> +  if (c_language != clk_cxx && c_language != clk_objcxx)
>>> +    opts->x_flag_complex_method = 2;
>>>  }
>>>
>>>  /* Common initialization before calling option handlers.  */
>>> Index: gcc/c-family/ChangeLog
>>> ===================================================================
>>> --- gcc/c-family/ChangeLog (revision 204712)
>>> +++ gcc/c-family/ChangeLog (working copy)
>>> @@ -1,3 +1,8 @@
>>> +2013-11-13  Cong Hou  <congh@google.com>
>>> +
>>> + * c-opts.c (c_common_init_options_struct): Don't let C++ comply with
>>> + C99-like requirements for complex multiply and divide.
>>> +
>>>  2013-11-12  Joseph Myers  <joseph@codesourcery.com>
>>>
>>>   * c-common.c (c_common_reswords): Add _Thread_local.