Re: [PATCH, TREE_LIST removal] remove calls to build_constructor_from_list from coverage.c

[Nathan Froyd <froydnj at codesourcery dot com>]

On Tue, May 11, 2010 at 10:58:33PM +0200, Steven Bosscher wrote:
> On Tue, May 11, 2010 at 7:19 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > The patch below ports over a couple of patches from the oldlto branch to
> > eliminate calls to build_constructor_from_list in coverage.c.  The port
> > is slightly modernized by the use of CONSTRUCTOR_APPEND_ELT.
> >
> > Testing in progress on x86-64-linux-gnu; will commit after testing
> > finishes.
> >
> > -Nathan
> >
> >    * coverage.c (build_fn_info_value): Call build_constructor instead of
> >    build_constructor_from_list.
> >    (build_ctr_info_value): Likewise.
> >    (build_gcov_info): Likewise.
> 
> Please track revisions you port in http://gcc.gnu.org/PR43977.

Ack, will do.  I see that you already tackled this bit.

> I already tried a few of these patches. They don't pass bootstrap. We
> need to figure out an interface for pushing constructor elements and
> calculating TREE_CONSTANT on the fly -- or modify build_constructor to
> just always iterate over all elements and compute TREE_CONSTANT there
> for everything that passes through there.

Pushing the computation of TREE_CONSTANT down into build_constructor
seems reasonable enough--eventually build_constructor_from_list will go
away and we'd have to compute it there anyway.  I see a couple of places
where a front-end directly sets TREE_CONSTANT on the result of
build_constructor...but I don't think it's worth trying to split
build_constructor into a variant that sets TREE_CONSTANT and one that
doesn't.

Does the following patch help bootstrap your patches?  It passes C/C++
bootstrap here (x86-64 Linux); I'm going to try and do a full language
bootstrap overnight.

-Nathan

	* tree.c (build_constructor): Compute TREE_CONSTANT for the
	resultant constructor.
	(build_constructor_single): Don't set TREE_CONSTANT.
	(build_constructor_from_list): Don't compute TREE_CONSTANT.

Index: tree.c
===================================================================
--- tree.c	(revision 159280)
+++ tree.c	(working copy)
@@ -1332,8 +1332,19 @@ tree
 build_constructor (tree type, VEC(constructor_elt,gc) *vals)
 {
   tree c = make_node (CONSTRUCTOR);
+  unsigned int i;
+  constructor_elt *elt;
+  bool constant_p = true;
+
   TREE_TYPE (c) = type;
   CONSTRUCTOR_ELTS (c) = vals;
+
+  for (i = 0; VEC_iterate (constructor_elt, vals, i, elt); i++)
+    if (!TREE_CONSTANT (elt->value))
+      constant_p = false;
+
+  TREE_CONSTANT (c) = constant_p;
+
   return c;
 }
 
@@ -1344,16 +1355,13 @@ build_constructor_single (tree type, tre
 {
   VEC(constructor_elt,gc) *v;
   constructor_elt *elt;
-  tree t;
 
   v = VEC_alloc (constructor_elt, gc, 1);
   elt = VEC_quick_push (constructor_elt, v, NULL);
   elt->index = index;
   elt->value = value;
 
-  t = build_constructor (type, v);
-  TREE_CONSTANT (t) = TREE_CONSTANT (value);
-  return t;
+  return build_constructor (type, v);
 }
 
 
@@ -1362,27 +1370,17 @@ build_constructor_single (tree type, tre
 tree
 build_constructor_from_list (tree type, tree vals)
 {
-  tree t, val;
+  tree t;
   VEC(constructor_elt,gc) *v = NULL;
-  bool constant_p = true;
 
   if (vals)
     {
       v = VEC_alloc (constructor_elt, gc, list_length (vals));
       for (t = vals; t; t = TREE_CHAIN (t))
-	{
-	  constructor_elt *elt = VEC_quick_push (constructor_elt, v, NULL);
-	  val = TREE_VALUE (t);
-	  elt->index = TREE_PURPOSE (t);
-	  elt->value = val;
-	  if (!TREE_CONSTANT (val))
-	    constant_p = false;
-	}
+	CONSTRUCTOR_APPEND_ELT (v, TREE_PURPOSE (t), TREE_VALUE (t));
     }
 
-  t = build_constructor (type, v);
-  TREE_CONSTANT (t) = constant_p;
-  return t;
+  return build_constructor (type, v);
 }
 
 /* Return a new FIXED_CST node whose type is TYPE and value is F.  */