public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH PING] unreviewed tree-slimming patches
@ 2011-05-25 14:33 Nathan Froyd
  2011-05-25 14:40 ` Jason Merrill
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Nathan Froyd @ 2011-05-25 14:33 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jason Merrill, Joseph S. Myers, java-patches, Richard Guenther

These patches:

  (C, C++, middle-end)
  [PATCH 14/18] move TS_STATEMENT_LIST to be a substructure of TS_TYPED
  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00560.html

  (C, Java, middle-end)
  [PATCH 18/18] make TS_BLOCK a substructure of TS_BASE
  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00564.html

are still pending review.  Jason commented on the TS_STATEMENT_LIST patch, but
the discussion didn't come to a resolution.  I forgot to CC the TS_BLOCK patch
to the Java folks the first time around.

Thanks,
-Nathan

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 14:33 [PATCH PING] unreviewed tree-slimming patches Nathan Froyd
@ 2011-05-25 14:40 ` Jason Merrill
  2011-05-26  8:47   ` Nathan Froyd
  2011-05-25 14:42 ` Richard Guenther
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2011-05-25 14:40 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches, Joseph S. Myers, java-patches, Richard Guenther

On 05/25/2011 10:00 AM, Nathan Froyd wrote:
> Jason commented on the TS_STATEMENT_LIST patch, but
> the discussion didn't come to a resolution.

Right, from your last mail I thought that you were investigating my 
question about add_stmt and your suggestion about dropping the NULL 
checking in append_to_statement_list_1.

Jason

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 14:33 [PATCH PING] unreviewed tree-slimming patches Nathan Froyd
  2011-05-25 14:40 ` Jason Merrill
@ 2011-05-25 14:42 ` Richard Guenther
  2011-05-25 17:44 ` Joseph S. Myers
  2011-05-25 19:31 ` Tom Tromey
  3 siblings, 0 replies; 12+ messages in thread
From: Richard Guenther @ 2011-05-25 14:42 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches, Jason Merrill, Joseph S. Myers, java-patches

On Wed, May 25, 2011 at 4:00 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> These patches:
>
>  (C, C++, middle-end)
>  [PATCH 14/18] move TS_STATEMENT_LIST to be a substructure of TS_TYPED
>  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00560.html
>
>  (C, Java, middle-end)
>  [PATCH 18/18] make TS_BLOCK a substructure of TS_BASE
>  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00564.html
>
> are still pending review.  Jason commented on the TS_STATEMENT_LIST patch, but
> the discussion didn't come to a resolution.  I forgot to CC the TS_BLOCK patch
> to the Java folks the first time around.

The middle-end parts of both patches are ok.

Thanks,
Richard.

> Thanks,
> -Nathan
>

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 14:33 [PATCH PING] unreviewed tree-slimming patches Nathan Froyd
  2011-05-25 14:40 ` Jason Merrill
  2011-05-25 14:42 ` Richard Guenther
@ 2011-05-25 17:44 ` Joseph S. Myers
  2011-05-25 19:31 ` Tom Tromey
  3 siblings, 0 replies; 12+ messages in thread
From: Joseph S. Myers @ 2011-05-25 17:44 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches, Jason Merrill, java-patches, Richard Guenther

On Wed, 25 May 2011, Nathan Froyd wrote:

> These patches:
> 
>   (C, C++, middle-end)
>   [PATCH 14/18] move TS_STATEMENT_LIST to be a substructure of TS_TYPED
>   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00560.html
> 
>   (C, Java, middle-end)
>   [PATCH 18/18] make TS_BLOCK a substructure of TS_BASE
>   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00564.html
> 
> are still pending review.  Jason commented on the TS_STATEMENT_LIST patch, but

The C changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 14:33 [PATCH PING] unreviewed tree-slimming patches Nathan Froyd
                   ` (2 preceding siblings ...)
  2011-05-25 17:44 ` Joseph S. Myers
