Re: Inline across -ffast-math boundary

[Rainer Orth]

Richard Biener  writes:

>> >> This new testcase does not pass on bare-metal configs (using newlib),
>> >> because:
>> >> warning: implicit declaration of function 'isnanf'
>> >> [-Wimplicit-function-declaration]
>> >> warning: incompatible implicit declaration of built-in function 'isnanf'
>> >>
>> >> I'm not sure what's the appropriate dg-require to avoid this?
>> >
>> > c99_runtime I guess.
>> >
>> Indeed, that what was used in a previous occurrence of a similar
>> problem (PR 69227).
>> 
>> I've attached the small (obvious?) patch to make the new inline-8.c
>> test UNSUPPORTED
>> without c99_runtime.
>> 
>> OK?
>
> Ok.

The testcase still FAILs on Solaris:

FAIL: gcc.dg/ipa/inline-8.c (test for excess errors)
Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/ipa/inline-8.c:12:10: warning: 
implicit declaration of function 'isnanf' [-Wimplicit-function-declaration]

The isnanf(3C) manpage claims that isnanf() is declared in 
(which is true), but the prototype is also in  under
__EXTENSIONS__ && !_XOPEN_SOURCE.

The function is not in C99 or XPG7, and instead of trying to do the
equivalent of AC_USE_SYSTEM_EXTENSIONS, I've followed the lead of
gcc.target/arm/pr59896.c and just declare it in the testcase.

Tested with the appropriate runtest invocations on i386-pc-solaris2.12
and x86_64-pc-linux-gnu, installed on mainline.

Rainer


2016-05-04  Rainer Orth  

* gcc.dg/ipa/inline-8.c (isnanf): Declare.

# HG changeset patch
# Parent  7654b342bb270f12f16abf76b78e235c2798832f
Declare isnanf in gcc.dg/ipa/inline-8.c

diff --git a/gcc/testsuite/gcc.dg/ipa/inline-8.c b/gcc/testsuite/gcc.dg/ipa/inline-8.c
--- a/gcc/testsuite/gcc.dg/ipa/inline-8.c
+++ b/gcc/testsuite/gcc.dg/ipa/inline-8.c
@@ -5,6 +5,7 @@
 /* { dg-options "-O2"  } */
 /* { dg-add-options c99_runtime } */
 #include 
+extern int isnanf (float);
 /* Can't be inlined because isnanf will be optimized out.  */
 int
 cmp (float a)

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Inline across -ffast-math boundary

[Aldy Hernandez]

On 05/03/2016 10:20 AM, Richard Biener wrote:

On Tue, 3 May 2016, Jan Hubicka wrote:


On 21/04/16 12:45, Jan Hubicka wrote:

this patch implements the long promised logic to inline across -ffast-math
boundary when eitehr caller or callee has no fp operations in it.  This is
needed to resolve code quality regression on Firefox with LTO where
-O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
otherwise.


-Ofast turns allow-store-data-races param on.
are such params handled by lto?


No, allow-store-data-races is ignored by LTO option handling and whatever is
used at final linktime wins.  THis is because it is --param parameter rather
than optimization option.

Given the nature of the option, I wonder why it was implemented as --param
thing?  It looks like typical optimization option to me and this way we lose
mechanizm to set it with function granuality like we can do other optimization
options.


History.  It was requested multiple times to turn it into a regular
option.


I seem to recall there was a discussion between Ian Taylor and Andrew 
Macleod whether it should be a --param or an option.  Obviously in my 
mind the --param argument won, though I can't remember who made that 
case or what the argument was.


Aldy

Re: Inline across -ffast-math boundary

[Richard Biener]

On Tue, 3 May 2016, Jan Hubicka wrote:

> > On 21/04/16 12:45, Jan Hubicka wrote:
> > > this patch implements the long promised logic to inline across -ffast-math
> > > boundary when eitehr caller or callee has no fp operations in it.  This is
> > > needed to resolve code quality regression on Firefox with LTO where
> > > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> > > otherwise.
> > 
> > -Ofast turns allow-store-data-races param on.
> > are such params handled by lto?
> 
> No, allow-store-data-races is ignored by LTO option handling and whatever is
> used at final linktime wins.  THis is because it is --param parameter rather
> than optimization option.
> 
> Given the nature of the option, I wonder why it was implemented as --param
> thing?  It looks like typical optimization option to me and this way we lose
> mechanizm to set it with function granuality like we can do other optimization
> options.

History.  It was requested multiple times to turn it into a regular
option.

Richard.

Re: Inline across -ffast-math boundary

[Richard Biener]

On Tue, 3 May 2016, Szabolcs Nagy wrote:

> On 21/04/16 12:45, Jan Hubicka wrote:
> > this patch implements the long promised logic to inline across -ffast-math
> > boundary when eitehr caller or callee has no fp operations in it.  This is
> > needed to resolve code quality regression on Firefox with LTO where
> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> > otherwise.
> 
> -Ofast turns allow-store-data-races param on.
> are such params handled by lto?

I don't think so.  We don't have per-function param values.  It
handles -Ofast itself though.

