| Message ID | CAK=A3=1YD0dWia2Qj=uYO2cpp6=TL0bR59n8HAKMGWAJFTtGAw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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? 2) It is very confusing for users who see huge performance difference between compiler generated code for Complex multiplication vs manually expanded code 3) The default setting can also block potential vectorization opportunities for complex operations 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. 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.
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.
See the following code:
#include <complex>
using std::complex;
template<typename _Tp, typename _Up>
complex<_Tp>&
mult_assign (complex<_Tp>& __y, const complex<_Up>& __z)
{
_Up& _M_real = __y.real();
_Up& _M_imag = __y.imag();
const _Tp __r = _M_real * __z.real() - _M_imag * __z.imag();
_M_imag = _M_real * __z.imag() + _M_imag * __z.real();
_M_real = __r;
return __y;
}
void foo (complex<float>& c1, complex<float>& c2)
{ c1 *= c2; }
void bar (complex<float>& c1, complex<float>& c2)
{ mult_assign(c1, c2); }
The function mult_assign is written almost by copying the
implementation of operator *= from <complex>. They have exactly the
same behavior from the view of the source code. However, the compiled
results of foo() and bar() are different: foo() is using builtin
function for multiplication but bar() is not. Just because of a name
change the final behavior is changed? This should not be how a
compiler is working.
thanks,
Cong
On Thu, Nov 14, 2013 at 10:17 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> 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.
On Thu, Nov 14, 2013 at 10:17 AM, Andrew Pinski <pinskia@gmail.com> wrote: > 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 Compared with people who just work on C++ or C but do not worry about rewrite nor cross language comparison? >and find that C is much slower because of > correctness? I think they have this backwards here. C++ should be > consistent with C here. Correctness by what definition? > >> 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. You can say the same thing that people who find C is slower can use the flag to disable it. thanks, David > > 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.
On Thu, Nov 14, 2013 at 11:12 AM, Xinliang David Li <davidxl@google.com> wrote: > On Thu, Nov 14, 2013 at 10:17 AM, Andrew Pinski <pinskia@gmail.com> wrote: >> 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 > > Compared with people who just work on C++ or C but do not worry about > rewrite nor cross language comparison? > >>and find that C is much slower because of >> correctness? I think they have this backwards here. C++ should be >> consistent with C here. > > Correctness by what definition? See also http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00568.html which is one of the libstdc++ maintainers opinion on this subject. > >> >>> 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. > > You can say the same thing that people who find C is slower can use > the flag to disable it. > > thanks, > > David > >> >> 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.
On 1/7/2014 8:46 PM, Andrew Pinski wrote: >>> 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. Obviously in a case like this, it is the programmer who should be able to decide between fast-and-acceptable and slow-and-accurate. This is an old debate (e.g. consider Cray, who always went for the fast-and-acceptable path, and was able to build machines that were interestingly fast partly as a result of this philosophy). So having a switch is not controversial But then the question is, what should the default be. The trouble with the slow-but-accurate is that many users will never know about the switch, and will judge the compiler ONLY on the basis that it is slow, without even knowing, noticing, or caring that it is "more correct" than the competition. We have seen gcc lose out in a number of head to head comparisons, because GCC defaulted to -O0 (optimization really really off, and don't care how horrible the code is) and the competition defaulted to optimization turned on. We even worked with one customer, and explained the issue, and they said "sorry, company procedures require us to run both compilers with their default settings, since that is perceived as being fairer!" Their conclusion was that gcc was unacceptably inefficient and they went with the competition. >> >> You can say the same thing that people who find C is slower can use >> the flag to disable it. >> >> thanks, >> >> David >> >>> >>> 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.
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.