[PATCH,bionic] Add -foptimize-sincos
Bionic prior to Gingerbread doesn't support sincos*, but upstream GCC enables sincos optimization for OPTION_BIONIC unconditionally since 4.6. I'd like to propose a new flag -foptimize-sincos for NDK to maintain backward compatibility. 1. For BIONIC: sincos optimization is disabled by default. Apps built for Gingerbread+ or AOSP platform build which uses the same compiler as NDK, can add -foptimize-sincos to enhance performance. (although it's likely that only benchmarks may benefit) 2. Other C libs aren't affected. sincos optimization is enabled for GLIBC and disabled for othres, regardless of this flag. Index: gcc/common.opt === --- gcc/common.opt (revision 199277) +++ gcc/common.opt (working copy) @@ -1591,6 +1591,10 @@ Common Report Var(flag_optimize_sibling_calls) Optimization Optimize sibling and tail recursive calls +foptimize-sincos +Common Report Var(flag_optimize_sincos) Optimization +Optimize calls to sin() and cos() with the same argument to sincos() + fpartial-inlining Common Report Var(flag_partial_inlining) Perform partial inlining Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 199277) +++ gcc/doc/invoke.texi (working copy) @@ -388,7 +388,7 @@ -fno-sched-interblock -fno-sched-spec -fno-signed-zeros @gol -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss @gol -fomit-frame-pointer -foptimize-register-move -foptimize-sibling-calls @gol --fpartial-inlining -fpeel-loops -fpredictive-commoning @gol +-foptimize-sincos -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol -fprefetch-loop-arrays -fprofile-report @gol -fprofile-correction -fprofile-dir=@var{path} -fprofile-generate @gol -fprofile-generate=@var{path} @gol @@ -6586,6 +6586,7 @@ -fipa-profile @gol -fipa-reference @gol -fmerge-constants +-foptimize-sincos @gol -fsplit-wide-types @gol -ftree-bit-ccp @gol -ftree-builtin-call-dce @gol @@ -6772,6 +6773,12 @@ Enabled at levels @option{-O2}, @option{-O3}, @option{-Os}. +@item -foptimize-sincos +@opindex foptimize-sincos +Optimize calls to sin() and cos() with the same argument to sincos() + +Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os}. + @item -fno-inline @opindex fno-inline Do not expand any functions inline apart from those marked with Index: gcc/config/linux.h === --- gcc/config/linux.h (revision 199277) +++ gcc/config/linux.h (working copy) @@ -102,7 +102,7 @@ /* Whether we have sincos that follows the GNU extension. */ #undef TARGET_HAS_SINCOS -#define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC) +#define TARGET_HAS_SINCOS (OPTION_GLIBC || (OPTION_BIONIC flag_optimize_sincos)) /* Whether we have Bionic libc runtime */ #undef TARGET_HAS_BIONIC
Re: [PATCH,bionic] Add -foptimize-sincos
On Fri, May 24, 2013 at 12:53 PM, Andrew Hsieh andrewhs...@google.com wrote: Bionic prior to Gingerbread doesn't support sincos*, but upstream GCC enables sincos optimization for OPTION_BIONIC unconditionally since 4.6. I'd like to propose a new flag -foptimize-sincos for NDK to maintain backward compatibility. 1. For BIONIC: sincos optimization is disabled by default. Apps built for Gingerbread+ or AOSP platform build which uses the same compiler as NDK, can add -foptimize-sincos to enhance performance. (although it's likely that only benchmarks may benefit) 2. Other C libs aren't affected. sincos optimization is enabled for GLIBC and disabled for othres, regardless of this flag. That's a pretty awful option name for one that makes us assume the target C library has a sincos function. I'd rather think about a way to specify, for all known builtins, whether GCC should generate calls to such function where they are not in the source program. That is, similar to how we have -f[no-]builtin-FOO introduce -f[no-]libc-FOO (with a better name for 'libc'?). That way there would be a way to specify that -floop-distribute-patterns should not produce calls to memset () for example. Richard. Index: gcc/common.opt === --- gcc/common.opt (revision 199277) +++ gcc/common.opt (working copy) @@ -1591,6 +1591,10 @@ Common Report Var(flag_optimize_sibling_calls) Optimization Optimize sibling and tail recursive calls +foptimize-sincos +Common Report Var(flag_optimize_sincos) Optimization +Optimize calls to sin() and cos() with the same argument to sincos() + fpartial-inlining Common Report Var(flag_partial_inlining) Perform partial inlining Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 199277) +++ gcc/doc/invoke.texi (working copy) @@ -388,7 +388,7 @@ -fno-sched-interblock -fno-sched-spec -fno-signed-zeros @gol -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss @gol -fomit-frame-pointer -foptimize-register-move -foptimize-sibling-calls @gol --fpartial-inlining -fpeel-loops -fpredictive-commoning @gol +-foptimize-sincos -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol -fprefetch-loop-arrays -fprofile-report @gol -fprofile-correction -fprofile-dir=@var{path} -fprofile-generate @gol -fprofile-generate=@var{path} @gol @@ -6586,6 +6586,7 @@ -fipa-profile @gol -fipa-reference @gol -fmerge-constants +-foptimize-sincos @gol -fsplit-wide-types @gol -ftree-bit-ccp @gol -ftree-builtin-call-dce @gol @@ -6772,6 +6773,12 @@ Enabled at levels @option{-O2}, @option{-O3}, @option{-Os}. +@item -foptimize-sincos +@opindex foptimize-sincos +Optimize calls to sin() and cos() with the same argument to sincos() + +Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os}. + @item -fno-inline @opindex fno-inline Do not expand any functions inline apart from those marked with Index: gcc/config/linux.h === --- gcc/config/linux.h (revision 199277) +++ gcc/config/linux.h (working copy) @@ -102,7 +102,7 @@ /* Whether we have sincos that follows the GNU extension. */ #undef TARGET_HAS_SINCOS -#define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC) +#define TARGET_HAS_SINCOS (OPTION_GLIBC || (OPTION_BIONIC flag_optimize_sincos)) /* Whether we have Bionic libc runtime */ #undef TARGET_HAS_BIONIC
Re: [PATCH,bionic] Add -foptimize-sincos
On Fri, May 24, 2013 at 02:10:18PM +0200, Richard Biener wrote: That's a pretty awful option name for one that makes us assume the target C library has a sincos function. I'd rather think about a way to specify, for all known builtins, whether GCC should generate calls to such function where they are not in the source program. That is, similar to how we have -f[no-]builtin-FOO introduce -f[no-]libc-FOO (with a better name for 'libc'?). That way there would be a way to specify that -floop-distribute-patterns should not produce calls to memset () for example. Yeah. Or we could be more aggressive at producing stpcpy, mempcpy etc. calls where it could be beneficial and have a way to disable that. What we currently do for stpcpy is c/c-decl.c (merge_decl) and cp/decl.c (duplicate_decls) has code that if not -fno-builtin-stpcpy and a compatible prototype is seen for stpcpy, then we allow it to be generated implicitly (set_builtin_decl_implicit_p (fncode, true);). Yeah, we could do the same for sincos, I bet most of the people don't prototype sin or cos themselves but include math.h, but in all cases it means the compiler will do it only if stpcpy resp. sincos is actually prototyped in the headers (right feature test macros for that). Jakub
Re: [PATCH,bionic] Add -foptimize-sincos
Richard, the target hook (libc_has_function) for what you described is waiting for a review: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01201.html However, it doesn't have command line options support. Alexander 2013/5/24 Richard Biener richard.guent...@gmail.com: On Fri, May 24, 2013 at 12:53 PM, Andrew Hsieh andrewhs...@google.com wrote: Bionic prior to Gingerbread doesn't support sincos*, but upstream GCC enables sincos optimization for OPTION_BIONIC unconditionally since 4.6. I'd like to propose a new flag -foptimize-sincos for NDK to maintain backward compatibility. 1. For BIONIC: sincos optimization is disabled by default. Apps built for Gingerbread+ or AOSP platform build which uses the same compiler as NDK, can add -foptimize-sincos to enhance performance. (although it's likely that only benchmarks may benefit) 2. Other C libs aren't affected. sincos optimization is enabled for GLIBC and disabled for othres, regardless of this flag. That's a pretty awful option name for one that makes us assume the target C library has a sincos function. I'd rather think about a way to specify, for all known builtins, whether GCC should generate calls to such function where they are not in the source program. That is, similar to how we have -f[no-]builtin-FOO introduce -f[no-]libc-FOO (with a better name for 'libc'?). That way there would be a way to specify that -floop-distribute-patterns should not produce calls to memset () for example. Richard. Index: gcc/common.opt === --- gcc/common.opt (revision 199277) +++ gcc/common.opt (working copy) @@ -1591,6 +1591,10 @@ Common Report Var(flag_optimize_sibling_calls) Optimization Optimize sibling and tail recursive calls +foptimize-sincos +Common Report Var(flag_optimize_sincos) Optimization +Optimize calls to sin() and cos() with the same argument to sincos() + fpartial-inlining Common Report Var(flag_partial_inlining) Perform partial inlining Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 199277) +++ gcc/doc/invoke.texi (working copy) @@ -388,7 +388,7 @@ -fno-sched-interblock -fno-sched-spec -fno-signed-zeros @gol -fno-toplevel-reorder -fno-trapping-math -fno-zero-initialized-in-bss @gol -fomit-frame-pointer -foptimize-register-move -foptimize-sibling-calls @gol --fpartial-inlining -fpeel-loops -fpredictive-commoning @gol +-foptimize-sincos -fpartial-inlining -fpeel-loops -fpredictive-commoning @gol -fprefetch-loop-arrays -fprofile-report @gol -fprofile-correction -fprofile-dir=@var{path} -fprofile-generate @gol -fprofile-generate=@var{path} @gol @@ -6586,6 +6586,7 @@ -fipa-profile @gol -fipa-reference @gol -fmerge-constants +-foptimize-sincos @gol -fsplit-wide-types @gol -ftree-bit-ccp @gol -ftree-builtin-call-dce @gol @@ -6772,6 +6773,12 @@ Enabled at levels @option{-O2}, @option{-O3}, @option{-Os}. +@item -foptimize-sincos +@opindex foptimize-sincos +Optimize calls to sin() and cos() with the same argument to sincos() + +Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os}. + @item -fno-inline @opindex fno-inline Do not expand any functions inline apart from those marked with Index: gcc/config/linux.h === --- gcc/config/linux.h (revision 199277) +++ gcc/config/linux.h (working copy) @@ -102,7 +102,7 @@ /* Whether we have sincos that follows the GNU extension. */ #undef TARGET_HAS_SINCOS -#define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC) +#define TARGET_HAS_SINCOS (OPTION_GLIBC || (OPTION_BIONIC flag_optimize_sincos)) /* Whether we have Bionic libc runtime */ #undef TARGET_HAS_BIONIC
Re: [PATCH,bionic] Add -foptimize-sincos
On Fri, May 24, 2013 at 2:18 PM, Jakub Jelinek ja...@redhat.com wrote: On Fri, May 24, 2013 at 02:10:18PM +0200, Richard Biener wrote: That's a pretty awful option name for one that makes us assume the target C library has a sincos function. I'd rather think about a way to specify, for all known builtins, whether GCC should generate calls to such function where they are not in the source program. That is, similar to how we have -f[no-]builtin-FOO introduce -f[no-]libc-FOO (with a better name for 'libc'?). That way there would be a way to specify that -floop-distribute-patterns should not produce calls to memset () for example. Yeah. Or we could be more aggressive at producing stpcpy, mempcpy etc. calls where it could be beneficial and have a way to disable that. What we currently do for stpcpy is c/c-decl.c (merge_decl) and cp/decl.c (duplicate_decls) has code that if not -fno-builtin-stpcpy and a compatible prototype is seen for stpcpy, then we allow it to be generated implicitly (set_builtin_decl_implicit_p (fncode, true);). But for example memset/memcpy always have that set, even if no prototype is in the source. So, is that decl_implicit_p really supposed to tell us whether we've seen a compatible prototype? Yeah, we could do the same for sincos, I bet most of the people don't prototype sin or cos themselves but include math.h, but in all cases it means the compiler will do it only if stpcpy resp. sincos is actually prototyped in the headers (right feature test macros for that). Right, that would be good enough for C and C++ - but what to do for Fortran? Richard. Jakub
Re: [PATCH,bionic] Add -foptimize-sincos
On Fri, May 24, 2013 at 02:23:45PM +0200, Richard Biener wrote: But for example memset/memcpy always have that set, even if no prototype is in the source. So, is that decl_implicit_p really supposed to tell us whether we've seen a compatible prototype? decl_implicit_p isn't whether we've seen a compatible prototype, but whether it is ok for the compiler to make calls to that function, even when there were none in the source. Usually this comes from how we define the builtin in builtins.def, whether it is a standard required function or some extension beyond the standard. And for stpcpy right now we are using this way (kind of hack). Right, that would be good enough for C and C++ - but what to do for Fortran? Sure. So what about have a -f{,no-}builtin-implicit-{builtin_name} with the default as is right now, that would just tweak decl_implicit_p if decl_explicit_p is true. Jakub
Re: [PATCH,bionic] Add -foptimize-sincos
On Fri, May 24, 2013 at 2:28 PM, Jakub Jelinek ja...@redhat.com wrote: On Fri, May 24, 2013 at 02:23:45PM +0200, Richard Biener wrote: But for example memset/memcpy always have that set, even if no prototype is in the source. So, is that decl_implicit_p really supposed to tell us whether we've seen a compatible prototype? decl_implicit_p isn't whether we've seen a compatible prototype, but whether it is ok for the compiler to make calls to that function, even when there were none in the source. Usually this comes from how we define the builtin in builtins.def, whether it is a standard required function or some extension beyond the standard. And for stpcpy right now we are using this way (kind of hack). Right, that would be good enough for C and C++ - but what to do for Fortran? Sure. So what about have a -f{,no-}builtin-implicit-{builtin_name} with the default as is right now, that would just tweak decl_implicit_p if decl_explicit_p is true. That works for me. We could still have a way for targets to override the default here. Richard. Jakub