Richard.

Re: Inline across -ffast-math boundary

[Jan Hubicka]

> On 21/04/16 12:45, Jan Hubicka wrote:
> > this patch implements the long promised logic to inline across -ffast-math
> > boundary when eitehr caller or callee has no fp operations in it.  This is
> > needed to resolve code quality regression on Firefox with LTO where
> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> > otherwise.
> 
> -Ofast turns allow-store-data-races param on.
> are such params handled by lto?

No, allow-store-data-races is ignored by LTO option handling and whatever is
used at final linktime wins.  THis is because it is --param parameter rather
than optimization option.

Given the nature of the option, I wonder why it was implemented as --param
thing?  It looks like typical optimization option to me and this way we lose
mechanizm to set it with function granuality like we can do other optimization
options.
Honza

Re: Inline across -ffast-math boundary

[Szabolcs Nagy]

On 21/04/16 12:45, Jan Hubicka wrote:
> this patch implements the long promised logic to inline across -ffast-math
> boundary when eitehr caller or callee has no fp operations in it.  This is
> needed to resolve code quality regression on Firefox with LTO where
> -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> otherwise.

-Ofast turns allow-store-data-races param on.
are such params handled by lto?

Re: Inline across -ffast-math boundary

[Richard Biener]

On Tue, 3 May 2016, Christophe Lyon wrote:

