More cgraph_remove_unreachable_nodes fixes

Submitted by Jan Hubicka on Oct. 26, 2010, 9:34 p.m.

Details

Message ID 20101026213439.GA3813@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Oct. 26, 2010, 9:34 p.m.
Hi,
cgraph_remove_unreachable_nodes still remove masters of clones from othr
partitions that confuse sanity checking.  Fixed by the following patch that
also cleans up how things are queued. Everything that is referenced by live
part of cgraph is enqueued, everything that is really reachable gets reachable
flag.

Bootstrapped/regtested x86_64-linux, also lto-bootstrapped and comitted.
	* ipa.c (process_references): Enqueue all referenced nodes;
	mark as reachable only non-external nodes.
	(cgraph_remove_unreachable_nodes): All referenced nodes should
	be enqueued; remove bogues node->needed check.

Comments

Jie Zhang Nov. 26, 2010, 10:34 a.m.
Hi Jan,

On 10/27/2010 05:34 AM, Jan Hubicka wrote:
> Hi,
> cgraph_remove_unreachable_nodes still remove masters of clones from othr
> partitions that confuse sanity checking.  Fixed by the following patch that
> also cleans up how things are queued. Everything that is referenced by live
> part of cgraph is enqueued, everything that is really reachable gets reachable
> flag.
>
> Bootstrapped/regtested x86_64-linux, also lto-bootstrapped and comitted.
> 	* ipa.c (process_references): Enqueue all referenced nodes;
> 	mark as reachable only non-external nodes.
> 	(cgraph_remove_unreachable_nodes): All referenced nodes should
> 	be enqueued; remove bogues node->needed check.

This patch causes a failure when building glibc for 
arm-none-linux-gnueabi target. The reduced test case is attached.

Without this patch, memchr is defined in the assembly:

	.size	__memchr, .-__memchr
         .weak   __GI_memchr
         .hidden __GI_memchr
         .set    __GI_memchr,__memchr
         .global memchr
         .set    memchr,__GI_memchr
	.ident	"GCC: (GNU) 4.6.0 20101026 (experimental)"

but with this patch, memchr is not defined:

	.size	__memchr, .-__memchr
	.weak	__GI_memchr
	.hidden	__GI_memchr
	.set	__GI_memchr,__memchr
	.ident	"GCC: (GNU) 4.6.0 20101026 (experimental)"

When compiling the test case, -O is used on the command line.


Regards,
Jie
Richard Guenther Nov. 26, 2010, 11:43 a.m.
On Fri, Nov 26, 2010 at 11:34 AM, Jie Zhang <jie@codesourcery.com> wrote:
> Hi Jan,
>
> On 10/27/2010 05:34 AM, Jan Hubicka wrote:
>>
>> Hi,
>> cgraph_remove_unreachable_nodes still remove masters of clones from othr
>> partitions that confuse sanity checking.  Fixed by the following patch
>> that
>> also cleans up how things are queued. Everything that is referenced by
>> live
>> part of cgraph is enqueued, everything that is really reachable gets
>> reachable
>> flag.
>>
>> Bootstrapped/regtested x86_64-linux, also lto-bootstrapped and comitted.
>>        * ipa.c (process_references): Enqueue all referenced nodes;
>>        mark as reachable only non-external nodes.
>>        (cgraph_remove_unreachable_nodes): All referenced nodes should
>>        be enqueued; remove bogues node->needed check.
>
> This patch causes a failure when building glibc for arm-none-linux-gnueabi
> target. The reduced test case is attached.
>
> Without this patch, memchr is defined in the assembly:
>
>        .size   __memchr, .-__memchr
>        .weak   __GI_memchr
>        .hidden __GI_memchr
>        .set    __GI_memchr,__memchr
>        .global memchr
>        .set    memchr,__GI_memchr
>        .ident  "GCC: (GNU) 4.6.0 20101026 (experimental)"
>
> but with this patch, memchr is not defined:
>
>        .size   __memchr, .-__memchr
>        .weak   __GI_memchr
>        .hidden __GI_memchr
>        .set    __GI_memchr,__memchr
>        .ident  "GCC: (GNU) 4.6.0 20101026 (experimental)"
>
> When compiling the test case, -O is used on the command line.