@ 2011-05-25 19:31 ` Tom Tromey
  2011-05-25 19:36   ` Nathan Froyd
  3 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2011-05-25 19:31 UTC (permalink / raw)
  To: Nathan Froyd
  Cc: gcc-patches, Jason Merrill, Joseph S. Myers, java-patches,
	Richard Guenther

>>>>> "Nathan" == Nathan Froyd <froydnj@codesourcery.com> writes:

Nathan>   (C, Java, middle-end)
Nathan>   [PATCH 18/18] make TS_BLOCK a substructure of TS_BASE
Nathan>   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00564.html

The Java parts are ok.

I think these sorts of changes should be obvious once approved from a
middle-end perspective, at least assuming that there are no regressions.
I say this because I think that once the core change has been decided
on, there is often just one way to go about fixing up the users; at
least, in a case like this where the consequence amounts to deleting
assignments.

I mentioned this idea before but I didn't see any discussion of it.  I
am happy to continue looking at patches like this if that is what the
more active maintainers would prefer.

Tom

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 19:31 ` Tom Tromey
@ 2011-05-25 19:36   ` Nathan Froyd
  0 siblings, 0 replies; 12+ messages in thread
From: Nathan Froyd @ 2011-05-25 19:36 UTC (permalink / raw)
  To: Tom Tromey
  Cc: gcc-patches, Jason Merrill, Joseph S. Myers, java-patches,
	Richard Guenther

On 05/25/2011 02:06 PM, Tom Tromey wrote:
>>>>>> "Nathan" == Nathan Froyd <froydnj@codesourcery.com> writes:
> 
> Nathan>   (C, Java, middle-end)
> Nathan>   [PATCH 18/18] make TS_BLOCK a substructure of TS_BASE
> Nathan>   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00564.html
> 
> The Java parts are ok.
> 
> I think these sorts of changes should be obvious once approved from a
> middle-end perspective, at least assuming that there are no regressions.
> 
> I mentioned this idea before but I didn't see any discussion of it.  I
> am happy to continue looking at patches like this if that is what the
> more active maintainers would prefer.

I think Jason mentioned considering them approved after waiting a week.  If we
want to enshrine that as policy, I think that'd be reasonable.  All in favor...?

-Nathan

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-25 14:40 ` Jason Merrill
@ 2011-05-26  8:47   ` Nathan Froyd
  2011-05-26 14:00     ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Nathan Froyd @ 2011-05-26  8:47 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On 05/25/2011 10:18 AM, Jason Merrill wrote:
> On 05/25/2011 10:00 AM, Nathan Froyd wrote:
>> Jason commented on the TS_STATEMENT_LIST patch, but
>> the discussion didn't come to a resolution.
> 
> Right, from your last mail I thought that you were investigating my question
> about add_stmt and your suggestion about dropping the NULL checking in
> append_to_statement_list_1.

Ah, OK.  That was not clear to me.

I stand by what I said about add_stmt.  Before, if we weren't
building_stmt_tree (i.e. cur_stmt_list was NULL), we'd just pass a pointer to
NULL into append_to_statement_list and append_to_statement_list would DTRT.
Now if we have a NULL stack, then passing a pointer to the stack doesn't work,
because it's not typed correctly and we'd be clobbering the VEC, not pushing
onto the stack.

An alternative solution would be to initialize cur_stmt_list somewhere with an
actual 1-element VEC; the check in add_stmt would then be unnecessary, as we'd
always be assured of having someplace in the stack to store it.  I don't trust
myself to write a patch like that tonight; I'll twiddle with that tomorrow.

As to the NULL checking in append_to_statement_list_1, I was being hasty and
naive.  There are scores of places throughout the compiler that depend on this
behavior; I don't think it's worthwhile to change all the callers to ensure
the pointer-to-statement_list points to a non-NULL thing.

-Nathan

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-26  8:47   ` Nathan Froyd
@ 2011-05-26 14:00     ` Jason Merrill
  2011-05-26 14:34       ` Nathan Froyd
  2011-05-27  4:54       ` Nathan Froyd
  0 siblings, 2 replies; 12+ messages in thread
