diff mbox series

Use get_create for fn_summary (PR ipa/87491).

Message ID d770d9c8-002c-0f2b-cc63-b6cd8ae99bd1@suse.cz
State New
Headers show
Series Use get_create for fn_summary (PR ipa/87491). | expand

Commit Message

Martin Liška Oct. 3, 2018, 8:56 a.m. UTC
Hi.

There's one another ICE when calling fn_summary->get for a node
that's not present. Back-trace is:

(gdb) bt
#0  inline_to_all_callers (node=<cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>, data=0x7fffffffd690) at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2260
#1  0x0000000000b0b2c1 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=<optimized out>, data=<optimized out>, callback=<optimized out>, this=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraph.h:3221
#2  cgraph_node::call_for_symbol_and_aliases_1 (this=this@entry=<cgraph_node * const 0x7ffff698f438 "a"/1>, callback=callback@entry=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, data=data@entry=0x7fffffffd690, 
    include_overwritable=include_overwritable@entry=true) at /home/marxin/Programming/gcc/gcc/cgraph.c:3745
#3  0x0000000001739422 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=true, data=0x7fffffffd690, callback=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, this=<cgraph_node * const 0x7ffff698f438 "a"/1>)
    at /home/marxin/Programming/gcc/gcc/cgraph.h:3225
#4  ipa_inline () at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2581

with -fdump-ipa-all -fno-inline-small-functions  -O3.
The problematic symbol is:

(gdb) p node
$1 = <cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>
(gdb) p node->debug()
*.LTHUNK0/2 (int B::*.LTHUNK0(int, int)) @0x7ffff698f5a0
  Type: function definition analyzed alias cpp_implicit_alias
  Visibility: prevailing_def_ironly artificial
  References: _ZN1B1aEii/1 (alias)
  Referring: 
  Availability: available
  First run: 0
  Function flags:
  Called by: virtual int B::_ZTv0_n24_N1B1aEii(int, int)/3 (can throw external) 
  Calls: 

Hope it's fine to use get_create and let the symbol be inlined?
Patch survives tests on x86_64-linux-gnu.

Ready for trunk?
Martin

gcc/ChangeLog:

2018-10-03  Martin Liska  <mliska@suse.cz>

	PR ipa/87491
	* ipa-inline.c (inline_to_all_callers_1): Use ::get_create
	at place where we can have a function that's not
	in summary.
---
 gcc/ipa-inline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Hubicka Oct. 3, 2018, 9:07 a.m. UTC | #1
> Hi.
> 
> There's one another ICE when calling fn_summary->get for a node
> that's not present. Back-trace is:
> 
> (gdb) bt
> #0  inline_to_all_callers (node=<cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>, data=0x7fffffffd690) at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2260
> #1  0x0000000000b0b2c1 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=<optimized out>, data=<optimized out>, callback=<optimized out>, this=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraph.h:3221
> #2  cgraph_node::call_for_symbol_and_aliases_1 (this=this@entry=<cgraph_node * const 0x7ffff698f438 "a"/1>, callback=callback@entry=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, data=data@entry=0x7fffffffd690, 
>     include_overwritable=include_overwritable@entry=true) at /home/marxin/Programming/gcc/gcc/cgraph.c:3745
> #3  0x0000000001739422 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=true, data=0x7fffffffd690, callback=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, this=<cgraph_node * const 0x7ffff698f438 "a"/1>)
>     at /home/marxin/Programming/gcc/gcc/cgraph.h:3225
> #4  ipa_inline () at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2581
> 
> with -fdump-ipa-all -fno-inline-small-functions  -O3.
> The problematic symbol is:
> 
> (gdb) p node
> $1 = <cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>
> (gdb) p node->debug()
> *.LTHUNK0/2 (int B::*.LTHUNK0(int, int)) @0x7ffff698f5a0
>   Type: function definition analyzed alias cpp_implicit_alias
>   Visibility: prevailing_def_ironly artificial
>   References: _ZN1B1aEii/1 (alias)
>   Referring: 
>   Availability: available
>   First run: 0
>   Function flags:
>   Called by: virtual int B::_ZTv0_n24_N1B1aEii(int, int)/3 (can throw external) 
>   Calls: 
> 
> Hope it's fine to use get_create and let the symbol be inlined?
> Patch survives tests on x86_64-linux-gnu.
> 
> Ready for trunk?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-10-03  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/87491
> 	* ipa-inline.c (inline_to_all_callers_1): Use ::get_create
> 	at place where we can have a function that's not
> 	in summary.

