diff mbox

Fix lto-profiledbootstrap [was: Merge cgraph_get_create_node and cgraph_get_create_real_symbol_node]

Message ID CAFULd4ajQw45c5SrHVvbebFfQoguPmJ8xpSGiB1MB48qAAZqLg@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Nov. 14, 2013, 8:51 p.m. UTC
Now with the patch attached.

On Thu, Nov 14, 2013 at 9:49 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Nov 14, 2013 at 7:16 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>
> Attached patch fixes lto-profiledbootstrap, introduced by:
>
>>>>> 2013-11-12  Martin Jambor  <mjambor@suse.cz>
>>>>>
>>>>> * cgraph.c (cgraph_get_create_node): Do what
>>>>> cgraph_get_create_real_symbol_node used to do.
>>>>> (cgraph_get_create_real_symbol_node): Removed.  Changed all users to
>>>>> call cgraph_get_create_node.
>>>>> * cgraph.h (cgraph_get_create_real_symbol_node): Removed.
>>>>> * lto-streamer-in.c (input_function): Call cgraph_get_node instead of
>>>>> cgraph_get_create_node.  Assert we get a node.
>
> The patch reverts lto-streamer-in.c functionality back to what was
> doing before the above patch.
>
> 2013-11-14  Uros Bizjak  <ubizjak@gmail.com>
>
>     * lto-streamer-in.c (input function): Call cgraph_create_node if
>     cgraph_get_node failed.
>
> Tested with lto-profiledbootstrap on x86_64-pc-linux-gnu, regression
> tested also with -m32 [1].
>
> OK for mainline?
>
> [1] http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg00989.html
>
> Uros.

Comments

Jan Hubicka Nov. 14, 2013, 11:46 p.m. UTC | #1
> > 2013-11-14  Uros Bizjak  <ubizjak@gmail.com>
> >
> >     * lto-streamer-in.c (input function): Call cgraph_create_node if
> >     cgraph_get_node failed.
> >
> > Tested with lto-profiledbootstrap on x86_64-pc-linux-gnu, regression
> > tested also with -m32 [1].
> >
> > OK for mainline?

OK (though it is a bit ugly - but lto streaming is very much a special case),
thanks!
Honza
> >
> > [1] http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg00989.html
> >
> > Uros.

> Index: lto-streamer-in.c
> ===================================================================
> --- lto-streamer-in.c	(revision 204807)
> +++ lto-streamer-in.c	(working copy)
> @@ -917,7 +917,8 @@ input_function (tree fn_decl, struct data_in *data
>    gimple_register_cfg_hooks ();
>  
>    node = cgraph_get_node (fn_decl);
> -  gcc_checking_assert (node);
> +  if (!node)
> +    node = cgraph_create_node (fn_decl);
>    input_struct_function_base (fn, data_in, ib);
>    input_cfg (ib_cfg, fn, node->count_materialization_scale);
>
Richard Biener Nov. 15, 2013, 9:55 a.m. UTC | #2
On Fri, Nov 15, 2013 at 12:46 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > 2013-11-14  Uros Bizjak  <ubizjak@gmail.com>
>> >
>> >     * lto-streamer-in.c (input function): Call cgraph_create_node if
>> >     cgraph_get_node failed.
>> >
>> > Tested with lto-profiledbootstrap on x86_64-pc-linux-gnu, regression
>> > tested also with -m32 [1].
>> >
>> > OK for mainline?
>
> OK (though it is a bit ugly - but lto streaming is very much a special case),
> thanks!

Why would the cgraph node be not present when we input the body!?
Isn't there a bug elsewhere??

IMHO this just papers over an issue.

Richard.

> Honza
>> >
>> > [1] http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg00989.html
>> >
>> > Uros.
>
>> Index: lto-streamer-in.c
>> ===================================================================
>> --- lto-streamer-in.c (revision 204807)
>> +++ lto-streamer-in.c (working copy)
>> @@ -917,7 +917,8 @@ input_function (tree fn_decl, struct data_in *data
>>    gimple_register_cfg_hooks ();
>>
>>    node = cgraph_get_node (fn_decl);
>> -  gcc_checking_assert (node);
>> +  if (!node)
>> +    node = cgraph_create_node (fn_decl);
>>    input_struct_function_base (fn, data_in, ib);
>>    input_cfg (ib_cfg, fn, node->count_materialization_scale);
>>
>
Jan Hubicka Nov. 15, 2013, 10:02 a.m. UTC | #3
> On Fri, Nov 15, 2013 at 12:46 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> > 2013-11-14  Uros Bizjak  <ubizjak@gmail.com>
> >> >
> >> >     * lto-streamer-in.c (input function): Call cgraph_create_node if
> >> >     cgraph_get_node failed.
> >> >
> >> > Tested with lto-profiledbootstrap on x86_64-pc-linux-gnu, regression
> >> > tested also with -m32 [1].
> >> >
> >> > OK for mainline?
> >
> > OK (though it is a bit ugly - but lto streaming is very much a special case),
> > thanks!
> 
> Why would the cgraph node be not present when we input the body!?
> Isn't there a bug elsewhere??
> 
> IMHO this just papers over an issue.

Hmm, right, I misread it to be lto-cgraph.c instead (where it would make sense,
since we may read nodes previously constructed earlier by decl sharing).

I will look into this.  We should to have nodes present here.

Honza
Uros Bizjak Nov. 15, 2013, 10:04 a.m. UTC | #4
On Fri, Nov 15, 2013 at 11:02 AM, Jan Hubicka <hubicka@ucw.cz> wrote:

>> >> >     * lto-streamer-in.c (input function): Call cgraph_create_node if
>> >> >     cgraph_get_node failed.
>> >> >
>> >> > Tested with lto-profiledbootstrap on x86_64-pc-linux-gnu, regression
>> >> > tested also with -m32 [1].
>> >> >
>> >> > OK for mainline?
>> >
>> > OK (though it is a bit ugly - but lto streaming is very much a special case),
>> > thanks!
>>
>> Why would the cgraph node be not present when we input the body!?
>> Isn't there a bug elsewhere??
>>
>> IMHO this just papers over an issue.
>
> Hmm, right, I misread it to be lto-cgraph.c instead (where it would make sense,
> since we may read nodes previously constructed earlier by decl sharing).
>
> I will look into this.  We should to have nodes present here.

Please also test your change with lto-profiledbootstrap. The problem
didn't trigger with lto-bootstrap.

Uros.
diff mbox

Patch

Index: lto-streamer-in.c
===================================================================
--- lto-streamer-in.c	(revision 204807)
+++ lto-streamer-in.c	(working copy)
@@ -917,7 +917,8 @@  input_function (tree fn_decl, struct data_in *data
   gimple_register_cfg_hooks ();
 
   node = cgraph_get_node (fn_decl);
-  gcc_checking_assert (node);
+  if (!node)
+    node = cgraph_create_node (fn_decl);
   input_struct_function_base (fn, data_in, ib);
   input_cfg (ib_cfg, fn, node->count_materialization_scale);