> On 3 May 2016 at 10:09, Richard Biener <rguent...@suse.de> wrote:
> > On Tue, 3 May 2016, Christophe Lyon wrote:
> >
> >> On 2 May 2016 at 19:01, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> >> On Thu, 21 Apr 2016, Jan Hubicka wrote:
> >> >>
> >> >> > Hi,
> >> >> > this patch implements the long promised logic to inline across 
> >> >> > -ffast-math
> >> >> > boundary when eitehr caller or callee has no fp operations in it.  
> >> >> > This is
> >> >> > needed to resolve code quality regression on Firefox with LTO where
> >> >> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of 
> >> >> > comdats
> >> >> > otherwise.
> >> >> >
> >> >> > Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know 
> >> >> > your opinion
> >> >> > on fp_expression_p predicate - it is bit ugly but I do not know how 
> >> >> > to implement
> >> >> > it better.
> >> >> >
> >> >> > We still won't inline -O1 code into -O2+ because flag_strict_overflow 
> >> >> > differs.
> >> >> > I will implement similar logic for overflows incrementally. Similarly 
> >> >> > flag_errno_math
> >> >> > can be handled better, but I am not sure it matters - I think wast 
> >> >> > majority of time
> >> >> > users set errno_math in sync with other -ffast-math implied flags.
> >> >>
> >> >> Note that for reasons PR70586 shows (const functions having possible
> >> >> trapping side-effect because of FP math or division) we'd like to
> >> >> have sth like "uses FP math" "uses possibly trapping integer math"
> >> >> "uses integer math with undefined overflow" on a per function level
> >> >> and propagated alongside pure/const/nothrow state.
> >> >>
> >> >> So maybe you can fit that into a more suitable place than just the
> >> >> inliner (which of course is interested in "uses FP math locally",
> >> >> not the transitive answer we need for PR70586).
> >> >
> >> > We don't really have much more suitable place - ipa-inline-analysis is
> >> > doing most of the analysis of function body that is usefull for IPA 
> >> > passes,
> >> > not only for inliner. It should be renamed perhaps to something like
> >> > function_body_summary.  I will do that later this stage1.
> >> >
> >> > For PR70686 in addition to transitive answer we will need to know that
> >> > the transformation is win. Const function may take a lot of time and
> >> > introducing new call on code path that did not used it previously is
> >> > bad idea unless we know that the function is very cheap (which may
> >> > be true only for fast builtins, I don't know)
> >> >
> >> > This patch impleemnts the suggested check for presence of FP parameters.
> >> > We can play with special casing the moves incrementally.
> >> >
> >> > Bootstrapped/regtested x86_64-linux.
> >> >
> >> > Index: testsuite/ChangeLog
> >> > ===
> >> > --- testsuite/ChangeLog (revision 235765)
> >> > +++ testsuite/ChangeLog (working copy)
> >> > @@ -1,3 +1,7 @@
> >> > +2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
> >> > +
> >> > +   * gcc.dg/ipa/inline-8.c: New testcase.
> >> > +
> >> >  2016-05-02  Jakub Jelinek  <ja...@redhat.com>
> >> >
> >> > PR rtl-optimization/70467
> >> > Index: testsuite/gcc.dg/ipa/inline-8.c
> >> > ===
> >> > --- testsuite/gcc.dg/ipa/inline-8.c (revision 0)
> >> > +++ testsuite/gcc.dg/ipa/inline-8.c (working copy)
> >> > @@ -0,0 +1,36 @@
> >> > +/* Verify that we do not inline isnanf test info -ffast-math code but 
> >> > that we
> >> > +   do inline trivial functions across -Ofast boundary.  */
> >> > +/* { dg-do run } */
> >> > +/* { dg-options "-O2"  } */
> >> > +#include 
> >> > +/* Can't be inlined because isnanf will b

Re: Inline across -ffast-math boundary

[Christophe Lyon]

On 3 May 2016 at 10:09, Richard Biener <rguent...@suse.de> wrote:
> On Tue, 3 May 2016, Christophe Lyon wrote:
>
>> On 2 May 2016 at 19:01, Jan Hubicka <hubi...@ucw.cz> wrote:
>> >> On Thu, 21 Apr 2016, Jan Hubicka wrote:
>> >>
>> >> > Hi,
>> >> > this patch implements the long promised logic to inline across 
>> >> > -ffast-math
>> >> > boundary when eitehr caller or callee has no fp operations in it.  This 
>> >> > is
>> >> > needed to resolve code quality regression on Firefox with LTO where
>> >> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
>> >> > otherwise.
>> >> >
>> >> > Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your 
>> >> > opinion
>> >> > on fp_expression_p predicate - it is bit ugly but I do not know how to 
>> >> > implement
>> >> > it better.
>> >> >
>> >> > We still won't inline -O1 code into -O2+ because flag_strict_overflow 
>> >> > differs.
>> >> > I will implement similar logic for overflows incrementally. Similarly 
>> >> > flag_errno_math
>> >> > can be handled better, but I am not sure it matters - I think wast 
>> >> > majority of time
>> >> > users set errno_math in sync with other -ffast-math implied flags.
>> >>
>> >> Note that for reasons PR70586 shows (const functions having possible
>> >> trapping side-effect because of FP math or division) we'd like to
>> >> have sth like "uses FP math" "uses possibly trapping integer math"
>> >> "uses integer math with undefined overflow" on a per function level
>> >> and propagated alongside pure/const/nothrow state.
>> >>
>> >> So maybe you can fit that into a more suitable place than just the
>> >> inliner (which of course is interested in "uses FP math locally",
>> >> not the transitive answer we need for PR70586).
>> >
>> > We don't really have much more suitable place - ipa-inline-analysis is
>> > doing most of the analysis of function body that is usefull for IPA passes,
>> > not only for inliner. It should be renamed perhaps to something like
>> > function_body_summary.  I will do that later this stage1.
>> >
>> > For PR70686 in addition to transitive answer we will need to know that
>> > the transformation is win. Const function may take a lot of time and
>> > introducing new call on code path that did not used it previously is
>> > bad idea unless we know that the function is very cheap (which may
>> > be true only for fast builtins, I don't know)
>> >
>> > This patch impleemnts the suggested check for presence of FP parameters.
>> > We can play with special casing the moves incrementally.
>> >
>> > Bootstrapped/regtested x86_64-linux.
>> >
>> > Index: testsuite/ChangeLog
>> > ===
>> > --- testsuite/ChangeLog (revision 235765)
>> > +++ testsuite/ChangeLog (working copy)
>> > @@ -1,3 +1,7 @@
>> > +2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
>> > +
>> > +   * gcc.dg/ipa/inline-8.c: New testcase.
>> > +
>> >  2016-05-02  Jakub Jelinek  <ja...@redhat.com>
>> >
>> > PR rtl-optimization/70467
>> > Index: testsuite/gcc.dg/ipa/inline-8.c
>> > ===
>> > --- testsuite/gcc.dg/ipa/inline-8.c (revision 0)
>> > +++ testsuite/gcc.dg/ipa/inline-8.c (working copy)
>> > @@ -0,0 +1,36 @@
>> > +/* Verify that we do not inline isnanf test info -ffast-math code but 
>> > that we
>> > +   do inline trivial functions across -Ofast boundary.  */
>> > +/* { dg-do run } */
>> > +/* { dg-options "-O2"  } */
>> > +#include 
>> > +/* Can't be inlined because isnanf will be optimized out.  */
>> > +int
>> > +cmp (float a)
>> > +{
>> > +  return isnanf (a);
>>
>> Hi,
>>
>> This new testcase does not pass on bare-metal configs (using newlib), 
>> because:
>> warning: implicit declaration of function 'isnanf'
>> [-Wimplicit-function-declaration]
>> warning: incompatible implicit declaration of built-in function 'isnanf'
>>
>> I'm not sure what's the appropriate

Re: Inline across -ffast-math boundary

[Richard Biener]

On Tue, 3 May 2016, Christophe Lyon wrote:

> On 2 May 2016 at 19:01, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> On Thu, 21 Apr 2016, Jan Hubicka wrote:
> >>
> >> > Hi,
> >> > this patch implements the long promised logic to inline across 
> >> > -ffast-math
> >> > boundary when eitehr caller or callee has no fp operations in it.  This 
> >> > is
> >> > needed to resolve code quality regression on Firefox with LTO where
> >> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> >> > otherwise.
> >> >
> >> > Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your 
> >> > opinion
> >> > on fp_expression_p predicate - it is bit ugly but I do not know how to 
> >> > implement
> >> > it better.
> >> >
> >> > We still won't inline -O1 code into -O2+ because flag_strict_overflow 
> >> > differs.
> >> > I will implement similar logic for overflows incrementally. Similarly 
> >> > flag_errno_math
> >> > can be handled better, but I am not sure it matters - I think wast 
> >> > majority of time
> >> > users set errno_math in sync with other -ffast-math implied flags.
> >>
> >> Note that for reasons PR70586 shows (const functions having possible
> >> trapping side-effect because of FP math or division) we'd like to
> >> have sth like "uses FP math" "uses possibly trapping integer math"
> >> "uses integer math with undefined overflow" on a per function level
> >> and propagated alongside pure/const/nothrow state.
> >>
> >> So maybe you can fit that into a more suitable place than just the
> >> inliner (which of course is interested in "uses FP math locally",
> >> not the transitive answer we need for PR70586).
> >
> > We don't really have much more suitable place - ipa-inline-analysis is
> > doing most of the analysis of function body that is usefull for IPA passes,
> > not only for inliner. It should be renamed perhaps to something like
> > function_body_summary.  I will do that later this stage1.
> >
> > For PR70686 in addition to transitive answer we will need to know that
> > the transformation is win. Const function may take a lot of time and
> > introducing new call on code path that did not used it previously is
> > bad idea unless we know that the function is very cheap (which may
> > be true only for fast builtins, I don't know)
> >
> > This patch impleemnts the suggested check for presence of FP parameters.
> > We can play with special casing the moves incrementally.
> >
> > Bootstrapped/regtested x86_64-linux.
> >
> > Index: testsuite/ChangeLog
> > ===
> > --- testsuite/ChangeLog (revision 235765)
> > +++ testsuite/ChangeLog (working copy)
> > @@ -1,3 +1,7 @@
> > +2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
> > +
> > +   * gcc.dg/ipa/inline-8.c: New testcase.
> > +
> >  2016-05-02  Jakub Jelinek  <ja...@redhat.com>
> >
> > PR rtl-optimization/70467
> > Index: testsuite/gcc.dg/ipa/inline-8.c
> > ===
> > --- testsuite/gcc.dg/ipa/inline-8.c (revision 0)
> > +++ testsuite/gcc.dg/ipa/inline-8.c (working copy)
> > @@ -0,0 +1,36 @@
> > +/* Verify that we do not inline isnanf test info -ffast-math code but that 
> > we
> > +   do inline trivial functions across -Ofast boundary.  */
> > +/* { dg-do run } */
> > +/* { dg-options "-O2"  } */
> > +#include 
> > +/* Can't be inlined because isnanf will be optimized out.  */
> > +int
> > +cmp (float a)
> > +{
> > +  return isnanf (a);
> 
> Hi,
> 
> This new testcase does not pass on bare-metal configs (using newlib), because:
> warning: implicit declaration of function 'isnanf'
> [-Wimplicit-function-declaration]
> warning: incompatible implicit declaration of built-in function 'isnanf'
> 
> I'm not sure what's the appropriate dg-require to avoid this?

c99_runtime I guess.

Richard.

> Christophe
> 
> > +}
> > +/* Can be inlined.  */
> > +int
> > +move (int a)
> > +{
> > +  return a;
> > +}
> > +float a;
> > +void
> > +set ()
> > +{
> > + a=nan("");
> > +}
> > +float b;
> > +__attribute__ ((optimize("Ofast")))
> > +int
> > +main()
> &

Re: Inline across -ffast-math boundary

[Christophe Lyon]

On 2 May 2016 at 19:01, Jan Hubicka <hubi...@ucw.cz> wrote:
>> On Thu, 21 Apr 2016, Jan Hubicka wrote:
>>
>> > Hi,
>> > this patch implements the long promised logic to inline across -ffast-math
>> > boundary when eitehr caller or callee has no fp operations in it.  This is
>> > needed to resolve code quality regression on Firefox with LTO where
>> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
>> > otherwise.
>> >
>> > Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your 
>> > opinion
>> > on fp_expression_p predicate - it is bit ugly but I do not know how to 
>> > implement
>> > it better.
>> >
>> > We still won't inline -O1 code into -O2+ because flag_strict_overflow 
>> > differs.
>> > I will implement similar logic for overflows incrementally. Similarly 
>> > flag_errno_math
>> > can be handled better, but I am not sure it matters - I think wast 
>> > majority of time
>> > users set errno_math in sync with other -ffast-math implied flags.
>>
>> Note that for reasons PR70586 shows (const functions having possible
>> trapping side-effect because of FP math or division) we'd like to
>> have sth like "uses FP math" "uses possibly trapping integer math"
>> "uses integer math with undefined overflow" on a per function level
>> and propagated alongside pure/const/nothrow state.
>>
>> So maybe you can fit that into a more suitable place than just the
>> inliner (which of course is interested in "uses FP math locally",
>> not the transitive answer we need for PR70586).
>
> We don't really have much more suitable place - ipa-inline-analysis is
> doing most of the analysis of function body that is usefull for IPA passes,
> not only for inliner. It should be renamed perhaps to something like
> function_body_summary.  I will do that later this stage1.
>
> For PR70686 in addition to transitive answer we will need to know that
> the transformation is win. Const function may take a lot of time and
> introducing new call on code path that did not used it previously is
> bad idea unless we know that the function is very cheap (which may
> be true only for fast builtins, I don't know)
>
> This patch impleemnts the suggested check for presence of FP parameters.
> We can play with special casing the moves incrementally.
>
> Bootstrapped/regtested x86_64-linux.
>
> Index: testsuite/ChangeLog
> ===
> --- testsuite/ChangeLog (revision 235765)
> +++ testsuite/ChangeLog (working copy)
> @@ -1,3 +1,7 @@
> +2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
> +
> +   * gcc.dg/ipa/inline-8.c: New testcase.
> +
>  2016-05-02  Jakub Jelinek  <ja...@redhat.com>
>
> PR rtl-optimization/70467
> Index: testsuite/gcc.dg/ipa/inline-8.c
> ===
> --- testsuite/gcc.dg/ipa/inline-8.c (revision 0)
> +++ testsuite/gcc.dg/ipa/inline-8.c (working copy)
> @@ -0,0 +1,36 @@
> +/* Verify that we do not inline isnanf test info -ffast-math code but that we
> +   do inline trivial functions across -Ofast boundary.  */
> +/* { dg-do run } */
> +/* { dg-options "-O2"  } */
> +#include 
> +/* Can't be inlined because isnanf will be optimized out.  */
> +int
> +cmp (float a)
> +{
> +  return isnanf (a);

Hi,

This new testcase does not pass on bare-metal configs (using newlib), because:
warning: implicit declaration of function 'isnanf'
[-Wimplicit-function-declaration]
warning: incompatible implicit declaration of built-in function 'isnanf'

I'm not sure what's the appropriate dg-require to avoid this?

Christophe

> +}
> +/* Can be inlined.  */
> +int
> +move (int a)
> +{
> +  return a;
> +}
> +float a;
> +void
> +set ()
> +{
> + a=nan("");
> +}
> +float b;
> +__attribute__ ((optimize("Ofast")))
> +int
> +main()
> +{
> +  b++;
> +  if (cmp(a))
> +__builtin_abort ();
> +  float a = move (1);
> +  if (!__builtin_constant_p (a))
> +__builtin_abort ();
> +  return 0;
> +}
> Index: ChangeLog
> ===
> --- ChangeLog   (revision 235765)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,18 @@
> +2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
> +
> +   * ipa-inline-analysis.c (reset_inline_summary): Clear fp_expressions
> +   (dump_inline_summary): Dump it.
> +   (fp_expression_p): New predicate.
> +   (estimate_function_bo

Re: Inline across -ffast-math boundary

[Jan Hubicka]

> On Mon, May 02, 2016 at 07:01:38PM +0200, Jan Hubicka wrote:
> > This patch impleemnts the suggested check for presence of FP parameters.
> > We can play with special casing the moves incrementally.
> 
> This patch has broken the build:
> 
> /home/marek/src/gcc/gcc/ipa-inline.c: In function ‘bool
> can_inline_edge_p(cgraph_edge*, bool, bool, bool)’:
> /home/marek/src/gcc/gcc/ipa-inline.c:341:55: error: ‘CIF_THUNK’ was not
> declared in this scope
>  e->inline_failed = e->caller->thunk.thunk_p ? CIF_THUNK :
> CIF_MISMATCHED_ARGUMENTS;

Sorry, I managed to miss independent changes that was in my tree.  I will revert
the evaluate_conditions_for_known_args and add CIF_THUNK shortly.

Honza
>^
> Makefile:1085: recipe for target 'ipa-inline.o' failed
> make: *** [ipa-inline.o] Error 1
> make: *** Waiting for unfinished jobs
> /home/marek/src/gcc/gcc/ipa-inline-analysis.c: In function ‘clause_t
> evaluate_conditions_for_known_args(cgraph_node*, bool, vec,
> vec)’:
> /home/marek/src/gcc/gcc/ipa-inline-analysis.c:854:27: error: invalid 
> conversion
> from ‘tree_node*’ to ‘long int’ [-fpermissive]
>c->offset, c->by_ref);
>^
> /home/marek/src/gcc/gcc/ipa-inline-analysis.c:854:27: error: too many 
> arguments
> to function ‘tree_node* ipa_find_agg_cst_for_param(ipa_agg_jump_function*, 
> long
> int, bool)’
> In file included from /home/marek/src/gcc/gcc/ipa-inline-analysis.c:90:0:
> /home/marek/src/gcc/gcc/ipa-prop.h:639:6: note: declared here
>  tree ipa_find_agg_cst_for_param (struct ipa_agg_jump_function *,
> HOST_WIDE_INT,
>   ^
> Makefile:1085: recipe for target 'ipa-inline-analysis.o' failed
> make: *** [ipa-inline-analysis.o] Error 1
> 
>   Marek

Re: Inline across -ffast-math boundary

[Marek Polacek]

On Mon, May 02, 2016 at 07:01:38PM +0200, Jan Hubicka wrote:
> This patch impleemnts the suggested check for presence of FP parameters.
> We can play with special casing the moves incrementally.

This patch has broken the build:

/home/marek/src/gcc/gcc/ipa-inline.c: In function ‘bool
can_inline_edge_p(cgraph_edge*, bool, bool, bool)’:
/home/marek/src/gcc/gcc/ipa-inline.c:341:55: error: ‘CIF_THUNK’ was not
declared in this scope
 e->inline_failed = e->caller->thunk.thunk_p ? CIF_THUNK :
CIF_MISMATCHED_ARGUMENTS;
   ^
Makefile:1085: recipe for target 'ipa-inline.o' failed
make: *** [ipa-inline.o] Error 1
make: *** Waiting for unfinished jobs
/home/marek/src/gcc/gcc/ipa-inline-analysis.c: In function ‘clause_t
evaluate_conditions_for_known_args(cgraph_node*, bool, vec,
vec)’:
/home/marek/src/gcc/gcc/ipa-inline-analysis.c:854:27: error: invalid conversion
from ‘tree_node*’ to ‘long int’ [-fpermissive]
   c->offset, c->by_ref);
   ^
/home/marek/src/gcc/gcc/ipa-inline-analysis.c:854:27: error: too many arguments
to function ‘tree_node* ipa_find_agg_cst_for_param(ipa_agg_jump_function*, long
int, bool)’
In file included from /home/marek/src/gcc/gcc/ipa-inline-analysis.c:90:0:
/home/marek/src/gcc/gcc/ipa-prop.h:639:6: note: declared here
 tree ipa_find_agg_cst_for_param (struct ipa_agg_jump_function *,
HOST_WIDE_INT,
  ^
Makefile:1085: recipe for target 'ipa-inline-analysis.o' failed
make: *** [ipa-inline-analysis.o] Error 1

Marek

Re: Inline across -ffast-math boundary

[Jan Hubicka]

> On Thu, 21 Apr 2016, Jan Hubicka wrote:
> 
> > Hi,
> > this patch implements the long promised logic to inline across -ffast-math
> > boundary when eitehr caller or callee has no fp operations in it.  This is
> > needed to resolve code quality regression on Firefox with LTO where
> > -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> > otherwise.
> > 
> > Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your 
> > opinion
> > on fp_expression_p predicate - it is bit ugly but I do not know how to 
> > implement
> > it better.
> > 
> > We still won't inline -O1 code into -O2+ because flag_strict_overflow 
> > differs.
> > I will implement similar logic for overflows incrementally. Similarly 
> > flag_errno_math
> > can be handled better, but I am not sure it matters - I think wast majority 
> > of time
> > users set errno_math in sync with other -ffast-math implied flags.
> 
> Note that for reasons PR70586 shows (const functions having possible
> trapping side-effect because of FP math or division) we'd like to
> have sth like "uses FP math" "uses possibly trapping integer math"
> "uses integer math with undefined overflow" on a per function level
> and propagated alongside pure/const/nothrow state.
> 
> So maybe you can fit that into a more suitable place than just the
> inliner (which of course is interested in "uses FP math locally",
> not the transitive answer we need for PR70586).

We don't really have much more suitable place - ipa-inline-analysis is 
doing most of the analysis of function body that is usefull for IPA passes,
not only for inliner. It should be renamed perhaps to something like
function_body_summary.  I will do that later this stage1.

For PR70686 in addition to transitive answer we will need to know that
the transformation is win. Const function may take a lot of time and
introducing new call on code path that did not used it previously is
bad idea unless we know that the function is very cheap (which may
be true only for fast builtins, I don't know)

This patch impleemnts the suggested check for presence of FP parameters.
We can play with special casing the moves incrementally.

Bootstrapped/regtested x86_64-linux.

Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 235765)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
+
+   * gcc.dg/ipa/inline-8.c: New testcase.
+
 2016-05-02  Jakub Jelinek  <ja...@redhat.com>
 
PR rtl-optimization/70467
Index: testsuite/gcc.dg/ipa/inline-8.c
===
--- testsuite/gcc.dg/ipa/inline-8.c (revision 0)
+++ testsuite/gcc.dg/ipa/inline-8.c (working copy)
@@ -0,0 +1,36 @@
+/* Verify that we do not inline isnanf test info -ffast-math code but that we
+   do inline trivial functions across -Ofast boundary.  */
+/* { dg-do run } */
+/* { dg-options "-O2"  } */
+#include 
+/* Can't be inlined because isnanf will be optimized out.  */
+int
+cmp (float a)
+{
+  return isnanf (a);
+}
+/* Can be inlined.  */
+int
+move (int a)
+{
+  return a;
+}
+float a;
+void
+set ()
+{
+ a=nan("");
+}
+float b;
+__attribute__ ((optimize("Ofast")))
+int
+main()
+{
+  b++;
+  if (cmp(a))
+__builtin_abort ();
+  float a = move (1);
+  if (!__builtin_constant_p (a))
+__builtin_abort ();
+  return 0;
+}
Index: ChangeLog
===
--- ChangeLog   (revision 235765)
+++ ChangeLog   (working copy)
@@ -1,3 +1,18 @@
+2016-05-02  Jan Hubicka  <hubi...@ucw.cz>
+
+   * ipa-inline-analysis.c (reset_inline_summary): Clear fp_expressions
+   (dump_inline_summary): Dump it.
+   (fp_expression_p): New predicate.
+   (estimate_function_body_sizes): Use it.
+   (inline_merge_summary): Merge fp_expressions.
+   (inline_read_section): Read fp_expressions.
+   (inline_write_summary): Write fp_expressions.
+   * ipa-inline.c (can_inline_edge_p): Permit inlining across fp math
+   codegen boundary if either caller or callee is !fp_expressions.
+   * ipa-inline.h (inline_summary): Add fp_expressions.
+   * ipa-inline-transform.c (inline_call): When inlining !fp_expressions
+   to fp_expressions be sure the fp generation flags are updated.
+
 2016-05-02  Jakub Jelinek  <ja...@redhat.com>
 
PR rtl-optimization/70467
Index: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c   (revision 235693)
+++ ipa-inline-analysis.c   (working copy)
@@ -850,7 +850,8 @@ evaluate_conditions_for_known_args (stru
  

Re: Inline across -ffast-math boundary

[Richard Biener]

On Thu, 21 Apr 2016, Jan Hubicka wrote:

> Hi,
> this patch implements the long promised logic to inline across -ffast-math
> boundary when eitehr caller or callee has no fp operations in it.  This is
> needed to resolve code quality regression on Firefox with LTO where
> -O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
> otherwise.
> 
> Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your 
> opinion
> on fp_expression_p predicate - it is bit ugly but I do not know how to 
> implement
> it better.
> 
> We still won't inline -O1 code into -O2+ because flag_strict_overflow differs.
> I will implement similar logic for overflows incrementally. Similarly 
> flag_errno_math
> can be handled better, but I am not sure it matters - I think wast majority 
> of time
> users set errno_math in sync with other -ffast-math implied flags.

Note that for reasons PR70586 shows (const functions having possible
trapping side-effect because of FP math or division) we'd like to
have sth like "uses FP math" "uses possibly trapping integer math"
"uses integer math with undefined overflow" on a per function level
and propagated alongside pure/const/nothrow state.

So maybe you can fit that into a more suitable place than just the
inliner (which of course is interested in "uses FP math locally",
not the transitive answer we need for PR70586).

More comments below.

> Honza
> 
> 
>   * ipa-inline-analysis.c (reset_inline_summary): Clear fp_expressions
>   (dump_inline_summary): Dump it.
>   (fp_expression_p): New predicate.
>   (estimate_function_body_sizes): Use it.
>   (inline_merge_summary): Merge fp_expressions.
>   (inline_read_section): Read fp_expressions.
>   (inline_write_summary): Write fp_expressions.
>   * ipa-inline.c (can_inline_edge_p): Permit inlining across fp math
>   codegen boundary if either caller or callee is !fp_expressions.
>   * ipa-inline.h (inline_summary): Add fp_expressions.
>   * ipa-inline-transform.c (inline_call): When inlining !fp_expressions
>   to fp_expressions be sure the fp generation flags are updated.
> 
>   * gcc.dg/ipa/inline-8.c: New testcase.
> Index: ipa-inline-analysis.c
> ===
> --- ipa-inline-analysis.c (revision 235312)
> +++ ipa-inline-analysis.c (working copy)
> @@ -1069,6 +1069,7 @@ reset_inline_summary (struct cgraph_node
>  reset_inline_edge_summary (e);
>for (e = node->indirect_calls; e; e = e->next_callee)
>  reset_inline_edge_summary (e);
> +  info->fp_expressions = false;
>  }
>  
>  /* Hook that is called by cgraph.c when a node is removed.  */
> @@ -1423,6 +1424,8 @@ dump_inline_summary (FILE *f, struct cgr
>   fprintf (f, " inlinable");
>if (s->contains_cilk_spawn)
>   fprintf (f, " contains_cilk_spawn");
> +  if (s->fp_expressions)
> + fprintf (f, " fp_expression");
>fprintf (f, "\n  self time:   %i\n", s->self_time);
>fprintf (f, "  global time: %i\n", s->time);
>fprintf (f, "  self size:   %i\n", s->self_size);
> @@ -2459,6 +2462,42 @@ clobber_only_eh_bb_p (basic_block bb, bo
>return true;
>  }
>  
> +/* Return true if STMT compute a floating point expression that may be 
> affected
> +   by -ffast-math and similar flags.  */
> +
> +static bool
> +fp_expression_p (gimple *stmt)
> +{
> +  tree fndecl;
> +
> +  if (gimple_code (stmt) == GIMPLE_ASSIGN
> +  /* Even conversion to and from float are FP expressions.  */
> +  && (FLOAT_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
> +   || FLOAT_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt
> +  /* Plain moves are safe.  */
> +  && (IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (gimple_assign_rhs_code 
> (stmt)))
> +   || TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_comparison))
> +return true;
> +
> +  /* Comparsions may be optimized with assumption that value is not NaN.  */
> +  if (gimple_code (stmt) == GIMPLE_COND
> +  && (FLOAT_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt)))
> +   || FLOAT_TYPE_P (TREE_TYPE (gimple_cond_rhs (stmt)
> +return true;
> +
> +  /* Builtins may be optimized depending on math mode.  We don't really have
> + list of these, so just check that there are no FP arguments.  */
> +  if (gimple_code (stmt) == GIMPLE_CALL
> +  && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
> +  && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
> +{
> +  for (unsigned int i=0; 

Inline across -ffast-math boundary

[Jan Hubicka]

Hi,
this patch implements the long promised logic to inline across -ffast-math
boundary when eitehr caller or callee has no fp operations in it.  This is
needed to resolve code quality regression on Firefox with LTO where
-O3/-O2/-Ofast flags are combined and we fail to inline a lot of comdats
otherwise.

Bootstrapped/regtested x86_64-linux. Ricahrd, I would like to know your opinion
on fp_expression_p predicate - it is bit ugly but I do not know how to implement
it better.

We still won't inline -O1 code into -O2+ because flag_strict_overflow differs.
I will implement similar logic for overflows incrementally. Similarly 
flag_errno_math
can be handled better, but I am not sure it matters - I think wast majority of 
time
users set errno_math in sync with other -ffast-math implied flags.

Honza


* ipa-inline-analysis.c (reset_inline_summary): Clear fp_expressions
(dump_inline_summary): Dump it.
(fp_expression_p): New predicate.
(estimate_function_body_sizes): Use it.
(inline_merge_summary): Merge fp_expressions.
(inline_read_section): Read fp_expressions.
(inline_write_summary): Write fp_expressions.
* ipa-inline.c (can_inline_edge_p): Permit inlining across fp math
codegen boundary if either caller or callee is !fp_expressions.
* ipa-inline.h (inline_summary): Add fp_expressions.
* ipa-inline-transform.c (inline_call): When inlining !fp_expressions
to fp_expressions be sure the fp generation flags are updated.

* gcc.dg/ipa/inline-8.c: New testcase.
Index: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c   (revision 235312)
+++ ipa-inline-analysis.c   (working copy)
@@ -1069,6 +1069,7 @@ reset_inline_summary (struct cgraph_node
 reset_inline_edge_summary (e);
   for (e = node->indirect_calls; e; e = e->next_callee)
 reset_inline_edge_summary (e);
+  info->fp_expressions = false;
 }
 
 /* Hook that is called by cgraph.c when a node is removed.  */
@@ -1423,6 +1424,8 @@ dump_inline_summary (FILE *f, struct cgr
fprintf (f, " inlinable");
   if (s->contains_cilk_spawn)
fprintf (f, " contains_cilk_spawn");
+  if (s->fp_expressions)
+   fprintf (f, " fp_expression");
   fprintf (f, "\n  self time:   %i\n", s->self_time);
   fprintf (f, "  global time: %i\n", s->time);
   fprintf (f, "  self size:   %i\n", s->self_size);
@@ -2459,6 +2462,42 @@ clobber_only_eh_bb_p (basic_block bb, bo
   return true;
 }
 
+/* Return true if STMT compute a floating point expression that may be affected
+   by -ffast-math and similar flags.  */
+
+static bool
+fp_expression_p (gimple *stmt)
+{
+  tree fndecl;
+
+  if (gimple_code (stmt) == GIMPLE_ASSIGN
+  /* Even conversion to and from float are FP expressions.  */
+  && (FLOAT_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ || FLOAT_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt
+  /* Plain moves are safe.  */
+  && (IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)))
+ || TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_comparison))
+return true;
+
+  /* Comparsions may be optimized with assumption that value is not NaN.  */
+  if (gimple_code (stmt) == GIMPLE_COND
+  && (FLOAT_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt)))
+ || FLOAT_TYPE_P (TREE_TYPE (gimple_cond_rhs (stmt)
+return true;
+
+  /* Builtins may be optimized depending on math mode.  We don't really have
+ list of these, so just check that there are no FP arguments.  */
+  if (gimple_code (stmt) == GIMPLE_CALL
+  && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
+  && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
+{
+  for (unsigned int i=0; i < gimple_call_num_args (stmt); i++)
+   if (FLOAT_TYPE_P (TREE_TYPE (gimple_call_arg (stmt, i
+ return true;
+}
+  return false;
+}
+
 /* Compute function body size parameters for NODE.
When EARLY is true, we compute only simple summaries without
non-trivial predicates to drive the early inliner.  */
@@ -2733,6 +2772,13 @@ estimate_function_body_sizes (struct cgr
   this_time * (2 - prob), );
}
 
+ if (!info->fp_expressions && fp_expression_p (stmt))
+   {
+ info->fp_expressions = true;
+ if (dump_file)
+   fprintf (dump_file, "   fp_expression set\n");
+   }
+
  gcc_assert (time >= 0);
  gcc_assert (size >= 0);
}
@@ -3577,6 +3623,8 @@ inline_merge_summary (struct cgraph_edge
   else
 toplev_predicate = true_predicate ();
 
+  info->fp_expressions |= callee_info->fp_expressions;
+