Please also open a bugreport.

Thanks,
Richard.

>
> Regards,
> Jie
>
Jie Zhang Nov. 26, 2010, 12:02 p.m.
On 11/26/2010 07:43 PM, Richard Guenther wrote:
> On Fri, Nov 26, 2010 at 11:34 AM, Jie Zhang<jie@codesourcery.com>  wrote:
>> Hi Jan,
>>
>> On 10/27/2010 05:34 AM, Jan Hubicka wrote:
>>>
>>> Hi,
>>> cgraph_remove_unreachable_nodes still remove masters of clones from othr
>>> partitions that confuse sanity checking.  Fixed by the following patch
>>> that
>>> also cleans up how things are queued. Everything that is referenced by
>>> live
>>> part of cgraph is enqueued, everything that is really reachable gets
>>> reachable
>>> flag.
>>>
>>> Bootstrapped/regtested x86_64-linux, also lto-bootstrapped and comitted.
>>>         * ipa.c (process_references): Enqueue all referenced nodes;
>>>         mark as reachable only non-external nodes.
>>>         (cgraph_remove_unreachable_nodes): All referenced nodes should
>>>         be enqueued; remove bogues node->needed check.
>>
>> This patch causes a failure when building glibc for arm-none-linux-gnueabi
>> target. The reduced test case is attached.
>>
>> Without this patch, memchr is defined in the assembly:
>>
>>         .size   __memchr, .-__memchr
>>         .weak   __GI_memchr
>>         .hidden __GI_memchr
>>         .set    __GI_memchr,__memchr
>>         .global memchr
>>         .set    memchr,__GI_memchr
>>         .ident  "GCC: (GNU) 4.6.0 20101026 (experimental)"
>>
>> but with this patch, memchr is not defined:
>>
>>         .size   __memchr, .-__memchr
>>         .weak   __GI_memchr
>>         .hidden __GI_memchr
>>         .set    __GI_memchr,__memchr
>>         .ident  "GCC: (GNU) 4.6.0 20101026 (experimental)"
>>
>> When compiling the test case, -O is used on the command line.
>
> Please also open a bugreport.
>
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46674


Regards,

Patch hide | download patch | download mbox

Index: ipa.c
===================================================================
--- ipa.c	(revision 165985)
+++ ipa.c	(working copy)
@@ -170,12 +170,11 @@  process_references (struct ipa_ref_list
 	{
 	  struct cgraph_node *node = ipa_ref_node (ref);
 	  if (!node->reachable
+	      && node->analyzed
 	      && (!DECL_EXTERNAL (node->decl)
 	          || before_inlining_p))
-	    {
-	      node->reachable = true;
-	      enqueue_cgraph_node (node, first);
-	    }
+	    node->reachable = true;
+	  enqueue_cgraph_node (node, first);
 	}
       else
 	{
@@ -304,15 +303,15 @@  cgraph_remove_unreachable_nodes (bool be
 	  if (node->reachable)
 	    {
 	      for (e = node->callees; e; e = e->next_callee)
-		if (!e->callee->reachable
-		    && node->analyzed
-		    && (!e->inline_failed || !e->callee->analyzed
-			|| (!DECL_EXTERNAL (e->callee->decl))
-			|| before_inlining_p))
-		  {
+		{
+		  if (!e->callee->reachable
+		      && node->analyzed
+		      && (!e->inline_failed
+			  || !DECL_EXTERNAL (e->callee->decl)
+			  || before_inlining_p))
 		    e->callee->reachable = true;
-		    enqueue_cgraph_node (e->callee, &first);
-		  }
+		  enqueue_cgraph_node (e->callee, &first);
+		}
 	      process_references (&node->ref_list, &first, &first_varpool, before_inlining_p);
 	    }
 
@@ -416,7 +415,7 @@  cgraph_remove_unreachable_nodes (bool be
 	      found = true;
 
 	  /* If so, we need to keep node in the callgraph.  */
-	  if (found || node->needed)
+	  if (found)
 	    {
 	      if (node->analyzed)
 		{