From: Jason Merrill @ 2011-05-26 14:00 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On 05/25/2011 10:21 PM, Nathan Froyd wrote:
> An alternative solution would be to initialize cur_stmt_list somewhere with an
> actual 1-element VEC;

Or just push NULL onto the stack and let append_to_statement_list_1 
allocate the VEC?

> the check in add_stmt would then be unnecessary, as we'd
> always be assured of having someplace in the stack to store it.  I don't trust
> myself to write a patch like that tonight; I'll twiddle with that tomorrow.

Right, that's what I was thinking about.  I think we should only need to 
do this once per function.

> behavior; I don't think it's worthwhile to change all the callers to ensure
> the pointer-to-statement_list points to a non-NULL thing.

Agreed.

Jason

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-26 14:00     ` Jason Merrill
@ 2011-05-26 14:34       ` Nathan Froyd
  2011-05-26 14:36         ` Jason Merrill
  2011-05-27  4:54       ` Nathan Froyd
  1 sibling, 1 reply; 12+ messages in thread
From: Nathan Froyd @ 2011-05-26 14:34 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On 05/26/2011 09:39 AM, Jason Merrill wrote:
> On 05/25/2011 10:21 PM, Nathan Froyd wrote:
>> An alternative solution would be to initialize cur_stmt_list somewhere with an
>> actual 1-element VEC;
> 
> Or just push NULL onto the stack and let append_to_statement_list_1 allocate
> the VEC?

Did you misspeak here?  append_to_statement_1 shouldn't be caring about VECs.
 Or do you mean pushing NULL_TREE someplace else, as in:

>> the check in add_stmt would then be unnecessary, as we'd
>> always be assured of having someplace in the stack to store it.  I don't trust
>> myself to write a patch like that tonight; I'll twiddle with that tomorrow.
> 
> Right, that's what I was thinking about.  I think we should only need to do
> this once per function.

...here?

-Nathan

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-26 14:34       ` Nathan Froyd
@ 2011-05-26 14:36         ` Jason Merrill
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Merrill @ 2011-05-26 14:36 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On 05/26/2011 09:46 AM, Nathan Froyd wrote:
> On 05/26/2011 09:39 AM, Jason Merrill wrote:
>> On 05/25/2011 10:21 PM, Nathan Froyd wrote:
>>> An alternative solution would be to initialize cur_stmt_list somewhere with an
>>> actual 1-element VEC;
>>
>> Or just push NULL onto the stack and let append_to_statement_list_1 allocate
>> the VEC?
>
> Did you misspeak here?  append_to_statement_1 shouldn't be caring about VECs.

Yes, I was confused.  I meant push NULL onto the VEC, which seems like 
what you meant too.

Jason

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-26 14:00     ` Jason Merrill
  2011-05-26 14:34       ` Nathan Froyd
@ 2011-05-27  4:54       ` Nathan Froyd
  2011-05-27  8:14         ` Jason Merrill
  1 sibling, 1 reply; 12+ messages in thread
From: Nathan Froyd @ 2011-05-27  4:54 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Thu, May 26, 2011 at 09:39:30AM -0400, Jason Merrill wrote:
> On 05/25/2011 10:21 PM, Nathan Froyd wrote:
> >An alternative solution would be to initialize cur_stmt_list somewhere with an
> >actual 1-element VEC;
> 
> Or just push NULL onto the stack and let append_to_statement_list_1
> allocate the VEC?
> 
> >the check in add_stmt would then be unnecessary, as we'd
> >always be assured of having someplace in the stack to store it.  I don't trust
> >myself to write a patch like that tonight; I'll twiddle with that tomorrow.
> 
> Right, that's what I was thinking about.  I think we should only
> need to do this once per function.