Creating empty (and thus bogus) summary is not a giid idea. 
We want to print summary of the ultimate alias target of node.

Honza
> ---
>  gcc/ipa-inline.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 

> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> index 025788522fb..eea2888011c 100644
> --- a/gcc/ipa-inline.c
> +++ b/gcc/ipa-inline.c
> @@ -2222,7 +2222,7 @@ inline_to_all_callers_1 (struct cgraph_node *node, void *data,
>  	  fprintf (dump_file,
>  		   "\nInlining %s size %i.\n",
>  		   node->name (),
> -		   ipa_fn_summaries->get (node)->size);
> +		   ipa_fn_summaries->get_create (node)->size);
>  	  fprintf (dump_file,
>  		   " Called once from %s %i insns.\n",
>  		   node->callers->caller->name (),
>
Martin Liška Oct. 3, 2018, 9:57 a.m. UTC | #2
On 10/3/18 11:07 AM, Jan Hubicka wrote:
>> Hi.
>>
>> There's one another ICE when calling fn_summary->get for a node
>> that's not present. Back-trace is:
>>
>> (gdb) bt
>> #0  inline_to_all_callers (node=<cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>, data=0x7fffffffd690) at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2260
>> #1  0x0000000000b0b2c1 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=<optimized out>, data=<optimized out>, callback=<optimized out>, this=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraph.h:3221
>> #2  cgraph_node::call_for_symbol_and_aliases_1 (this=this@entry=<cgraph_node * const 0x7ffff698f438 "a"/1>, callback=callback@entry=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, data=data@entry=0x7fffffffd690, 
>>     include_overwritable=include_overwritable@entry=true) at /home/marxin/Programming/gcc/gcc/cgraph.c:3745
>> #3  0x0000000001739422 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=true, data=0x7fffffffd690, callback=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, this=<cgraph_node * const 0x7ffff698f438 "a"/1>)
>>     at /home/marxin/Programming/gcc/gcc/cgraph.h:3225
>> #4  ipa_inline () at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2581
>>
>> with -fdump-ipa-all -fno-inline-small-functions  -O3.
>> The problematic symbol is:
>>
>> (gdb) p node
>> $1 = <cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>
>> (gdb) p node->debug()
>> *.LTHUNK0/2 (int B::*.LTHUNK0(int, int)) @0x7ffff698f5a0
>>   Type: function definition analyzed alias cpp_implicit_alias
>>   Visibility: prevailing_def_ironly artificial
>>   References: _ZN1B1aEii/1 (alias)
>>   Referring: 
>>   Availability: available
>>   First run: 0
>>   Function flags:
>>   Called by: virtual int B::_ZTv0_n24_N1B1aEii(int, int)/3 (can throw external) 
>>   Calls: 
>>
>> Hope it's fine to use get_create and let the symbol be inlined?
>> Patch survives tests on x86_64-linux-gnu.
>>
>> Ready for trunk?
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-10-03  Martin Liska  <mliska@suse.cz>
>>
>> 	PR ipa/87491
>> 	* ipa-inline.c (inline_to_all_callers_1): Use ::get_create
>> 	at place where we can have a function that's not
>> 	in summary.
> 
> Creating empty (and thus bogus) summary is not a giid idea. 
> We want to print summary of the ultimate alias target of node.

