[PATCH,bionic] Add -foptimize-sincos

2013-05-24 Thread Andrew Hsieh
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

2013-05-24 Thread Richard Biener
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

2013-05-24 Thread Jakub Jelinek
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

2013-05-24 Thread Alexander Ivchenko
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

2013-05-24 Thread Richard Biener
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

2013-05-24 Thread Jakub Jelinek
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

2013-05-24 Thread Richard Biener
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