On Thu, Nov 14, 2013 at 8:25 AM, Xinliang David Li <davi...@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 <pins...@gmail.com> wrote: >> On Wed, Nov 13, 2013 at 5:26 PM, Cong Hou <co...@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 <co...@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 <jos...@codesourcery.com> >>> >>> * c-common.c (c_common_reswords): Add _Thread_local.
