Ping^3 [PATCH V2] Add attribute hot judgement for INLINE_HINT_known_hot hint.

Message ID SJ0PR11MB5600A9CDD52DBEC97E81257D9E2D9@SJ0PR11MB5600.namprd11.prod.outlook.com
State Accepted
Headers
Series Ping^3 [PATCH V2] Add attribute hot judgement for INLINE_HINT_known_hot hint. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Li, Pan2 via Gcc-patches Oct. 21, 2022, 1:52 a.m. UTC
  Hi Honza,

Gentle ping  https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601934.html

gcc/ChangeLog

  * ipa-inline-analysis.cc (do_estimate_edge_time): Add function attribute
  judgement for INLINE_HINT_known_hot hint.

gcc/testsuite/ChangeLog:

  * gcc.dg/ipa/inlinehint-6.c: New test.
---
 gcc/ipa-inline-analysis.cc              | 13 ++++---
 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c | 47 +++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
  

Comments

Jeff Law Oct. 29, 2022, 4:28 a.m. UTC | #1
On 10/20/22 19:52, Cui, Lili via Gcc-patches wrote:
> Hi Honza,
>
> Gentle ping  https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601934.html
>
> gcc/ChangeLog
>
>    * ipa-inline-analysis.cc (do_estimate_edge_time): Add function attribute
>    judgement for INLINE_HINT_known_hot hint.
>
> gcc/testsuite/ChangeLog:
>
>    * gcc.dg/ipa/inlinehint-6.c: New test.
> ---
>   gcc/ipa-inline-analysis.cc              | 13 ++++---
>   gcc/testsuite/gcc.dg/ipa/inlinehint-6.c | 47 +++++++++++++++++++++++++
>   2 files changed, 56 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
>
> diff --git a/gcc/ipa-inline-analysis.cc b/gcc/ipa-inline-analysis.cc
> index 1ca685d1b0e..7bd29c36590 100644
> --- a/gcc/ipa-inline-analysis.cc
> +++ b/gcc/ipa-inline-analysis.cc
> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "ipa-utils.h"
>   #include "cfgexpand.h"
>   #include "gimplify.h"
> +#include "attribs.h"
>   
>   /* Cached node/edge growths.  */
>   fast_call_summary<edge_growth_cache_entry *, va_heap> *edge_growth_cache = NULL;
> @@ -249,15 +250,19 @@ do_estimate_edge_time (struct cgraph_edge *edge, sreal *ret_nonspec_time)
>         hints = estimates.hints;
>       }
>   
> -  /* When we have profile feedback, we can quite safely identify hot
> -     edges and for those we disable size limits.  Don't do that when
> -     probability that caller will call the callee is low however, since it
> +  /* When we have profile feedback or function attribute, we can quite safely
> +     identify hot edges and for those we disable size limits.  Don't do that
> +     when probability that caller will call the callee is low however, since it
>        may hurt optimization of the caller's hot path.  */
> -  if (edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
> +  if ((edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
>         && (edge->count.ipa () * 2
>   	  > (edge->caller->inlined_to
>   	     ? edge->caller->inlined_to->count.ipa ()
>   	     : edge->caller->count.ipa ())))
> +      || (lookup_attribute ("hot", DECL_ATTRIBUTES (edge->caller->decl))
> +	  != NULL
> +	 && lookup_attribute ("hot", DECL_ATTRIBUTES (edge->callee->decl))
> +	  != NULL))
>       hints |= INLINE_HINT_known_hot;

Is the theory here that if the user has marked the caller and callee as 
hot, then we're going to assume an edge between them is hot too?  That's 
not necessarily true, it could be they're both hot, but via other call 
chains.  But it's probably a reasonable heuristic in practice.


OK


jeff
  
Li, Pan2 via Gcc-patches Oct. 31, 2022, 1:44 a.m. UTC | #2
> 
> On 10/20/22 19:52, Cui, Lili via Gcc-patches wrote:
> > Hi Honza,
> >
> > Gentle ping
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601934.html
> >
> > gcc/ChangeLog
> >
> >    * ipa-inline-analysis.cc (do_estimate_edge_time): Add function attribute
> >    judgement for INLINE_HINT_known_hot hint.
> >
> > gcc/testsuite/ChangeLog:
> >
> >    * gcc.dg/ipa/inlinehint-6.c: New test.
> > ---
> >   gcc/ipa-inline-analysis.cc              | 13 ++++---
> >   gcc/testsuite/gcc.dg/ipa/inlinehint-6.c | 47
> +++++++++++++++++++++++++
> >   2 files changed, 56 insertions(+), 4 deletions(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
> >
> > diff --git a/gcc/ipa-inline-analysis.cc b/gcc/ipa-inline-analysis.cc
> > index 1ca685d1b0e..7bd29c36590 100644
> > --- a/gcc/ipa-inline-analysis.cc
> > +++ b/gcc/ipa-inline-analysis.cc
> > @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "ipa-utils.h"
> >   #include "cfgexpand.h"
> >   #include "gimplify.h"
> > +#include "attribs.h"
> >
> >   /* Cached node/edge growths.  */
> >   fast_call_summary<edge_growth_cache_entry *, va_heap>
> > *edge_growth_cache = NULL; @@ -249,15 +250,19 @@
> do_estimate_edge_time (struct cgraph_edge *edge, sreal
> *ret_nonspec_time)
> >         hints = estimates.hints;
> >       }
> >
> > -  /* When we have profile feedback, we can quite safely identify hot
> > -     edges and for those we disable size limits.  Don't do that when
> > -     probability that caller will call the callee is low however, since it
> > +  /* When we have profile feedback or function attribute, we can quite
> safely
> > +     identify hot edges and for those we disable size limits.  Don't do that
> > +     when probability that caller will call the callee is low
> > + however, since it
> >        may hurt optimization of the caller's hot path.  */
> > -  if (edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
> > +  if ((edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
> >         && (edge->count.ipa () * 2
> >   	  > (edge->caller->inlined_to
> >   	     ? edge->caller->inlined_to->count.ipa ()
> >   	     : edge->caller->count.ipa ())))
> > +      || (lookup_attribute ("hot", DECL_ATTRIBUTES (edge->caller->decl))
> > +	  != NULL
> > +	 && lookup_attribute ("hot", DECL_ATTRIBUTES (edge->callee->decl))
> > +	  != NULL))
> >       hints |= INLINE_HINT_known_hot;
> 
> Is the theory here that if the user has marked the caller and callee as hot,
> then we're going to assume an edge between them is hot too?  That's not
> necessarily true, it could be they're both hot, but via other call chains.  But it's
> probably a reasonable heuristic in practice.
> 
Yes,  thanks Jeff.

Lili.
> 
> OK
> 
> 
> jeff
>
  
Jeff Law Oct. 31, 2022, 4:35 p.m. UTC | #3
On 10/30/22 19:44, Cui, Lili wrote:
>> On 10/20/22 19:52, Cui, Lili via Gcc-patches wrote:
>>> Hi Honza,
>>>
>>> Gentle ping
>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601934.html
>>>
>>> gcc/ChangeLog
>>>
>>>     * ipa-inline-analysis.cc (do_estimate_edge_time): Add function attribute
>>>     judgement for INLINE_HINT_known_hot hint.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.dg/ipa/inlinehint-6.c: New test.
>>> ---
>>>    gcc/ipa-inline-analysis.cc              | 13 ++++---
>>>    gcc/testsuite/gcc.dg/ipa/inlinehint-6.c | 47
>> +++++++++++++++++++++++++
>>>    2 files changed, 56 insertions(+), 4 deletions(-)
>>>    create mode 100644 gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
>>>
>>> diff --git a/gcc/ipa-inline-analysis.cc b/gcc/ipa-inline-analysis.cc
>>> index 1ca685d1b0e..7bd29c36590 100644
>>> --- a/gcc/ipa-inline-analysis.cc
>>> +++ b/gcc/ipa-inline-analysis.cc
>>> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
>>>    #include "ipa-utils.h"
>>>    #include "cfgexpand.h"
>>>    #include "gimplify.h"
>>> +#include "attribs.h"
>>>
>>>    /* Cached node/edge growths.  */
>>>    fast_call_summary<edge_growth_cache_entry *, va_heap>
>>> *edge_growth_cache = NULL; @@ -249,15 +250,19 @@
>> do_estimate_edge_time (struct cgraph_edge *edge, sreal
>> *ret_nonspec_time)
>>>          hints = estimates.hints;
>>>        }
>>>
>>> -  /* When we have profile feedback, we can quite safely identify hot
>>> -     edges and for those we disable size limits.  Don't do that when
>>> -     probability that caller will call the callee is low however, since it
>>> +  /* When we have profile feedback or function attribute, we can quite
>> safely
>>> +     identify hot edges and for those we disable size limits.  Don't do that
>>> +     when probability that caller will call the callee is low
>>> + however, since it
>>>         may hurt optimization of the caller's hot path.  */
>>> -  if (edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
>>> +  if ((edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
>>>          && (edge->count.ipa () * 2
>>>    	  > (edge->caller->inlined_to
>>>    	     ? edge->caller->inlined_to->count.ipa ()
>>>    	     : edge->caller->count.ipa ())))
>>> +      || (lookup_attribute ("hot", DECL_ATTRIBUTES (edge->caller->decl))
>>> +	  != NULL
>>> +	 && lookup_attribute ("hot", DECL_ATTRIBUTES (edge->callee->decl))
>>> +	  != NULL))
>>>        hints |= INLINE_HINT_known_hot;
>> Is the theory here that if the user has marked the caller and callee as hot,
>> then we're going to assume an edge between them is hot too?  That's not
>> necessarily true, it could be they're both hot, but via other call chains.  But it's
>> probably a reasonable heuristic in practice.
>>
> Yes,  thanks Jeff.

Thanks for the confirmation.  This is OK for the trunk.

jeff
  

Patch

diff --git a/gcc/ipa-inline-analysis.cc b/gcc/ipa-inline-analysis.cc
index 1ca685d1b0e..7bd29c36590 100644
--- a/gcc/ipa-inline-analysis.cc
+++ b/gcc/ipa-inline-analysis.cc
@@ -48,6 +48,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "ipa-utils.h"
 #include "cfgexpand.h"
 #include "gimplify.h"
+#include "attribs.h"
 
 /* Cached node/edge growths.  */
 fast_call_summary<edge_growth_cache_entry *, va_heap> *edge_growth_cache = NULL;
@@ -249,15 +250,19 @@  do_estimate_edge_time (struct cgraph_edge *edge, sreal *ret_nonspec_time)
       hints = estimates.hints;
     }
 
-  /* When we have profile feedback, we can quite safely identify hot
-     edges and for those we disable size limits.  Don't do that when
-     probability that caller will call the callee is low however, since it
+  /* When we have profile feedback or function attribute, we can quite safely
+     identify hot edges and for those we disable size limits.  Don't do that
+     when probability that caller will call the callee is low however, since it
      may hurt optimization of the caller's hot path.  */
-  if (edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
+  if ((edge->count.ipa ().initialized_p () && edge->maybe_hot_p ()
       && (edge->count.ipa () * 2
 	  > (edge->caller->inlined_to
 	     ? edge->caller->inlined_to->count.ipa ()
 	     : edge->caller->count.ipa ())))
+      || (lookup_attribute ("hot", DECL_ATTRIBUTES (edge->caller->decl))
+	  != NULL
+	 && lookup_attribute ("hot", DECL_ATTRIBUTES (edge->callee->decl))
+	  != NULL))
     hints |= INLINE_HINT_known_hot;
 
   gcc_checking_assert (size >= 0);
diff --git a/gcc/testsuite/gcc.dg/ipa/inlinehint-6.c b/gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
new file mode 100644
index 00000000000..1f3be641c6d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/inlinehint-6.c
@@ -0,0 +1,47 @@ 
+/* { dg-options "-O3 -c -fdump-ipa-inline-details -fno-early-inlining -fno-ipa-cp"  } */
+/* { dg-add-options bind_pic_locally } */
+
+#define size_t long long int
+
+struct A
+{
+  size_t f1, f2, f3, f4;
+};
+struct C
+{
+  struct A a;
+  size_t b;
+};
+struct C x;
+
+__attribute__((hot)) struct C callee (struct A *a, struct C *c)
+{
+  c->a=(*a);
+
+  if((c->b + 7) & 17)
+   {
+      c->a.f1 = c->a.f2 + c->a.f1;
+      c->a.f2 = c->a.f3 - c->a.f2;
+      c->a.f3 = c->a.f2 + c->a.f3;
+      c->a.f4 = c->a.f2 - c->a.f4;
+      c->b = c->a.f2;
+
+    }
+  return *c;
+}
+
+__attribute__((hot)) struct C caller (size_t d, size_t e, size_t f, size_t g, struct C *c)
+{
+  struct A a;
+  a.f1 = 1 + d;
+  a.f2 = e;
+  a.f3 = 12 + f;
+  a.f4 = 68 + g;
+  if (c->b > 0)
+    return callee (&a, c);
+  else
+    return *c;
+}
+
+/* { dg-final { scan-ipa-dump "known_hot"  "inline"  } } */
+