Patchwork More cgraph_remove_unreachable_nodes fixes

login
register
mail settings
Submitter Jan Hubicka
Date Oct. 26, 2010, 9:34 p.m.
Message ID <20101026213439.GA3813@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/69294/
State New
Headers show

Comments

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.
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

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)
 		{