public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Your patch to PR 30700
@ 2007-07-26 13:01 Diego Novillo
  2007-07-26 19:29 ` Doug Kwan (關振德)
  0 siblings, 1 reply; 2+ messages in thread
From: Diego Novillo @ 2007-07-26 13:01 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, "Doug Kwan (???)"

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


Jan,

Your patch http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=123922 is
causing an ICE when compiling this snippet with -g

namespace NS {
  int x = 0;
  int &ref = x;
}

using NS::ref;

The problem is that we are emitting debug info for 'x' during parsing,
so cgraph_global_info_ready is still false.  Since 'x' has already been
processed in this case, we will have a varpool entry for it.

Since it's too early, the 'needed' indicator may be too pessimistic, but
it didn't seem to me that we have much of a choice.  So, I thought that
if the varpool entry for the decl is already finalized, we can use it.

The other option would be to try and push this analysis much later after
parsing, but that seems fairly intrusive.  I'm currently testing the
attached patch.  Honza, does this make sense at all?

Thanks.

[-- Attachment #2: 20070726-31899.diff --]
[-- Type: text/x-patch, Size: 1358 bytes --]

2007-07-26  Diego Novillo  <dnovillo@google.com>

	PR 31899
	* dwarf2out.c (reference_to_unused): Handle VAR_DECLs even if
	not all the cgraph has been built.  Assert that the
	corresponding varpool node has been finalized.

testsuite/ChangeLog

	* g++.dg/debug/31899.C: New test.

Index: testsuite/g++.dg/debug/31899.C
===================================================================
--- testsuite/g++.dg/debug/31899.C	(revision 0)
+++ testsuite/g++.dg/debug/31899.C	(revision 0)
@@ -0,0 +1,8 @@
+// { dg-do compile }
+// { dg-options "-g"
+namespace NS {
+  int x = 0;
+  int &ref = x;
+}
+
+using NS::ref;
Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 126951)
+++ dwarf2out.c	(working copy)
@@ -10300,12 +10300,15 @@ reference_to_unused (tree * tp, int * wa
     return *tp;
   else if (!flag_unit_at_a_time)
     return NULL_TREE;
-  else if (!cgraph_global_info_ready
-	   && (TREE_CODE (*tp) == VAR_DECL || TREE_CODE (*tp) == FUNCTION_DECL))
+  else if (!cgraph_global_info_ready && TREE_CODE (*tp) == FUNCTION_DECL)
     gcc_unreachable ();
   else if (DECL_P (*tp) && TREE_CODE (*tp) == VAR_DECL)
     {
       struct varpool_node *node = varpool_node (*tp);
+
+      if (!cgraph_global_info_ready)
+	gcc_assert (node->finalized);
+
       if (!node->needed)
 	return *tp;
     }

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

* Re: Your patch to PR 30700
  2007-07-26 13:01 Your patch to PR 30700 Diego Novillo
@ 2007-07-26 19:29 ` Doug Kwan (關振德)
  0 siblings, 0 replies; 2+ messages in thread
From: Doug Kwan (關振德) @ 2007-07-26 19:29 UTC (permalink / raw)
  To: Diego Novillo; +Cc: Jan Hubicka, gcc-patches

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

Alternatively, I would like to propose replacing the assert into a
simple "return *tp".  Without the complete call-graph, we cannot tell
if a DECL will be still unused eventually.  So a reasonable thing to
do is not emit DWARF2 info early.

I'm running dejagnu right now.

-Doug

2007/7/26, Diego Novillo <dnovillo@google.com>:
>
> Jan,
>
> Your patch http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=123922 is
> causing an ICE when compiling this snippet with -g
>
> namespace NS {
>   int x = 0;
>   int &ref = x;
> }
>
> using NS::ref;
>
> The problem is that we are emitting debug info for 'x' during parsing,
> so cgraph_global_info_ready is still false.  Since 'x' has already been
> processed in this case, we will have a varpool entry for it.
>
> Since it's too early, the 'needed' indicator may be too pessimistic, but
> it didn't seem to me that we have much of a choice.  So, I thought that
> if the varpool entry for the decl is already finalized, we can use it.
>
> The other option would be to try and push this analysis much later after
> parsing, but that seems fairly intrusive.  I'm currently testing the
> attached patch.  Honza, does this make sense at all?
>
> Thanks.
>
>

[-- Attachment #2: diffs-31899.txt --]
[-- Type: text/plain, Size: 1842 bytes --]

2007-07-26  Doug Kwan  <dougkwan@google.com>

  * dwarf2out.c: (may_reference_to_unsued) : Renamed from reference_to_unsued
    as it is now more conservative than before. Replace unreachable assert
    into a return that tells caller that tree may reference an unused DECL.
    (rtl_for_decl_init): Rename callee to may_reference_to_usused.

--- dwarf2out.c.orig	2007-07-26 11:28:11.000000000 -0700
+++ dwarf2out.c	2007-07-26 11:31:33.000000000 -0700
@@ -9993,9 +9993,10 @@
 /* Determine whether the evaluation of EXPR references any variables
    or functions which aren't otherwise used (and therefore may not be
    output).  */
+
 static tree
-reference_to_unused (tree * tp, int * walk_subtrees,
-		     void * data ATTRIBUTE_UNUSED)
+may_reference_to_unused (tree * tp, int * walk_subtrees,
+			 void * data ATTRIBUTE_UNUSED)
 {
   if (! EXPR_P (*tp) && ! CONSTANT_CLASS_P (*tp))
     *walk_subtrees = 0;
@@ -10007,7 +10008,12 @@
     return NULL_TREE;
   else if (!cgraph_global_info_ready
 	   && (TREE_CODE (*tp) == VAR_DECL || TREE_CODE (*tp) == FUNCTION_DECL))
-    gcc_unreachable ();
+    {
+      /* We don't know if a DECL is really unsued until we have seen
+ 	 the complete call graph.  With no reliable information, we need to
+	 be conservative.  */
+      return *tp;
+    }
   else if (DECL_P (*tp) && TREE_CODE (*tp) == VAR_DECL)
     {
       struct cgraph_varpool_node *node = cgraph_varpool_node (*tp);
@@ -10063,7 +10069,7 @@
      immediate RTL constant, expand it now.  We must be careful not to
      reference variables which won't be output.  */
   else if (initializer_constant_valid_p (init, type)
-	   && ! walk_tree (&init, reference_to_unused, NULL, NULL))
+	   && ! walk_tree (&init, may_reference_to_unused, NULL, NULL))
     {
       rtl = expand_expr (init, NULL_RTX, VOIDmode, EXPAND_INITIALIZER);
 

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

end of thread, other threads:[~2007-07-26 18:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-26 13:01 Your patch to PR 30700 Diego Novillo
2007-07-26 19:29 ` Doug Kwan (關振德)

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