public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] inline: improve internal function costs
@ 2023-06-01 16:30 Andre Vieira (lists)
  2023-06-02  9:13 ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Andre Vieira (lists) @ 2023-06-01 16:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Biener, Richard Sandiford, jakub

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

Hi,

This is a follow-up of the internal function patch to add widening and 
narrowing patterns.  This patch improves the inliner cost estimation for 
internal functions.

Bootstrapped and regression tested on aarch64-unknown-linux-gnu.

gcc/ChangeLog:

         * ipa-fnsummary.cc (analyze_function_body): Correctly handle
         non-zero costed internal functions.
         * tree-inline.cc (estimate_num_insns): Improve costing for internal
         functions.

[-- Attachment #2: internal_fn_inline.patch --]
[-- Type: text/plain, Size: 2757 bytes --]

diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc
index b328bb8ce14b0725f6e5607da9d1e2f61e9baf62..449961fe44e4d86bf61e625dff0759d58e1e80ba 100644
--- a/gcc/ipa-fnsummary.cc
+++ b/gcc/ipa-fnsummary.cc
@@ -2862,16 +2862,19 @@ analyze_function_body (struct cgraph_node *node, bool early)
 		 to happen, but we cannot do that for call statements
 		 because edges are accounted specially.  */
 
-	      if (*(is_gimple_call (stmt) ? &bb_predicate : &p) != false)
+	      if (*(is_gimple_call (stmt) && !gimple_call_internal_p (stmt)
+		    ? &bb_predicate : &p) != false)
 		{
 		  time += final_time;
 		  size += this_size;
 		}
 
 	      /* We account everything but the calls.  Calls have their own
-	         size/time info attached to cgraph edges.  This is necessary
-	         in order to make the cost disappear after inlining.  */
-	      if (!is_gimple_call (stmt))
+		 size/time info attached to cgraph edges.  This is necessary
+		 in order to make the cost disappear after inlining.  The only
+		 exceptions are internal calls.  */
+	      if (!is_gimple_call (stmt)
+		  || gimple_call_internal_p (stmt))
 		{
 		  if (prob)
 		    {
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 99efddc36c8906a797583a569424336e961c35d1..bac84d277254703369c27993dcad048de8d4ff70 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -4427,7 +4427,48 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
 	tree decl;
 
 	if (gimple_call_internal_p (stmt))
-	  return 0;
+	  {
+	    switch (gimple_call_internal_fn (stmt))
+	      {
+	      default:
+		return 1;
+
+	      case IFN_GOMP_TARGET_REV:
+	      case IFN_GOMP_USE_SIMT:
+	      case IFN_GOMP_SIMT_ENTER_ALLOC:
+	      case IFN_GOMP_SIMT_EXIT:
+	      case IFN_GOMP_SIMT_LANE:
+	      case IFN_GOMP_SIMT_VF:
+	      case IFN_GOMP_SIMT_LAST_LANE:
+	      case IFN_GOMP_SIMT_ORDERED_PRED:
+	      case IFN_GOMP_SIMT_VOTE_ANY:
+	      case IFN_GOMP_SIMT_XCHG_BFLY:
+	      case IFN_GOMP_SIMT_XCHG_IDX:
+	      case IFN_GOMP_SIMD_LANE:
+	      case IFN_GOMP_SIMD_VF:
+	      case IFN_GOMP_SIMD_LAST_LANE:
+	      case IFN_GOMP_SIMD_ORDERED_START:
+	      case IFN_GOMP_SIMD_ORDERED_END:
+	      case IFN_BUILTIN_EXPECT:
+	      case IFN_ANNOTATE:
+	      case IFN_NOP:
+	      case IFN_UNIQUE:
+	      case IFN_DEFERRED_INIT:
+	      case IFN_ASSUME:
+		return 0;
+
+	      case IFN_UBSAN_NULL:
+	      case IFN_UBSAN_BOUNDS:
+	      case IFN_UBSAN_VPTR:
+	      case IFN_UBSAN_CHECK_ADD:
+	      case IFN_UBSAN_CHECK_SUB:
+	      case IFN_UBSAN_CHECK_MUL:
+	      case IFN_UBSAN_PTR:
+	      case IFN_UBSAN_OBJECT_SIZE:
+		/* Estimating a compare and jump.  */
+		return 2;
+	      }
+	  }
 	else if ((decl = gimple_call_fndecl (stmt))
 		 && fndecl_built_in_p (decl))
 	  {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] inline: improve internal function costs
  2023-06-01 16:30 [PATCH] inline: improve internal function costs Andre Vieira (lists)
@ 2023-06-02  9:13 ` Richard Biener
  2023-06-02  9:44   ` Andre Vieira (lists)
  2023-06-05  3:04   ` Jan Hubicka
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Biener @ 2023-06-02  9:13 UTC (permalink / raw)
  To: Andre Vieira (lists); +Cc: gcc-patches, Richard Sandiford, jakub, Jan Hubicka

On Thu, 1 Jun 2023, Andre Vieira (lists) wrote:

> Hi,
> 
> This is a follow-up of the internal function patch to add widening and
> narrowing patterns.  This patch improves the inliner cost estimation for
> internal functions.

I have no idea why calls are special in IPA analyze_function_body
and so I cannot say whether treating all internal fn calls as
non-calls is correct there.  Honza?

The tree-inline.cc change is OK though (you can push that separately).

Thanks,
Richard.

> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
> 
> gcc/ChangeLog:
> 
>         * ipa-fnsummary.cc (analyze_function_body): Correctly handle
>         non-zero costed internal functions.
>         * tree-inline.cc (estimate_num_insns): Improve costing for internal
>         functions.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] inline: improve internal function costs
  2023-06-02  9:13 ` Richard Biener
@ 2023-06-02  9:44   ` Andre Vieira (lists)
  2023-06-05  3:04   ` Jan Hubicka
  1 sibling, 0 replies; 6+ messages in thread
From: Andre Vieira (lists) @ 2023-06-02  9:44 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Richard Sandiford, jakub, Jan Hubicka



On 02/06/2023 10:13, Richard Biener wrote:
> On Thu, 1 Jun 2023, Andre Vieira (lists) wrote:
> 
>> Hi,
>>
>> This is a follow-up of the internal function patch to add widening and
>> narrowing patterns.  This patch improves the inliner cost estimation for
>> internal functions.
> 
> I have no idea why calls are special in IPA analyze_function_body
> and so I cannot say whether treating all internal fn calls as
> non-calls is correct there.  Honza?
> 
> The tree-inline.cc change is OK though (you can push that separately).
I can't though, it ICEs on libgcc compilation (and other tests in 
testsuite). The estimate function is used by IPA to compute size and 
without the changes there it hits an assert because the 
estimate_num_insns no longer matches what ipa records in its 
size_time_table.

I'll wait for Honza to comment.
> 
> Thanks,
> Richard.
> 
>> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
>>
>> gcc/ChangeLog:
>>
>>          * ipa-fnsummary.cc (analyze_function_body): Correctly handle
>>          non-zero costed internal functions.
>>          * tree-inline.cc (estimate_num_insns): Improve costing for internal
>>          functions.
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] inline: improve internal function costs
  2023-06-02  9:13 ` Richard Biener
  2023-06-02  9:44   ` Andre Vieira (lists)
@ 2023-06-05  3:04   ` Jan Hubicka
  2023-06-12 12:38     ` Andre Vieira (lists)
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2023-06-05  3:04 UTC (permalink / raw)
  To: Richard Biener
  Cc: Andre Vieira (lists), gcc-patches, Richard Sandiford, jakub

> On Thu, 1 Jun 2023, Andre Vieira (lists) wrote:
> 
> > Hi,
> > 
> > This is a follow-up of the internal function patch to add widening and
> > narrowing patterns.  This patch improves the inliner cost estimation for
> > internal functions.
> 
> I have no idea why calls are special in IPA analyze_function_body
> and so I cannot say whether treating all internal fn calls as
> non-calls is correct there.  Honza?

The reason is that normal statements are acconted as part of the
function body, while calls have their costs attached to call edges
(so it can be adjusted when call is inlined to otherwise optimized).

However since internal functions have no cgraph edges, this looks like
a bug that we do not test it.  (the code was written before internal
calls was introduced).

I wonder if we don't want to have is_noninternal_gimple_call that could
be used by IPA code to test whether cgraph edge should exist for
the statement.
> 
> The tree-inline.cc change is OK though (you can push that separately).
The rest is OK too.
Honza
> 
> Thanks,
> Richard.
> 
> > Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
> > 
> > gcc/ChangeLog:
> > 
> >         * ipa-fnsummary.cc (analyze_function_body): Correctly handle
> >         non-zero costed internal functions.
> >         * tree-inline.cc (estimate_num_insns): Improve costing for internal
> >         functions.
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] inline: improve internal function costs
  2023-06-05  3:04   ` Jan Hubicka
@ 2023-06-12 12:38     ` Andre Vieira (lists)
  2023-06-12 13:01       ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Andre Vieira (lists) @ 2023-06-12 12:38 UTC (permalink / raw)
  To: Jan Hubicka, Richard Biener, Richard Sandiford; +Cc: gcc-patches, jakub