I see, fixed in attached patch.
Is it ok now?

Martin

> 
> Honza
>> ---
>>  gcc/ipa-inline.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
> 
>> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
>> index 025788522fb..eea2888011c 100644
>> --- a/gcc/ipa-inline.c
>> +++ b/gcc/ipa-inline.c
>> @@ -2222,7 +2222,7 @@ inline_to_all_callers_1 (struct cgraph_node *node, void *data,
>>  	  fprintf (dump_file,
>>  		   "\nInlining %s size %i.\n",
>>  		   node->name (),
>> -		   ipa_fn_summaries->get (node)->size);
>> +		   ipa_fn_summaries->get_create (node)->size);
>>  	  fprintf (dump_file,
>>  		   " Called once from %s %i insns.\n",
>>  		   node->callers->caller->name (),
>>
>
From a68cd435af85bf61811c6947bc5c97d65e2af2a3 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 3 Oct 2018 10:14:14 +0200
Subject: [PATCH] Call ultimate_alias_target for node being inlined (PR
 ipa/87491).

gcc/ChangeLog:

2018-10-03  Martin Liska  <mliska@suse.cz>

	PR ipa/87491
	* ipa-inline.c (inline_to_all_callers_1):
	Call ultimate_alias_target for node being inlined.
---
 gcc/ipa-inline.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 025788522fb..4f8ed1520f1 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -2219,10 +2219,11 @@ inline_to_all_callers_1 (struct cgraph_node *node, void *data,
 
       if (dump_file)
 	{
+	  cgraph_node *ultimate = node->ultimate_alias_target ();
 	  fprintf (dump_file,
 		   "\nInlining %s size %i.\n",
-		   node->name (),
-		   ipa_fn_summaries->get (node)->size);
+		   ultimate->name (),
+		   ipa_fn_summaries->get (ultimate)->size);
 	  fprintf (dump_file,
 		   " Called once from %s %i insns.\n",
 		   node->callers->caller->name (),
Jeff Law Oct. 3, 2018, 4:56 p.m. UTC | #3
On 10/3/18 3:57 AM, Martin Liška wrote:
> On 10/3/18 11:07 AM, Jan Hubicka wrote:
>>> Hi.
>>>
>>> There's one another ICE when calling fn_summary->get for a node
>>> that's not present. Back-trace is:
>>>
>>> (gdb) bt
>>> #0  inline_to_all_callers (node=<cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>, data=0x7fffffffd690) at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2260
>>> #1  0x0000000000b0b2c1 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=<optimized out>, data=<optimized out>, callback=<optimized out>, this=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraph.h:3221
>>> #2  cgraph_node::call_for_symbol_and_aliases_1 (this=this@entry=<cgraph_node * const 0x7ffff698f438 "a"/1>, callback=callback@entry=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, data=data@entry=0x7fffffffd690, 
>>>     include_overwritable=include_overwritable@entry=true) at /home/marxin/Programming/gcc/gcc/cgraph.c:3745
>>> #3  0x0000000001739422 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=true, data=0x7fffffffd690, callback=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, this=<cgraph_node * const 0x7ffff698f438 "a"/1>)
>>>     at /home/marxin/Programming/gcc/gcc/cgraph.h:3225
>>> #4  ipa_inline () at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2581
>>>
>>> with -fdump-ipa-all -fno-inline-small-functions  -O3.
>>> The problematic symbol is:
>>>
>>> (gdb) p node
>>> $1 = <cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>
>>> (gdb) p node->debug()
>>> *.LTHUNK0/2 (int B::*.LTHUNK0(int, int)) @0x7ffff698f5a0
>>>   Type: function definition analyzed alias cpp_implicit_alias
>>>   Visibility: prevailing_def_ironly artificial
>>>   References: _ZN1B1aEii/1 (alias)
>>>   Referring: 
>>>   Availability: available
>>>   First run: 0
>>>   Function flags:
>>>   Called by: virtual int B::_ZTv0_n24_N1B1aEii(int, int)/3 (can throw external) 
>>>   Calls: 
>>>
>>> Hope it's fine to use get_create and let the symbol be inlined?
>>> Patch survives tests on x86_64-linux-gnu.
>>>
>>> Ready for trunk?
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-10-03  Martin Liska  <mliska@suse.cz>
>>>
>>> 	PR ipa/87491
>>> 	* ipa-inline.c (inline_to_all_callers_1): Use ::get_create
>>> 	at place where we can have a function that's not
>>> 	in summary.
>> Creating empty (and thus bogus) summary is not a giid idea. 
>> We want to print summary of the ultimate alias target of node.
> I see, fixed in attached patch.
> Is it ok now?
> 
> Martin
> 
>> Honza
>>> ---
>>>  gcc/ipa-inline.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>
>>> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
>>> index 025788522fb..eea2888011c 100644
>>> --- a/gcc/ipa-inline.c
>>> +++ b/gcc/ipa-inline.c
>>> @@ -2222,7 +2222,7 @@ inline_to_all_callers_1 (struct cgraph_node *node, void *data,
>>>  	  fprintf (dump_file,
>>>  		   "\nInlining %s size %i.\n",
>>>  		   node->name (),
>>> -		   ipa_fn_summaries->get (node)->size);
>>> +		   ipa_fn_summaries->get_create (node)->size);
>>>  	  fprintf (dump_file,
>>>  		   " Called once from %s %i insns.\n",
>>>  		   node->callers->caller->name (),
>>>
> 
> 0001-Call-ultimate_alias_target-for-node-being-inlined-PR.patch
> 
> From a68cd435af85bf61811c6947bc5c97d65e2af2a3 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 3 Oct 2018 10:14:14 +0200
> Subject: [PATCH] Call ultimate_alias_target for node being inlined (PR
>  ipa/87491).
> 
> gcc/ChangeLog:
> 
> 2018-10-03  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/87491
> 	* ipa-inline.c (inline_to_all_callers_1):
> 	Call ultimate_alias_target for node being inlined.
OK.  FWIW, I would be suspicious of a get_create call in dumping code --
couldn't that result in creating a node in the graph only when dumping
was enabled?  If that interacted badly with something else it'd be a
pain to find.

Jeff
Martin Liška Oct. 4, 2018, 8:08 a.m. UTC | #4
On 10/3/18 6:56 PM, Jeff Law wrote:
> On 10/3/18 3:57 AM, Martin Liška wrote:
>> On 10/3/18 11:07 AM, Jan Hubicka wrote:
>>>> Hi.
>>>>
>>>> There's one another ICE when calling fn_summary->get for a node
>>>> that's not present. Back-trace is:
>>>>
>>>> (gdb) bt
>>>> #0  inline_to_all_callers (node=<cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>, data=0x7fffffffd690) at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2260
>>>> #1  0x0000000000b0b2c1 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=<optimized out>, data=<optimized out>, callback=<optimized out>, this=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraph.h:3221
>>>> #2  cgraph_node::call_for_symbol_and_aliases_1 (this=this@entry=<cgraph_node * const 0x7ffff698f438 "a"/1>, callback=callback@entry=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, data=data@entry=0x7fffffffd690, 
>>>>     include_overwritable=include_overwritable@entry=true) at /home/marxin/Programming/gcc/gcc/cgraph.c:3745
>>>> #3  0x0000000001739422 in cgraph_node::call_for_symbol_and_aliases (include_overwritable=true, data=0x7fffffffd690, callback=0x1737440 <inline_to_all_callers(cgraph_node*, void*)>, this=<cgraph_node * const 0x7ffff698f438 "a"/1>)
>>>>     at /home/marxin/Programming/gcc/gcc/cgraph.h:3225
>>>> #4  ipa_inline () at /home/marxin/Programming/gcc/gcc/ipa-inline.c:2581
>>>>
>>>> with -fdump-ipa-all -fno-inline-small-functions  -O3.
>>>> The problematic symbol is:
>>>>
>>>> (gdb) p node
>>>> $1 = <cgraph_node * 0x7ffff698f5a0 "*.LTHUNK0"/2>
>>>> (gdb) p node->debug()
>>>> *.LTHUNK0/2 (int B::*.LTHUNK0(int, int)) @0x7ffff698f5a0
>>>>   Type: function definition analyzed alias cpp_implicit_alias
>>>>   Visibility: prevailing_def_ironly artificial
>>>>   References: _ZN1B1aEii/1 (alias)
>>>>   Referring: 
>>>>   Availability: available
>>>>   First run: 0
>>>>   Function flags:
>>>>   Called by: virtual int B::_ZTv0_n24_N1B1aEii(int, int)/3 (can throw external) 
>>>>   Calls: 
>>>>
>>>> Hope it's fine to use get_create and let the symbol be inlined?
>>>> Patch survives tests on x86_64-linux-gnu.
>>>>
>>>> Ready for trunk?
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-10-03  Martin Liska  <mliska@suse.cz>
>>>>
>>>> 	PR ipa/87491
>>>> 	* ipa-inline.c (inline_to_all_callers_1): Use ::get_create
>>>> 	at place where we can have a function that's not
>>>> 	in summary.
>>> Creating empty (and thus bogus) summary is not a giid idea. 
>>> We want to print summary of the ultimate alias target of node.
>> I see, fixed in attached patch.
>> Is it ok now?
>>
>> Martin
>>
>>> Honza
>>>> ---
>>>>  gcc/ipa-inline.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>
>>>> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
>>>> index 025788522fb..eea2888011c 100644
>>>> --- a/gcc/ipa-inline.c
>>>> +++ b/gcc/ipa-inline.c
>>>> @@ -2222,7 +2222,7 @@ inline_to_all_callers_1 (struct cgraph_node *node, void *data,
>>>>  	  fprintf (dump_file,
>>>>  		   "\nInlining %s size %i.\n",
>>>>  		   node->name (),
>>>> -		   ipa_fn_summaries->get (node)->size);
>>>> +		   ipa_fn_summaries->get_create (node)->size);
>>>>  	  fprintf (dump_file,
>>>>  		   " Called once from %s %i insns.\n",
>>>>  		   node->callers->caller->name (),
>>>>
>>
>> 0001-Call-ultimate_alias_target-for-node-being-inlined-PR.patch
>>
>> From a68cd435af85bf61811c6947bc5c97d65e2af2a3 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Wed, 3 Oct 2018 10:14:14 +0200
>> Subject: [PATCH] Call ultimate_alias_target for node being inlined (PR
>>  ipa/87491).
>>
>> gcc/ChangeLog:
>>
>> 2018-10-03  Martin Liska  <mliska@suse.cz>
>>
>> 	PR ipa/87491
>> 	* ipa-inline.c (inline_to_all_callers_1):
>> 	Call ultimate_alias_target for node being inlined.
> OK.  FWIW, I would be suspicious of a get_create call in dumping code --
> couldn't that result in creating a node in the graph only when dumping
> was enabled?  If that interacted badly with something else it'd be a
> pain to find.

Yes, that's why my second patch uses ultimate alias target.
Patch installed, thanks for review.

Martin

> 
> Jeff
>
diff mbox series

Patch

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 025788522fb..eea2888011c 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -2222,7 +2222,7 @@  inline_to_all_callers_1 (struct cgraph_node *node, void *data,
 	  fprintf (dump_file,
 		   "\nInlining %s size %i.\n",
 		   node->name (),
-		   ipa_fn_summaries->get (node)->size);
+		   ipa_fn_summaries->get_create (node)->size);
 	  fprintf (dump_file,
 		   " Called once from %s %i insns.\n",
 		   node->callers->caller->name (),