Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-23 Thread Richard Biener
On Fri, Aug 23, 2019 at 3:40 AM luoxhu  wrote:
>
> Hi Richard,
>
> On 2019/8/13 17:10, Richard Biener wrote:
> > On Tue, Aug 13, 2019 at 4:22 AM luoxhu  wrote:
> >>
> >> Hi Richard,
> >>
> >> On 2019/8/12 16:51, Richard Biener wrote:
> >>> On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:
> 
>  Hi Richard,
>  Thanks for your comments, updated the v2 patch as below:
>  1. Define and use builtin_with_linkage_p.
>  2. Add comments.
>  3. Add a testcase.
> 
>  In LTO mode, if static library and dynamic library contains same
>  function and both libraries are passed as arguments, linker will link
>  the function in dynamic library no matter the sequence.  This patch
>  will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
>  is a math function, then the function in static library will be linked
>  first if its sequence is ahead of the dynamic library.
> >>>
> >>> Comments below
> >>>
>  gcc/ChangeLog
> 
>    2019-08-12  Xiong Hu Luo  
> 
>    PR lto/91287
>    * builtins.c (builtin_with_linkage_p): New function.
>    * builtins.h (builtin_with_linkage_p): New function.
>    * symtab.c (write_symbol): Use builtin_with_linkage_p.
>    * lto-streamer-out.c 
>  (symtab_node::output_to_lto_symbol_table_p):
>    Likewise.
> 
>  gcc/testsuite/ChangeLog
> 
>    2019-08-12  Xiong Hu Luo  
> 
>    PR lto/91287
>    * gcc.dg/pr91287.c: New testcase.
>  ---
> gcc/builtins.c | 89 ++
> gcc/builtins.h |  2 +
> gcc/lto-streamer-out.c |  4 +-
> gcc/symtab.c   | 13 -
> gcc/testsuite/gcc.dg/pr91287.c | 40 +++
> 5 files changed, 145 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/pr91287.c
> 
>  diff --git a/gcc/builtins.c b/gcc/builtins.c
>  index 695a9d191af..f4dea941a27 100644
>  --- a/gcc/builtins.c
>  +++ b/gcc/builtins.c
>  @@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
>   *p = (char)tree_to_uhwi (t);
>   return true;
> }
>  +
>  +/* Return true if DECL is a specified builtin math function.  These 
>  functions
>  +   should have symbol in symbol table to provide linkage with faster 
>  version of
>  +   libraries.  */
> >>>
> >>> The comment should read like
> >>>
> >>> /* Return true if the builtin DECL is implemented in a standard
> >>> library.  Otherwise
> >>>  returns false which doesn't guarantee it is not (thus the list of
> >>> handled builtins
> >>>  below may be incomplete).  */
> >>>
>  +bool
>  +builtin_with_linkage_p (tree decl)
>  +{
>  +  if (!decl)
>  +return false;
> >>>
> >>> Omit this check please.
> >>>
>  +  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
>  +switch (DECL_FUNCTION_CODE (decl))
>  +{
>  +  CASE_FLT_FN (BUILT_IN_ACOS):
>  +  CASE_FLT_FN (BUILT_IN_ACOSH):
>  +  CASE_FLT_FN (BUILT_IN_ASIN):
>  +  CASE_FLT_FN (BUILT_IN_ASINH):
>  +  CASE_FLT_FN (BUILT_IN_ATAN):
>  +  CASE_FLT_FN (BUILT_IN_ATANH):
>  +  CASE_FLT_FN (BUILT_IN_ATAN2):
>  +  CASE_FLT_FN (BUILT_IN_CBRT):
>  +  CASE_FLT_FN (BUILT_IN_CEIL):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
>  +  CASE_FLT_FN (BUILT_IN_COPYSIGN):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
>  +  CASE_FLT_FN (BUILT_IN_COS):
>  +  CASE_FLT_FN (BUILT_IN_COSH):
>  +  CASE_FLT_FN (BUILT_IN_ERF):
>  +  CASE_FLT_FN (BUILT_IN_ERFC):
>  +  CASE_FLT_FN (BUILT_IN_EXP):
>  +  CASE_FLT_FN (BUILT_IN_EXP2):
>  +  CASE_FLT_FN (BUILT_IN_EXPM1):
>  +  CASE_FLT_FN (BUILT_IN_FABS):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
>  +  CASE_FLT_FN (BUILT_IN_FDIM):
>  +  CASE_FLT_FN (BUILT_IN_FLOOR):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
>  +  CASE_FLT_FN (BUILT_IN_FMA):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
>  +  CASE_FLT_FN (BUILT_IN_FMAX):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
>  +  CASE_FLT_FN (BUILT_IN_FMIN):
>  +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
>  +  CASE_FLT_FN (BUILT_IN_FMOD):
>  +  CASE_FLT_FN (BUILT_IN_FREXP):
>  +  CASE_FLT_FN (BUILT_IN_HYPOT):
>  +  CASE_FLT_FN (BUILT_IN_ILOGB):
>  +  CASE_FLT_FN (BUILT_IN_LDEXP):
>  +  CASE_FLT_FN (BUILT_IN_LGAMMA):
>  +  CASE_FLT_FN (BUILT_IN_LLRINT):
>  +  CASE_FLT_FN (BUILT_IN_LLROUND):
>  +  CASE_FLT_FN (BUILT_IN_LOG):
>  +  CASE_FLT_FN (BUILT_IN_LOG10):
>  +  CASE_FLT_FN (BUILT_IN_LOG1P):
>  +  CASE_FLT_FN (BUILT_IN_LOG2):
> 

Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-22 Thread luoxhu

Hi Richard,

On 2019/8/13 17:10, Richard Biener wrote:

On Tue, Aug 13, 2019 at 4:22 AM luoxhu  wrote:


Hi Richard,

On 2019/8/12 16:51, Richard Biener wrote:

On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:


Hi Richard,
Thanks for your comments, updated the v2 patch as below:
1. Define and use builtin_with_linkage_p.
2. Add comments.
3. Add a testcase.

In LTO mode, if static library and dynamic library contains same
function and both libraries are passed as arguments, linker will link
the function in dynamic library no matter the sequence.  This patch
will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
is a math function, then the function in static library will be linked
first if its sequence is ahead of the dynamic library.


Comments below


gcc/ChangeLog

  2019-08-12  Xiong Hu Luo  

  PR lto/91287
  * builtins.c (builtin_with_linkage_p): New function.
  * builtins.h (builtin_with_linkage_p): New function.
  * symtab.c (write_symbol): Use builtin_with_linkage_p.
  * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
  Likewise.

gcc/testsuite/ChangeLog

  2019-08-12  Xiong Hu Luo  

  PR lto/91287
  * gcc.dg/pr91287.c: New testcase.
---
   gcc/builtins.c | 89 ++
   gcc/builtins.h |  2 +
   gcc/lto-streamer-out.c |  4 +-
   gcc/symtab.c   | 13 -
   gcc/testsuite/gcc.dg/pr91287.c | 40 +++
   5 files changed, 145 insertions(+), 3 deletions(-)
   create mode 100644 gcc/testsuite/gcc.dg/pr91287.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 695a9d191af..f4dea941a27 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
 *p = (char)tree_to_uhwi (t);
 return true;
   }
+
+/* Return true if DECL is a specified builtin math function.  These functions
+   should have symbol in symbol table to provide linkage with faster version of
+   libraries.  */


The comment should read like

/* Return true if the builtin DECL is implemented in a standard
library.  Otherwise
 returns false which doesn't guarantee it is not (thus the list of
handled builtins
 below may be incomplete).  */


+bool
+builtin_with_linkage_p (tree decl)
+{
+  if (!decl)
+return false;


Omit this check please.


+  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
+switch (DECL_FUNCTION_CODE (decl))
+{
+  CASE_FLT_FN (BUILT_IN_ACOS):
+  CASE_FLT_FN (BUILT_IN_ACOSH):
+  CASE_FLT_FN (BUILT_IN_ASIN):
+  CASE_FLT_FN (BUILT_IN_ASINH):
+  CASE_FLT_FN (BUILT_IN_ATAN):
+  CASE_FLT_FN (BUILT_IN_ATANH):
+  CASE_FLT_FN (BUILT_IN_ATAN2):
+  CASE_FLT_FN (BUILT_IN_CBRT):
+  CASE_FLT_FN (BUILT_IN_CEIL):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
+  CASE_FLT_FN (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN (BUILT_IN_COS):
+  CASE_FLT_FN (BUILT_IN_COSH):
+  CASE_FLT_FN (BUILT_IN_ERF):
+  CASE_FLT_FN (BUILT_IN_ERFC):
+  CASE_FLT_FN (BUILT_IN_EXP):
+  CASE_FLT_FN (BUILT_IN_EXP2):
+  CASE_FLT_FN (BUILT_IN_EXPM1):
+  CASE_FLT_FN (BUILT_IN_FABS):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
+  CASE_FLT_FN (BUILT_IN_FDIM):
+  CASE_FLT_FN (BUILT_IN_FLOOR):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
+  CASE_FLT_FN (BUILT_IN_FMA):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
+  CASE_FLT_FN (BUILT_IN_FMAX):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
+  CASE_FLT_FN (BUILT_IN_FMIN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
+  CASE_FLT_FN (BUILT_IN_FMOD):
+  CASE_FLT_FN (BUILT_IN_FREXP):
+  CASE_FLT_FN (BUILT_IN_HYPOT):
+  CASE_FLT_FN (BUILT_IN_ILOGB):
+  CASE_FLT_FN (BUILT_IN_LDEXP):
+  CASE_FLT_FN (BUILT_IN_LGAMMA):
+  CASE_FLT_FN (BUILT_IN_LLRINT):
+  CASE_FLT_FN (BUILT_IN_LLROUND):
+  CASE_FLT_FN (BUILT_IN_LOG):
+  CASE_FLT_FN (BUILT_IN_LOG10):
+  CASE_FLT_FN (BUILT_IN_LOG1P):
+  CASE_FLT_FN (BUILT_IN_LOG2):
+  CASE_FLT_FN (BUILT_IN_LOGB):
+  CASE_FLT_FN (BUILT_IN_LRINT):
+  CASE_FLT_FN (BUILT_IN_LROUND):
+  CASE_FLT_FN (BUILT_IN_MODF):
+  CASE_FLT_FN (BUILT_IN_NAN):
+  CASE_FLT_FN (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
+  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
+  CASE_FLT_FN (BUILT_IN_POW):
+  CASE_FLT_FN (BUILT_IN_REMAINDER):
+  CASE_FLT_FN (BUILT_IN_REMQUO):
+  CASE_FLT_FN (BUILT_IN_RINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
+  CASE_FLT_FN (BUILT_IN_ROUND):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
+  CASE_FLT_FN (BUILT_IN_SCALBLN):
+  CASE_FLT_FN (BUILT_IN_SCALBN):
+  CASE_FLT_FN (BUILT_IN_SIN):
+  CASE_FLT_FN (BUILT_IN_SINH):
+  CASE_FLT_FN (BUILT_IN_SINCOS):
+  CASE_FLT_FN (BUILT_IN_SQRT):
+  

Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-13 Thread Richard Biener
On Tue, Aug 13, 2019 at 4:22 AM luoxhu  wrote:
>
> Hi Richard,
>
> On 2019/8/12 16:51, Richard Biener wrote:
> > On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:
> >>
> >> Hi Richard,
> >> Thanks for your comments, updated the v2 patch as below:
> >> 1. Define and use builtin_with_linkage_p.
> >> 2. Add comments.
> >> 3. Add a testcase.
> >>
> >> In LTO mode, if static library and dynamic library contains same
> >> function and both libraries are passed as arguments, linker will link
> >> the function in dynamic library no matter the sequence.  This patch
> >> will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
> >> is a math function, then the function in static library will be linked
> >> first if its sequence is ahead of the dynamic library.
> >
> > Comments below
> >
> >> gcc/ChangeLog
> >>
> >>  2019-08-12  Xiong Hu Luo  
> >>
> >>  PR lto/91287
> >>  * builtins.c (builtin_with_linkage_p): New function.
> >>  * builtins.h (builtin_with_linkage_p): New function.
> >>  * symtab.c (write_symbol): Use builtin_with_linkage_p.
> >>  * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
> >>  Likewise.
> >>
> >> gcc/testsuite/ChangeLog
> >>
> >>  2019-08-12  Xiong Hu Luo  
> >>
> >>  PR lto/91287
> >>  * gcc.dg/pr91287.c: New testcase.
> >> ---
> >>   gcc/builtins.c | 89 ++
> >>   gcc/builtins.h |  2 +
> >>   gcc/lto-streamer-out.c |  4 +-
> >>   gcc/symtab.c   | 13 -
> >>   gcc/testsuite/gcc.dg/pr91287.c | 40 +++
> >>   5 files changed, 145 insertions(+), 3 deletions(-)
> >>   create mode 100644 gcc/testsuite/gcc.dg/pr91287.c
> >>
> >> diff --git a/gcc/builtins.c b/gcc/builtins.c
> >> index 695a9d191af..f4dea941a27 100644
> >> --- a/gcc/builtins.c
> >> +++ b/gcc/builtins.c
> >> @@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
> >> *p = (char)tree_to_uhwi (t);
> >> return true;
> >>   }
> >> +
> >> +/* Return true if DECL is a specified builtin math function.  These 
> >> functions
> >> +   should have symbol in symbol table to provide linkage with faster 
> >> version of
> >> +   libraries.  */
> >
> > The comment should read like
> >
> > /* Return true if the builtin DECL is implemented in a standard
> > library.  Otherwise
> > returns false which doesn't guarantee it is not (thus the list of
> > handled builtins
> > below may be incomplete).  */
> >
> >> +bool
> >> +builtin_with_linkage_p (tree decl)
> >> +{
> >> +  if (!decl)
> >> +return false;
> >
> > Omit this check please.
> >
> >> +  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
> >> +switch (DECL_FUNCTION_CODE (decl))
> >> +{
> >> +  CASE_FLT_FN (BUILT_IN_ACOS):
> >> +  CASE_FLT_FN (BUILT_IN_ACOSH):
> >> +  CASE_FLT_FN (BUILT_IN_ASIN):
> >> +  CASE_FLT_FN (BUILT_IN_ASINH):
> >> +  CASE_FLT_FN (BUILT_IN_ATAN):
> >> +  CASE_FLT_FN (BUILT_IN_ATANH):
> >> +  CASE_FLT_FN (BUILT_IN_ATAN2):
> >> +  CASE_FLT_FN (BUILT_IN_CBRT):
> >> +  CASE_FLT_FN (BUILT_IN_CEIL):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
> >> +  CASE_FLT_FN (BUILT_IN_COPYSIGN):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
> >> +  CASE_FLT_FN (BUILT_IN_COS):
> >> +  CASE_FLT_FN (BUILT_IN_COSH):
> >> +  CASE_FLT_FN (BUILT_IN_ERF):
> >> +  CASE_FLT_FN (BUILT_IN_ERFC):
> >> +  CASE_FLT_FN (BUILT_IN_EXP):
> >> +  CASE_FLT_FN (BUILT_IN_EXP2):
> >> +  CASE_FLT_FN (BUILT_IN_EXPM1):
> >> +  CASE_FLT_FN (BUILT_IN_FABS):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
> >> +  CASE_FLT_FN (BUILT_IN_FDIM):
> >> +  CASE_FLT_FN (BUILT_IN_FLOOR):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
> >> +  CASE_FLT_FN (BUILT_IN_FMA):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
> >> +  CASE_FLT_FN (BUILT_IN_FMAX):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
> >> +  CASE_FLT_FN (BUILT_IN_FMIN):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
> >> +  CASE_FLT_FN (BUILT_IN_FMOD):
> >> +  CASE_FLT_FN (BUILT_IN_FREXP):
> >> +  CASE_FLT_FN (BUILT_IN_HYPOT):
> >> +  CASE_FLT_FN (BUILT_IN_ILOGB):
> >> +  CASE_FLT_FN (BUILT_IN_LDEXP):
> >> +  CASE_FLT_FN (BUILT_IN_LGAMMA):
> >> +  CASE_FLT_FN (BUILT_IN_LLRINT):
> >> +  CASE_FLT_FN (BUILT_IN_LLROUND):
> >> +  CASE_FLT_FN (BUILT_IN_LOG):
> >> +  CASE_FLT_FN (BUILT_IN_LOG10):
> >> +  CASE_FLT_FN (BUILT_IN_LOG1P):
> >> +  CASE_FLT_FN (BUILT_IN_LOG2):
> >> +  CASE_FLT_FN (BUILT_IN_LOGB):
> >> +  CASE_FLT_FN (BUILT_IN_LRINT):
> >> +  CASE_FLT_FN (BUILT_IN_LROUND):
> >> +  CASE_FLT_FN (BUILT_IN_MODF):
> >> +  CASE_FLT_FN (BUILT_IN_NAN):
> >> +  CASE_FLT_FN (BUILT_IN_NEARBYINT):
> >> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
> >> +  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
> >> +  CASE_FLT_FN 

Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-12 Thread luoxhu



On 2019/8/13 10:22, luoxhu wrote:
diff --git a/gcc/testsuite/gcc.dg/pr91287.c 
b/gcc/testsuite/gcc.dg/pr91287.c

new file mode 100644
index 000..c816e0537aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91287.c
@@ -0,0 +1,40 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */


You don't use -flto here so the testcase doesn't exercise any of the patched
code.  Does it work when you add -flto here?  That is, do
scan-symbol[-not] properly use gcc-nm or the linker plugin?


-flto is needed here to check this patch correctness, my mistake here, 
thanks for catching.  atan2 will exists in pr91287.o even without lto as

pr91287.o has the instruction "bl atan2".
After adding -flto the case also works as symbol is written to pr91287.o.
PS: update other changes in patch attached.

What's more, this test case depends on this patch 
https://www.sourceware.org/ml/binutils/2019-08/msg00113.html.

otherwise nm will report error (use plugin or local gcc-nm is OK):

PASS: gcc.dg/pr91287.c (test for excess errors)
ERROR: gcc.dg/pr91287.c: error executing dg-final: /usr/bin/nm: pr91287.o: 
plugin needed to handle lto object
UNRESOLVED: gcc.dg/pr91287.c: error executing dg-final: /usr/bin/nm: 
pr91287.o: plugin needed to handle lto object


Xionghu



Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-12 Thread luoxhu

Hi Richard,

On 2019/8/12 16:51, Richard Biener wrote:

On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:


Hi Richard,
Thanks for your comments, updated the v2 patch as below:
1. Define and use builtin_with_linkage_p.
2. Add comments.
3. Add a testcase.

In LTO mode, if static library and dynamic library contains same
function and both libraries are passed as arguments, linker will link
the function in dynamic library no matter the sequence.  This patch
will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
is a math function, then the function in static library will be linked
first if its sequence is ahead of the dynamic library.


Comments below


gcc/ChangeLog

 2019-08-12  Xiong Hu Luo  

 PR lto/91287
 * builtins.c (builtin_with_linkage_p): New function.
 * builtins.h (builtin_with_linkage_p): New function.
 * symtab.c (write_symbol): Use builtin_with_linkage_p.
 * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
 Likewise.

gcc/testsuite/ChangeLog

 2019-08-12  Xiong Hu Luo  

 PR lto/91287
 * gcc.dg/pr91287.c: New testcase.
---
  gcc/builtins.c | 89 ++
  gcc/builtins.h |  2 +
  gcc/lto-streamer-out.c |  4 +-
  gcc/symtab.c   | 13 -
  gcc/testsuite/gcc.dg/pr91287.c | 40 +++
  5 files changed, 145 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr91287.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 695a9d191af..f4dea941a27 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
*p = (char)tree_to_uhwi (t);
return true;
  }
+
+/* Return true if DECL is a specified builtin math function.  These functions
+   should have symbol in symbol table to provide linkage with faster version of
+   libraries.  */


The comment should read like

/* Return true if the builtin DECL is implemented in a standard
library.  Otherwise
returns false which doesn't guarantee it is not (thus the list of
handled builtins
below may be incomplete).  */


+bool
+builtin_with_linkage_p (tree decl)
+{
+  if (!decl)
+return false;


Omit this check please.


+  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
+switch (DECL_FUNCTION_CODE (decl))
+{
+  CASE_FLT_FN (BUILT_IN_ACOS):
+  CASE_FLT_FN (BUILT_IN_ACOSH):
+  CASE_FLT_FN (BUILT_IN_ASIN):
+  CASE_FLT_FN (BUILT_IN_ASINH):
+  CASE_FLT_FN (BUILT_IN_ATAN):
+  CASE_FLT_FN (BUILT_IN_ATANH):
+  CASE_FLT_FN (BUILT_IN_ATAN2):
+  CASE_FLT_FN (BUILT_IN_CBRT):
+  CASE_FLT_FN (BUILT_IN_CEIL):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
+  CASE_FLT_FN (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN (BUILT_IN_COS):
+  CASE_FLT_FN (BUILT_IN_COSH):
+  CASE_FLT_FN (BUILT_IN_ERF):
+  CASE_FLT_FN (BUILT_IN_ERFC):
+  CASE_FLT_FN (BUILT_IN_EXP):
+  CASE_FLT_FN (BUILT_IN_EXP2):
+  CASE_FLT_FN (BUILT_IN_EXPM1):
+  CASE_FLT_FN (BUILT_IN_FABS):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
+  CASE_FLT_FN (BUILT_IN_FDIM):
+  CASE_FLT_FN (BUILT_IN_FLOOR):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
+  CASE_FLT_FN (BUILT_IN_FMA):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
+  CASE_FLT_FN (BUILT_IN_FMAX):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
+  CASE_FLT_FN (BUILT_IN_FMIN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
+  CASE_FLT_FN (BUILT_IN_FMOD):
+  CASE_FLT_FN (BUILT_IN_FREXP):
+  CASE_FLT_FN (BUILT_IN_HYPOT):
+  CASE_FLT_FN (BUILT_IN_ILOGB):
+  CASE_FLT_FN (BUILT_IN_LDEXP):
+  CASE_FLT_FN (BUILT_IN_LGAMMA):
+  CASE_FLT_FN (BUILT_IN_LLRINT):
+  CASE_FLT_FN (BUILT_IN_LLROUND):
+  CASE_FLT_FN (BUILT_IN_LOG):
+  CASE_FLT_FN (BUILT_IN_LOG10):
+  CASE_FLT_FN (BUILT_IN_LOG1P):
+  CASE_FLT_FN (BUILT_IN_LOG2):
+  CASE_FLT_FN (BUILT_IN_LOGB):
+  CASE_FLT_FN (BUILT_IN_LRINT):
+  CASE_FLT_FN (BUILT_IN_LROUND):
+  CASE_FLT_FN (BUILT_IN_MODF):
+  CASE_FLT_FN (BUILT_IN_NAN):
+  CASE_FLT_FN (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
+  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
+  CASE_FLT_FN (BUILT_IN_POW):
+  CASE_FLT_FN (BUILT_IN_REMAINDER):
+  CASE_FLT_FN (BUILT_IN_REMQUO):
+  CASE_FLT_FN (BUILT_IN_RINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
+  CASE_FLT_FN (BUILT_IN_ROUND):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
+  CASE_FLT_FN (BUILT_IN_SCALBLN):
+  CASE_FLT_FN (BUILT_IN_SCALBN):
+  CASE_FLT_FN (BUILT_IN_SIN):
+  CASE_FLT_FN (BUILT_IN_SINH):
+  CASE_FLT_FN (BUILT_IN_SINCOS):
+  CASE_FLT_FN (BUILT_IN_SQRT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_SQRT):
+  CASE_FLT_FN (BUILT_IN_TAN):
+  CASE_FLT_FN (BUILT_IN_TANH):
+  CASE_FLT_FN 

Re: [Patch v2] Enable math functions linking with static library for LTO

2019-08-12 Thread Richard Biener
On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:
>
> Hi Richard,
> Thanks for your comments, updated the v2 patch as below:
> 1. Define and use builtin_with_linkage_p.
> 2. Add comments.
> 3. Add a testcase.
>
> In LTO mode, if static library and dynamic library contains same
> function and both libraries are passed as arguments, linker will link
> the function in dynamic library no matter the sequence.  This patch
> will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
> is a math function, then the function in static library will be linked
> first if its sequence is ahead of the dynamic library.

Comments below

> gcc/ChangeLog
>
> 2019-08-12  Xiong Hu Luo  
>
> PR lto/91287
> * builtins.c (builtin_with_linkage_p): New function.
> * builtins.h (builtin_with_linkage_p): New function.
> * symtab.c (write_symbol): Use builtin_with_linkage_p.
> * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
> Likewise.
>
> gcc/testsuite/ChangeLog
>
> 2019-08-12  Xiong Hu Luo  
>
> PR lto/91287
> * gcc.dg/pr91287.c: New testcase.
> ---
>  gcc/builtins.c | 89 ++
>  gcc/builtins.h |  2 +
>  gcc/lto-streamer-out.c |  4 +-
>  gcc/symtab.c   | 13 -
>  gcc/testsuite/gcc.dg/pr91287.c | 40 +++
>  5 files changed, 145 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr91287.c
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 695a9d191af..f4dea941a27 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
>*p = (char)tree_to_uhwi (t);
>return true;
>  }
> +
> +/* Return true if DECL is a specified builtin math function.  These functions
> +   should have symbol in symbol table to provide linkage with faster version 
> of
> +   libraries.  */

The comment should read like

/* Return true if the builtin DECL is implemented in a standard
library.  Otherwise
   returns false which doesn't guarantee it is not (thus the list of
handled builtins
   below may be incomplete).  */

> +bool
> +builtin_with_linkage_p (tree decl)
> +{
> +  if (!decl)
> +return false;

Omit this check please.

> +  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
> +switch (DECL_FUNCTION_CODE (decl))
> +{
> +  CASE_FLT_FN (BUILT_IN_ACOS):
> +  CASE_FLT_FN (BUILT_IN_ACOSH):
> +  CASE_FLT_FN (BUILT_IN_ASIN):
> +  CASE_FLT_FN (BUILT_IN_ASINH):
> +  CASE_FLT_FN (BUILT_IN_ATAN):
> +  CASE_FLT_FN (BUILT_IN_ATANH):
> +  CASE_FLT_FN (BUILT_IN_ATAN2):
> +  CASE_FLT_FN (BUILT_IN_CBRT):
> +  CASE_FLT_FN (BUILT_IN_CEIL):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
> +  CASE_FLT_FN (BUILT_IN_COPYSIGN):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
> +  CASE_FLT_FN (BUILT_IN_COS):
> +  CASE_FLT_FN (BUILT_IN_COSH):
> +  CASE_FLT_FN (BUILT_IN_ERF):
> +  CASE_FLT_FN (BUILT_IN_ERFC):
> +  CASE_FLT_FN (BUILT_IN_EXP):
> +  CASE_FLT_FN (BUILT_IN_EXP2):
> +  CASE_FLT_FN (BUILT_IN_EXPM1):
> +  CASE_FLT_FN (BUILT_IN_FABS):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
> +  CASE_FLT_FN (BUILT_IN_FDIM):
> +  CASE_FLT_FN (BUILT_IN_FLOOR):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
> +  CASE_FLT_FN (BUILT_IN_FMA):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
> +  CASE_FLT_FN (BUILT_IN_FMAX):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
> +  CASE_FLT_FN (BUILT_IN_FMIN):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
> +  CASE_FLT_FN (BUILT_IN_FMOD):
> +  CASE_FLT_FN (BUILT_IN_FREXP):
> +  CASE_FLT_FN (BUILT_IN_HYPOT):
> +  CASE_FLT_FN (BUILT_IN_ILOGB):
> +  CASE_FLT_FN (BUILT_IN_LDEXP):
> +  CASE_FLT_FN (BUILT_IN_LGAMMA):
> +  CASE_FLT_FN (BUILT_IN_LLRINT):
> +  CASE_FLT_FN (BUILT_IN_LLROUND):
> +  CASE_FLT_FN (BUILT_IN_LOG):
> +  CASE_FLT_FN (BUILT_IN_LOG10):
> +  CASE_FLT_FN (BUILT_IN_LOG1P):
> +  CASE_FLT_FN (BUILT_IN_LOG2):
> +  CASE_FLT_FN (BUILT_IN_LOGB):
> +  CASE_FLT_FN (BUILT_IN_LRINT):
> +  CASE_FLT_FN (BUILT_IN_LROUND):
> +  CASE_FLT_FN (BUILT_IN_MODF):
> +  CASE_FLT_FN (BUILT_IN_NAN):
> +  CASE_FLT_FN (BUILT_IN_NEARBYINT):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
> +  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
> +  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
> +  CASE_FLT_FN (BUILT_IN_POW):
> +  CASE_FLT_FN (BUILT_IN_REMAINDER):
> +  CASE_FLT_FN (BUILT_IN_REMQUO):
> +  CASE_FLT_FN (BUILT_IN_RINT):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
> +  CASE_FLT_FN (BUILT_IN_ROUND):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
> +  CASE_FLT_FN (BUILT_IN_SCALBLN):
> +  CASE_FLT_FN (BUILT_IN_SCALBN):
> +  CASE_FLT_FN (BUILT_IN_SIN):
> +  CASE_FLT_FN (BUILT_IN_SINH):
> +  CASE_FLT_FN (BUILT_IN_SINCOS):
> +  CASE_FLT_FN 

[Patch v2] Enable math functions linking with static library for LTO

2019-08-12 Thread luoxhu
Hi Richard,
Thanks for your comments, updated the v2 patch as below:
1. Define and use builtin_with_linkage_p.
2. Add comments.
3. Add a testcase.

In LTO mode, if static library and dynamic library contains same
function and both libraries are passed as arguments, linker will link
the function in dynamic library no matter the sequence.  This patch
will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
is a math function, then the function in static library will be linked
first if its sequence is ahead of the dynamic library.

gcc/ChangeLog

2019-08-12  Xiong Hu Luo  

PR lto/91287
* builtins.c (builtin_with_linkage_p): New function.
* builtins.h (builtin_with_linkage_p): New function.
* symtab.c (write_symbol): Use builtin_with_linkage_p.
* lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
Likewise.

gcc/testsuite/ChangeLog

2019-08-12  Xiong Hu Luo  

PR lto/91287
* gcc.dg/pr91287.c: New testcase.
---
 gcc/builtins.c | 89 ++
 gcc/builtins.h |  2 +
 gcc/lto-streamer-out.c |  4 +-
 gcc/symtab.c   | 13 -
 gcc/testsuite/gcc.dg/pr91287.c | 40 +++
 5 files changed, 145 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr91287.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 695a9d191af..f4dea941a27 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
   *p = (char)tree_to_uhwi (t);
   return true;
 }
+
+/* Return true if DECL is a specified builtin math function.  These functions
+   should have symbol in symbol table to provide linkage with faster version of
+   libraries.  */
+
+bool
+builtin_with_linkage_p (tree decl)
+{
+  if (!decl)
+return false;
+  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
+switch (DECL_FUNCTION_CODE (decl))
+{
+  CASE_FLT_FN (BUILT_IN_ACOS):
+  CASE_FLT_FN (BUILT_IN_ACOSH):
+  CASE_FLT_FN (BUILT_IN_ASIN):
+  CASE_FLT_FN (BUILT_IN_ASINH):
+  CASE_FLT_FN (BUILT_IN_ATAN):
+  CASE_FLT_FN (BUILT_IN_ATANH):
+  CASE_FLT_FN (BUILT_IN_ATAN2):
+  CASE_FLT_FN (BUILT_IN_CBRT):
+  CASE_FLT_FN (BUILT_IN_CEIL):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
+  CASE_FLT_FN (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN (BUILT_IN_COS):
+  CASE_FLT_FN (BUILT_IN_COSH):
+  CASE_FLT_FN (BUILT_IN_ERF):
+  CASE_FLT_FN (BUILT_IN_ERFC):
+  CASE_FLT_FN (BUILT_IN_EXP):
+  CASE_FLT_FN (BUILT_IN_EXP2):
+  CASE_FLT_FN (BUILT_IN_EXPM1):
+  CASE_FLT_FN (BUILT_IN_FABS):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
+  CASE_FLT_FN (BUILT_IN_FDIM):
+  CASE_FLT_FN (BUILT_IN_FLOOR):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
+  CASE_FLT_FN (BUILT_IN_FMA):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
+  CASE_FLT_FN (BUILT_IN_FMAX):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
+  CASE_FLT_FN (BUILT_IN_FMIN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
+  CASE_FLT_FN (BUILT_IN_FMOD):
+  CASE_FLT_FN (BUILT_IN_FREXP):
+  CASE_FLT_FN (BUILT_IN_HYPOT):
+  CASE_FLT_FN (BUILT_IN_ILOGB):
+  CASE_FLT_FN (BUILT_IN_LDEXP):
+  CASE_FLT_FN (BUILT_IN_LGAMMA):
+  CASE_FLT_FN (BUILT_IN_LLRINT):
+  CASE_FLT_FN (BUILT_IN_LLROUND):
+  CASE_FLT_FN (BUILT_IN_LOG):
+  CASE_FLT_FN (BUILT_IN_LOG10):
+  CASE_FLT_FN (BUILT_IN_LOG1P):
+  CASE_FLT_FN (BUILT_IN_LOG2):
+  CASE_FLT_FN (BUILT_IN_LOGB):
+  CASE_FLT_FN (BUILT_IN_LRINT):
+  CASE_FLT_FN (BUILT_IN_LROUND):
+  CASE_FLT_FN (BUILT_IN_MODF):
+  CASE_FLT_FN (BUILT_IN_NAN):
+  CASE_FLT_FN (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
+  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
+  CASE_FLT_FN (BUILT_IN_POW):
+  CASE_FLT_FN (BUILT_IN_REMAINDER):
+  CASE_FLT_FN (BUILT_IN_REMQUO):
+  CASE_FLT_FN (BUILT_IN_RINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
+  CASE_FLT_FN (BUILT_IN_ROUND):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
+  CASE_FLT_FN (BUILT_IN_SCALBLN):
+  CASE_FLT_FN (BUILT_IN_SCALBN):
+  CASE_FLT_FN (BUILT_IN_SIN):
+  CASE_FLT_FN (BUILT_IN_SINH):
+  CASE_FLT_FN (BUILT_IN_SINCOS):
+  CASE_FLT_FN (BUILT_IN_SQRT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_SQRT):
+  CASE_FLT_FN (BUILT_IN_TAN):
+  CASE_FLT_FN (BUILT_IN_TANH):
+  CASE_FLT_FN (BUILT_IN_TGAMMA):
+  CASE_FLT_FN (BUILT_IN_TRUNC):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_TRUNC):
+   return true;
+  default:
+   break;
+}
+  return false;
+}
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 1ffb491d785..91cbd81be48 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -151,4 +151,6 @@ extern internal_fn replacement_internal_fn (gcall *);
 extern void warn_string_no_nul