On 05/06/2023 04:04, Jan Hubicka wrote:
>> On Thu, 1 Jun 2023, Andre Vieira (lists) wrote:
>>
>>> Hi,
>>>
>>> This is a follow-up of the internal function patch to add widening and
>>> narrowing patterns.  This patch improves the inliner cost estimation for
>>> internal functions.
>>
>> I have no idea why calls are special in IPA analyze_function_body
>> and so I cannot say whether treating all internal fn calls as
>> non-calls is correct there.  Honza?
> 
> The reason is that normal statements are acconted as part of the
> function body, while calls have their costs attached to call edges
> (so it can be adjusted when call is inlined to otherwise optimized).
> 
> However since internal functions have no cgraph edges, this looks like
> a bug that we do not test it.  (the code was written before internal
> calls was introduced).
>

This sounds to me like you agree with my approach to treat internal 
calls different to regular calls.

> I wonder if we don't want to have is_noninternal_gimple_call that could
> be used by IPA code to test whether cgraph edge should exist for
> the statement.

I'm happy to add such a helper function @richi,rsandifo: you ok with that?
>>
>> The tree-inline.cc change is OK though (you can push that separately).
> The rest is OK too.
> Honza
>>
>> Thanks,
>> Richard.
>>
>>> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
>>>
>>> gcc/ChangeLog:
>>>
>>>          * ipa-fnsummary.cc (analyze_function_body): Correctly handle
>>>          non-zero costed internal functions.
>>>          * tree-inline.cc (estimate_num_insns): Improve costing for internal
>>>          functions.
>>>
>>
>> -- 
>> Richard Biener <rguenther@suse.de>
>> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
>> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
>> HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] inline: improve internal function costs
  2023-06-12 12:38     ` Andre Vieira (lists)
@ 2023-06-12 13:01       ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2023-06-12 13:01 UTC (permalink / raw)
  To: Andre Vieira (lists); +Cc: Jan Hubicka, Richard Sandiford, gcc-patches, jakub

On Mon, 12 Jun 2023, Andre Vieira (lists) wrote:

> 
> 
> On 05/06/2023 04:04, Jan Hubicka wrote:
> >> On Thu, 1 Jun 2023, Andre Vieira (lists) wrote:
> >>
> >>> Hi,
> >>>
> >>> This is a follow-up of the internal function patch to add widening and
> >>> narrowing patterns.  This patch improves the inliner cost estimation for
> >>> internal functions.
> >>
> >> I have no idea why calls are special in IPA analyze_function_body
> >> and so I cannot say whether treating all internal fn calls as
> >> non-calls is correct there.  Honza?
> > 
> > The reason is that normal statements are acconted as part of the
> > function body, while calls have their costs attached to call edges
> > (so it can be adjusted when call is inlined to otherwise optimized).
> > 
> > However since internal functions have no cgraph edges, this looks like
> > a bug that we do not test it.  (the code was written before internal
> > calls was introduced).
> >
> 
> This sounds to me like you agree with my approach to treat internal calls
> different to regular calls.
> 
> > I wonder if we don't want to have is_noninternal_gimple_call that could
> > be used by IPA code to test whether cgraph edge should exist for
> > the statement.
> 
> I'm happy to add such a helper function @richi,rsandifo: you ok with that?

It's a bit of an ugly name, if we want something that keys on calls
that have an edge it should be obvious it does this.  I wouldn't
add is_noninternal_gimple_call.  With LTO and libgcc and internal
optab fns it's also less obvious in cases we want to have say
.DIVMODDI3 (...) which in the end maps to a LTOed libcall from libgcc.a 
...

Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-12 13:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 16:30 [PATCH] inline: improve internal function costs Andre Vieira (lists)
2023-06-02  9:13 ` Richard Biener
2023-06-02  9:44   ` Andre Vieira (lists)
2023-06-05  3:04   ` Jan Hubicka
2023-06-12 12:38     ` Andre Vieira (lists)
2023-06-12 13:01       ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).