Sigh, I am an idiot.  It appears that we always have something pushed by
the time add_stmt is called.  (I ran into problems implementing the
above approach, as I wound up with [ NULL_TREE, <tree> ] and that gave
pop_stmt heartburn.)  I can't recall why I added the check in the first
place; it might have been because I ran into problems in the C FE and
blindly assumed I needed the same fix in the C++ FE.

Below is a revised patch with the dubious code taken out of add_stmt and
an assert added.  Bootstrap/testing in progress on
x86_64-unknown-linux-gnu, but we've built libstdc++ a couple of times,
so I don't anticipate any surprises.  OK to commit?

-Nathan

gcc/
	* c-decl.c (c_push_function_context): Copy the current statement
	list stack.
	(add_stmt): Check building_stmt_list_p and push_stmt if necessary.
	(finish_struct): Call building_stmt_list_p instead of checking
	cur_stmt_list.
	* c-parser.c (c_parser_postfix_expression): Likewise.
	* c-typeck.c (c_end_compound_stmt): Likewise.
	* print-tree.c (print_node) [STATEMENT_LIST]: Don't print TREE_CHAIN.
	* tree-iterator.c (stmt_list_cache): Change to a VEC.
	(alloc_stmt_list): Adjust for stmt_list_cache's new type.
	(free_stmt_list): Likewise.
	* tree.h (struct tree_statement_list): Include typed_tree instead
	of tree_common.
	* tree.c (initialize_tree_contains_struct): Mark TS_STATEMENT_LIST
	as TS_TYPED instead of TS_COMMON.

gcc/c-family/
	* c-common.h (struct stmt_tree_s) [x_cur_stmt_list]: Change to a VEC.
	(stmt_list_stack): Define.
	(cur_stmt_list): Adjust for new type of x_cur_stmt_list.
	* c-semantics.c (push_stmt_list, pop_stmt_list): Likewise.

gcc/cp/
	* cp-tree.h (building_stmt_tree): Delete.
	* decl.c (save_function_data): Tweak initializer for x_cur_stmt_list.
	(build_aggr_init_full_exprs): Call building_stmt_list_p
	instead of building_stmt_tree.
	(initialize_local_var): Likewise.
	(finish_function): Likewise.
	* decl2.c (finish_anon_union): Likewise.
	* init.c (begin_init_stmts): Likewise.
	(finish_init_stmts): Likewise.
	(expand_aggr_init_1): Likewise.
	* name-lookup.c (do_local_using_decl): Likewise.
	(do_namespace_alias): Likewise.
	(do_using_directive): Likewise.
	(cp_emit_debug_info_for_using): Likewise.
	* semantics.c (add_stmt): Assert that stmt_list_stack is non-empty.

diff --git a/gcc/c-decl.c b/gcc/c-decl.c
index a7cc965..1d43126 100644
--- a/gcc/c-decl.c
+++ b/gcc/c-decl.c
@@ -552,6 +552,8 @@ add_stmt (tree t)
 
   /* Add T to the statement-tree.  Non-side-effect statements need to be
      recorded during statement expressions.  */
+  if (!building_stmt_list_p ())
+    push_stmt_list ();
   append_to_statement_list_force (t, &cur_stmt_list);
 
   return t;
@@ -7188,7 +7190,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
   /* If we're inside a function proper, i.e. not file-scope and not still
      parsing parameters, then arrange for the size of a variable sized type
      to be bound now.  */
-  if (cur_stmt_list && variably_modified_type_p (t, NULL_TREE))
+  if (building_stmt_list_p () && variably_modified_type_p (t, NULL_TREE))
     add_stmt (build_stmt (loc,
 			  DECL_EXPR, build_decl (loc, TYPE_DECL, NULL, t)));
 
@@ -8424,6 +8426,8 @@ c_push_function_context (void)
   cfun->language = p;
 
   p->base.x_stmt_tree = c_stmt_tree;
+  c_stmt_tree.x_cur_stmt_list
+    = VEC_copy (tree, gc, c_stmt_tree.x_cur_stmt_list);
   p->x_break_label = c_break_label;
   p->x_cont_label = c_cont_label;
   p->x_switch_stack = c_switch_stack;
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 65f8ad1..1b658b1 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -452,8 +452,8 @@ typedef enum ref_operator {
 /* Information about a statement tree.  */
 
 struct GTY(()) stmt_tree_s {
-  /* The current statement list being collected.  */
-  tree x_cur_stmt_list;
+  /* A stack of statement lists being collected.  */
+  VEC(tree,gc) *x_cur_stmt_list;
 
   /* In C++, Nonzero if we should treat statements as full
      expressions.  In particular, this variable is no-zero if at the
@@ -483,11 +483,17 @@ struct GTY(()) c_language_function {
   struct stmt_tree_s x_stmt_tree;
 };
 
+#define stmt_list_stack (current_stmt_tree ()->x_cur_stmt_list)
+
 /* When building a statement-tree, this is the current statement list
-   being collected.  It's TREE_CHAIN is a back-pointer to the previous
-   statement list.  */
+   being collected.  We define it in this convoluted way, rather than
+   using VEC_last, because it must be an lvalue.  */
+
+#define cur_stmt_list							\
+  (*(VEC_address (tree, stmt_list_stack)				\
+     + VEC_length (tree, stmt_list_stack) - 1))
 
-#define cur_stmt_list (current_stmt_tree ()->x_cur_stmt_list)
+#define building_stmt_list_p() (!VEC_empty (tree, stmt_list_stack))
 
 /* Language-specific hooks.  */
 
diff --git a/gcc/c-family/c-semantics.c b/gcc/c-family/c-semantics.c
index a5bd9ba..cb0f2be 100644
--- a/gcc/c-family/c-semantics.c
+++ b/gcc/c-family/c-semantics.c
@@ -38,8 +38,7 @@ push_stmt_list (void)
 {
   tree t;
   t = alloc_stmt_list ();
-  TREE_CHAIN (t) = cur_stmt_list;
-  cur_stmt_list = t;
+  VEC_safe_push (tree, gc, stmt_list_stack, t);
   return t;
 }
 
@@ -48,21 +47,23 @@ push_stmt_list (void)
 tree
 pop_stmt_list (tree t)
 {
-  tree u = cur_stmt_list, chain;
+  tree u = NULL_TREE;
 
   /* Pop statement lists until we reach the target level.  The extra
      nestings will be due to outstanding cleanups.  */
   while (1)
     {
-      chain = TREE_CHAIN (u);
-      TREE_CHAIN (u) = NULL_TREE;
-      if (chain)
-	STATEMENT_LIST_HAS_LABEL (chain) |= STATEMENT_LIST_HAS_LABEL (u);
+      u = VEC_pop (tree, stmt_list_stack);
+      if (!VEC_empty (tree, stmt_list_stack))
+	{
+	  tree x = VEC_last (tree, stmt_list_stack);
+	  STATEMENT_LIST_HAS_LABEL (x) |= STATEMENT_LIST_HAS_LABEL (u);
+	}
       if (t == u)
 	break;
-      u = chain;
     }
-  cur_stmt_list = chain;
+
+  gcc_assert (u != NULL_TREE);
 
   /* If the statement list is completely empty, just return it.  This is
      just as good small as build_empty_stmt, with the advantage that
diff --git a/gcc/c-parser.c b/gcc/c-parser.c
index cb70bed..65966a9 100644
--- a/gcc/c-parser.c
+++ b/gcc/c-parser.c
@@ -6126,7 +6126,7 @@ c_parser_postfix_expression (c_parser *parser)
 	  c_parser_consume_token (parser);
 	  brace_loc = c_parser_peek_token (parser)->location;
 	  c_parser_consume_token (parser);
-	  if (cur_stmt_list == NULL)
+	  if (!building_stmt_list_p ())
 	    {
 	      error_at (loc, "braced-group within expression allowed "
 			"only inside a function");
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index e609611..a451606 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -9293,7 +9293,7 @@ c_end_compound_stmt (location_t loc, tree stmt, bool do_scope)
      do the wrong thing for ({ { 1; } }) or ({ 1; { } }).  In particular,
      STATEMENT_LISTs merge, and thus we can lose track of what statement
      was really last.  */
-  if (cur_stmt_list
+  if (building_stmt_list_p ()
       && STATEMENT_LIST_STMT_EXPR (cur_stmt_list)
       && TREE_CODE (stmt) != BIND_EXPR)
     {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ada01fb..6b0bd2b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -293,10 +293,6 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
 #define same_type_p(TYPE1, TYPE2) \
   comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
 
-/* Nonzero if we are presently building a statement tree, rather
-   than expanding each statement as we encounter it.  */
-#define building_stmt_tree()  (cur_stmt_list != NULL_TREE)
-
 /* Returns nonzero iff NODE is a declaration for the global function
    `main'.  */
 #define DECL_MAIN_P(NODE)				\
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index a956dbb..2ae937b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5352,13 +5352,13 @@ build_aggr_init_full_exprs (tree decl, tree init, int flags)
      
 {
   int saved_stmts_are_full_exprs_p = 0;
-  if (building_stmt_tree ())
+  if (building_stmt_list_p ())
     {
       saved_stmts_are_full_exprs_p = stmts_are_full_exprs_p ();
       current_stmt_tree ()->stmts_are_full_exprs_p = 1;
     }
   init = build_aggr_init (decl, init, flags, tf_warning_or_error);
-  if (building_stmt_tree ())
+  if (building_stmt_list_p ())
     current_stmt_tree ()->stmts_are_full_exprs_p =
       saved_stmts_are_full_exprs_p;
   return init;
@@ -5751,7 +5751,7 @@ initialize_local_var (tree decl, tree init)
 	  if (cleanup && TREE_CODE (type) != ARRAY_TYPE)
 	    wrap_temporary_cleanups (init, cleanup);
 
-	  gcc_assert (building_stmt_tree ());
+	  gcc_assert (building_stmt_list_p ());
 	  saved_stmts_are_full_exprs_p = stmts_are_full_exprs_p ();
 	  current_stmt_tree ()->stmts_are_full_exprs_p = 1;
 	  finish_expr_stmt (init);
@@ -12918,7 +12918,7 @@ save_function_data (tree decl)
   DECL_SAVED_FUNCTION_DATA (decl) = f;
 
   /* Clear out the bits we don't need.  */
-  f->base.x_stmt_tree.x_cur_stmt_list = NULL_TREE;
+  f->base.x_stmt_tree.x_cur_stmt_list = NULL;
   f->bindings = NULL;
   f->x_local_names = NULL;
 }
@@ -13163,7 +13163,7 @@ finish_function (int flags)
       This caused &foo to be of type ptr-to-const-function
       which then got a warning when stored in a ptr-to-function variable.  */
 
-  gcc_assert (building_stmt_tree ());
+  gcc_assert (building_stmt_list_p ());
   /* The current function is being defined, so its DECL_INITIAL should
      be set, and unless there's a multiple definition, it should be
      error_mark_node.  */
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 91a6644..9005f7e 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1437,7 +1437,7 @@ finish_anon_union (tree anon_union_decl)
     }
 
   pushdecl (anon_union_decl);
-  if (building_stmt_tree ()
+  if (building_stmt_list_p ()
       && at_function_scope_p ())
     add_decl_expr (anon_union_decl);
   else if (!processing_template_decl)
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 6336dd7..d92dacc 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -61,7 +61,7 @@ static int diagnose_uninitialized_cst_or_ref_member_1 (tree, tree, bool, bool);
 static bool
 begin_init_stmts (tree *stmt_expr_p, tree *compound_stmt_p)
 {
-  bool is_global = !building_stmt_tree ();
+  bool is_global = !building_stmt_list_p ();
 
   *stmt_expr_p = begin_stmt_expr ();
   *compound_stmt_p = begin_compound_stmt (BCS_NO_SCOPE);
@@ -79,7 +79,7 @@ finish_init_stmts (bool is_global, tree stmt_expr, tree compound_stmt)
 
   stmt_expr = finish_stmt_expr (stmt_expr, true);
 
-  gcc_assert (!building_stmt_tree () == is_global);
+  gcc_assert (!building_stmt_list_p () == is_global);
 
   return stmt_expr;
 }
@@ -1552,7 +1552,7 @@ expand_aggr_init_1 (tree binfo, tree true_exp, tree exp, tree init, int flags,
   tree type = TREE_TYPE (exp);
 
   gcc_assert (init != error_mark_node && type != error_mark_node);
-  gcc_assert (building_stmt_tree ());
+  gcc_assert (building_stmt_list_p ());
 
   /* Use a function returning the desired type to initialize EXP for us.
      If the function is a constructor, and its first argument is
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 3d07ff6..0e762fd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2475,7 +2475,7 @@ do_local_using_decl (tree decl, tree scope, tree name)
   if (decl == NULL_TREE)
     return;
 
-  if (building_stmt_tree ()
+  if (building_stmt_list_p ()
       && at_function_scope_p ())
     add_decl_expr (decl);
 
@@ -3570,7 +3570,7 @@ do_namespace_alias (tree alias, tree name_space)
   pushdecl (alias);
 
   /* Emit debug info for namespace alias.  */
-  if (!building_stmt_tree ())
+  if (!building_stmt_list_p ())
     (*debug_hooks->global_decl) (alias);
 }
 
@@ -3718,7 +3718,7 @@ do_using_directive (tree name_space)
 
   gcc_assert (TREE_CODE (name_space) == NAMESPACE_DECL);
 
-  if (building_stmt_tree ())
+  if (building_stmt_list_p ())
     add_stmt (build_stmt (input_location, USING_STMT, name_space));
   name_space = ORIGINAL_NAMESPACE (name_space);
 
@@ -5890,7 +5890,7 @@ cp_emit_debug_info_for_using (tree t, tree context)
   for (t = OVL_CURRENT (t); t; t = OVL_NEXT (t))
     if (TREE_CODE (t) != TEMPLATE_DECL)
       {
-	if (building_stmt_tree ())
+	if (building_stmt_list_p ())
 	  add_stmt (build_stmt (input_location, USING_STMT, t));
 	else
 	  (*debug_hooks->imported_module_or_decl) (t, NULL_TREE, context, false);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 7833d76..557bf4c 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -393,6 +393,7 @@ add_stmt (tree t)
 
   /* Add T to the statement-tree.  Non-side-effect statements need to be
      recorded during statement expressions.  */
+  gcc_checking_assert (!VEC_empty (tree, stmt_list_stack));
   append_to_statement_list_force (t, &cur_stmt_list);
 
   return t;
diff --git a/gcc/print-tree.c b/gcc/print-tree.c
index 58c9613..1a1e33f 100644
--- a/gcc/print-tree.c
+++ b/gcc/print-tree.c
@@ -911,7 +911,6 @@ print_node (FILE *file, const char *prefix, tree node, int indent)
 		print_node (file, "stmt", tsi_stmt (i), indent + 4);
 	      }
 	  }
-	  print_node (file, "chain", TREE_CHAIN (node), indent + 4);
 	  break;
 
 	case BLOCK:
diff --git a/gcc/tree-iterator.c b/gcc/tree-iterator.c
index 05764da..44b6bed 100644
--- a/gcc/tree-iterator.c
+++ b/gcc/tree-iterator.c
@@ -31,17 +31,16 @@ along with GCC; see the file COPYING3.  If not see
 /* This is a cache of STATEMENT_LIST nodes.  We create and destroy them
    fairly often during gimplification.  */
 
-static GTY ((deletable (""))) tree stmt_list_cache;
+static GTY ((deletable (""))) VEC(tree,gc) *stmt_list_cache;
 
 tree
 alloc_stmt_list (void)
 {
-  tree list = stmt_list_cache;
-  if (list)
+  tree list;
+  if (!VEC_empty (tree, stmt_list_cache))
     {
-      stmt_list_cache = TREE_CHAIN (list);
-      gcc_assert (stmt_list_cache != list);
-      memset (list, 0, sizeof(struct tree_common));
+      list = VEC_pop (tree, stmt_list_cache);
+      memset (list, 0, sizeof(struct tree_base));
       TREE_SET_CODE (list, STATEMENT_LIST);
     }
   else
@@ -55,11 +54,7 @@ free_stmt_list (tree t)
 {
   gcc_assert (!STATEMENT_LIST_HEAD (t));
   gcc_assert (!STATEMENT_LIST_TAIL (t));
-  /* If this triggers, it's a sign that the same list is being freed
-     twice.  */
-  gcc_assert (t != stmt_list_cache || stmt_list_cache == NULL);
-  TREE_CHAIN (t) = stmt_list_cache;
-  stmt_list_cache = t;
+  VEC_safe_push (tree, gc, stmt_list_cache, t);
 }
 
 /* A subroutine of append_to_statement_list{,_force}.  T is not NULL.  */
diff --git a/gcc/tree.c b/gcc/tree.c
index 178873c..85c0516 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -383,6 +383,7 @@ initialize_tree_contains_struct (void)
 	case TS_CONSTRUCTOR:
 	case TS_EXP:
 	case TS_IDENTIFIER:
+	case TS_STATEMENT_LIST:
 	  MARK_TS_TYPED (code);
 	  break;
 
@@ -391,7 +392,6 @@ initialize_tree_contains_struct (void)
 	case TS_LIST:
 	case TS_VEC:
 	case TS_BINFO:
-	case TS_STATEMENT_LIST:
 	case TS_OMP_CLAUSE:
 	case TS_OPTIMIZATION:
 	case TS_TARGET_OPTION:
diff --git a/gcc/tree.h b/gcc/tree.h
index e214b12..f63764a 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3556,7 +3556,7 @@ struct GTY ((chain_next ("%h.next"), chain_prev ("%h.prev"))) tree_statement_lis
 
 struct GTY(()) tree_statement_list
  {
-  struct tree_common common;
+  struct tree_typed typed;
   struct tree_statement_list_node *head;
   struct tree_statement_list_node *tail;
 };

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

* Re: [PATCH PING] unreviewed tree-slimming patches
  2011-05-27  4:54       ` Nathan Froyd
@ 2011-05-27  8:14         ` Jason Merrill
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Merrill @ 2011-05-27  8:14 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On 05/26/2011 08:50 PM, Nathan Froyd wrote:
> Sigh, I am an idiot.  It appears that we always have something pushed by
> the time add_stmt is called.  (I ran into problems implementing the
> above approach, as I wound up with [ NULL_TREE,<tree>  ] and that gave
> pop_stmt heartburn.)  I can't recall why I added the check in the first
> place; it might have been because I ran into problems in the C FE and
> blindly assumed I needed the same fix in the C++ FE.

A reasonable assumption; I wonder why C is doing things differently.

The patch is OK, but could you look into that inconsistency?

Jason

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

end of thread, other threads:[~2011-05-27  4:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 14:33 [PATCH PING] unreviewed tree-slimming patches Nathan Froyd
2011-05-25 14:40 ` Jason Merrill
2011-05-26  8:47   ` Nathan Froyd
2011-05-26 14:00     ` Jason Merrill
2011-05-26 14:34       ` Nathan Froyd
2011-05-26 14:36         ` Jason Merrill
2011-05-27  4:54       ` Nathan Froyd
2011-05-27  8:14         ` Jason Merrill
2011-05-25 14:42 ` Richard Guenther
2011-05-25 17:44 ` Joseph S. Myers
2011-05-25 19:31 ` Tom Tromey
2011-05-25 19:36   ` Nathan Froyd

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