Fix PR52977

Hi,

On Mon, 16 Apr 2012, Richard Guenther wrote:

> This fixes PR52977 - for PCH to work with its pointer relocation code we 
> have to avoid dereferencing pointers to compute array lengths in 
> structures.  So we have to unfortunately keep duplicated info about 
> VECTOR_CST vector lengths.

That's a pity and caused me to look a bit at gengtype.  We can do better, 
and compute the lengths before all fields are accessed (and mangled in the 
PCH case).  With the patch below we emit such code:

        case TS_VECTOR:
          {
            size_t l0 = (size_t)(TYPE_VECTOR_SUBPARTS (TREE_TYPE ((tree)&((*x).generic.vector))));
            if ((void *)(x) == this_obj)
              op (&((*x).generic.vector.typed.type), cookie);
            {
              size_t i0;
              for (i0 = 0; i0 != l0; i0++) {
                if ((void *)(x) == this_obj)
                  op (&((*x).generic.vector.elts[i0]), cookie);
              }
            }
          }

Note how .generic.vector.typed.type is only mangled after TREE_TYPE is 
used to calculate the length.

I regstrapped this on x86_64-linux, all languages, no regression.  Okay 
for trunk (saving the all-important memory for vector constants again :) 
)?


Ciao,
Michael.
------------------
	PR middle-end/52977
	* tree.h (VECTOR_CST_NELTS): Use part number of types again.
	(struct tree_vector): Adjust GTY length.
	* tree.c (make_vector_stat): Don't set VECTOR_CST_NELTS.

	* gengtype.c (struct walk_type_data): Add in_record_p and loopcounter
	members.
	(walk_type, <TYPE_POINTER, TYPE_ARRAY>): Handle case where our
	caller emitted the length calulation already.
	(walk_type, <TYPE_UNION, TYPE_STRUCT>): Emit length calculations
	before handling any of the fields for structs.

On Thu, 19 Apr 2012, Michael Matz wrote:

> Hi,
> 
> On Mon, 16 Apr 2012, Richard Guenther wrote:
> 
> > This fixes PR52977 - for PCH to work with its pointer relocation code we 
> > have to avoid dereferencing pointers to compute array lengths in 
> > structures.  So we have to unfortunately keep duplicated info about 
> > VECTOR_CST vector lengths.
> 
> That's a pity and caused me to look a bit at gengtype.  We can do better, 
> and compute the lengths before all fields are accessed (and mangled in the 
> PCH case).  With the patch below we emit such code:
> 
>         case TS_VECTOR:
>           {
>             size_t l0 = (size_t)(TYPE_VECTOR_SUBPARTS (TREE_TYPE ((tree)&((*x).generic.vector))));
>             if ((void *)(x) == this_obj)
>               op (&((*x).generic.vector.typed.type), cookie);
>             {
>               size_t i0;
>               for (i0 = 0; i0 != l0; i0++) {
>                 if ((void *)(x) == this_obj)
>                   op (&((*x).generic.vector.elts[i0]), cookie);
>               }
>             }
>           }
> 
> Note how .generic.vector.typed.type is only mangled after TREE_TYPE is 
> used to calculate the length.
> 
> I regstrapped this on x86_64-linux, all languages, no regression.  Okay 
> for trunk (saving the all-important memory for vector constants again :) 
> )?

Heh.  Not for the memory usage but for the non-duplicate info.

Ok.

Thanks,
Richard.

> 
> Ciao,
> Michael.
> ------------------
> 	PR middle-end/52977
> 	* tree.h (VECTOR_CST_NELTS): Use part number of types again.
> 	(struct tree_vector): Adjust GTY length.
> 	* tree.c (make_vector_stat): Don't set VECTOR_CST_NELTS.
> 
> 	* gengtype.c (struct walk_type_data): Add in_record_p and loopcounter
> 	members.
> 	(walk_type, <TYPE_POINTER, TYPE_ARRAY>): Handle case where our
> 	caller emitted the length calulation already.
> 	(walk_type, <TYPE_UNION, TYPE_STRUCT>): Emit length calculations
> 	before handling any of the fields for structs.
> 
> Index: tree.h
> ===================================================================
> --- tree.h	(revision 186580)
> +++ tree.h	(working copy)
> @@ -1534,14 +1534,13 @@ struct GTY(()) tree_complex {
>  };
>  
>  /* In a VECTOR_CST node.  */
> -#define VECTOR_CST_NELTS(NODE) (VECTOR_CST_CHECK (NODE)->vector.length)
> +#define VECTOR_CST_NELTS(NODE) (TYPE_VECTOR_SUBPARTS (TREE_TYPE (NODE)))
>  #define VECTOR_CST_ELTS(NODE) (VECTOR_CST_CHECK (NODE)->vector.elts)
>  #define VECTOR_CST_ELT(NODE,IDX) (VECTOR_CST_CHECK (NODE)->vector.elts[IDX])
>  
>  struct GTY(()) tree_vector {
>    struct tree_typed typed;
> -  unsigned length;
> -  tree GTY ((length ("%h.length"))) elts[1];
> +  tree GTY ((length ("TYPE_VECTOR_SUBPARTS (TREE_TYPE ((tree)&%h))"))) elts[1];
>  };
>  
>  #include "symtab.h"
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 186580)
> +++ tree.c	(working copy)
> @@ -1329,7 +1329,6 @@ make_vector_stat (unsigned len MEM_STAT_
>  
>    TREE_SET_CODE (t, VECTOR_CST);
>    TREE_CONSTANT (t) = 1;
> -  VECTOR_CST_NELTS (t) = len;
>  
>    return t;
>  }
> Index: gengtype.c
> ===================================================================
> --- gengtype.c	(revision 186580)
> +++ gengtype.c	(working copy)
> @@ -2291,6 +2291,8 @@ struct walk_type_data
>    const char *reorder_fn;
>    bool needs_cast_p;
>    bool fn_wants_lvalue;
> +  bool in_record_p;
> +  int loopcounter;
>  };
>  
>  /* Print a mangled name representing T to OF.  */
> @@ -2592,7 +2594,7 @@ walk_type (type_p t, struct walk_type_da
>  	  }
>  	else
>  	  {
> -	    int loopcounter = d->counter++;
> +	    int loopcounter = d->loopcounter;
>  	    const char *oldval = d->val;
>  	    const char *oldprevval3 = d->prev_val[3];
>  	    char *newval;
> @@ -2602,7 +2604,10 @@ walk_type (type_p t, struct walk_type_da
>  	    oprintf (d->of, "%*ssize_t i%d;\n", d->indent, "", loopcounter);
>  	    oprintf (d->of, "%*sfor (i%d = 0; i%d != (size_t)(", d->indent,
>  		     "", loopcounter, loopcounter);
> -	    output_escaped_param (d, length, "length");
> +	    if (!d->in_record_p)
> +	      output_escaped_param (d, length, "length");
> +	    else
> +	      oprintf (d->of, "l%d", loopcounter);
>  	    oprintf (d->of, "); i%d++) {\n", loopcounter);
>  	    d->indent += 2;
>  	    d->val = newval = xasprintf ("%s[i%d]", oldval, loopcounter);
> @@ -2624,7 +2629,7 @@ walk_type (type_p t, struct walk_type_da
>  
>      case TYPE_ARRAY:
>        {
> -	int loopcounter = d->counter++;
> +	int loopcounter;
>  	const char *oldval = d->val;
>  	char *newval;
>  
> @@ -2633,6 +2638,11 @@ walk_type (type_p t, struct walk_type_da
>  	if (t->u.a.p->kind == TYPE_SCALAR)
>  	  break;
>  
> +	if (length)
> +	  loopcounter = d->loopcounter;
> +	else
> +	  loopcounter = d->counter++;
> +
>  	/* When walking an array, compute the length and store it in a
>  	   local variable before walking the array elements, instead of
>  	   recomputing the length expression each time through the loop.
> @@ -2643,13 +2653,16 @@ walk_type (type_p t, struct walk_type_da
>  	oprintf (d->of, "%*s{\n", d->indent, "");
>  	d->indent += 2;
>  	oprintf (d->of, "%*ssize_t i%d;\n", d->indent, "", loopcounter);
> -	oprintf (d->of, "%*ssize_t l%d = (size_t)(",
> -		 d->indent, "", loopcounter);
> -	if (length)
> -	  output_escaped_param (d, length, "length");
> -	else
> -	  oprintf (d->of, "%s", t->u.a.len);
> -	oprintf (d->of, ");\n");
> +	if (!d->in_record_p || !length)
> +	  {
> +	    oprintf (d->of, "%*ssize_t l%d = (size_t)(",
> +		     d->indent, "", loopcounter);
> +	    if (length)
> +	      output_escaped_param (d, length, "length");
> +	    else
> +	      oprintf (d->of, "%s", t->u.a.len);
> +	    oprintf (d->of, ");\n");
> +	  }
>  
>  	oprintf (d->of, "%*sfor (i%d = 0; i%d != l%d; i%d++) {\n",
>  		 d->indent, "",
> @@ -2678,6 +2691,9 @@ walk_type (type_p t, struct walk_type_da
>  	const int union_p = t->kind == TYPE_UNION;
>  	int seen_default_p = 0;
>  	options_p o;
> +	int lengths_seen = 0;
> +	int endcounter;
> +	bool any_length_seen = false;
>  
>  	if (!t->u.s.line.file)
>  	  error_at_line (d->line, "incomplete structure `%s'", t->u.s.tag);
> @@ -2713,6 +2729,45 @@ walk_type (type_p t, struct walk_type_da
>  	    d->indent += 2;
>  	    oprintf (d->of, "%*s{\n", d->indent, "");
>  	  }
> +
> +	for (f = t->u.s.fields; f; f = f->next)
> +	  {
> +	    options_p oo;
> +	    int skip_p = 0;
> +	    const char *fieldlength = NULL;
> +
> +	    d->reorder_fn = NULL;
> +	    for (oo = f->opt; oo; oo = oo->next)
> +	      if (strcmp (oo->name, "skip") == 0)
> +		skip_p = 1;
> +	      else if (strcmp (oo->name, "length") == 0
> +		       && oo->kind == OPTION_STRING)
> +		fieldlength = oo->info.string;
> +
> +	    if (skip_p)
> +	      continue;
> +	    if (fieldlength)
> +	      {
> +	        lengths_seen++;
> +		d->counter++;
> +		if (!union_p)
> +		  {
> +		    if (!any_length_seen)
> +		      {
> +			oprintf (d->of, "%*s{\n", d->indent, "");
> +			d->indent += 2;
> +		      }
> +		    any_length_seen = true;
> +
> +		    oprintf (d->of, "%*ssize_t l%d = (size_t)(",
> +			     d->indent, "", d->counter - 1);
> +		    output_escaped_param (d, fieldlength, "length");
> +		    oprintf (d->of, ");\n");
> +		  }
> +	      }
> +	  }
> +	endcounter = d->counter;
> +
>  	for (f = t->u.s.fields; f; f = f->next)
>  	  {
>  	    options_p oo;
> @@ -2721,6 +2776,7 @@ walk_type (type_p t, struct walk_type_da
>  	    int skip_p = 0;
>  	    int default_p = 0;
>  	    int use_param_p = 0;
> +	    const char *fieldlength = NULL;
>  	    char *newval;
>  
>  	    d->reorder_fn = NULL;
> @@ -2741,6 +2797,9 @@ walk_type (type_p t, struct walk_type_da
>  	      else if (strncmp (oo->name, "use_param", 9) == 0
>  		       && (oo->name[9] == '\0' || ISDIGIT (oo->name[9])))
>  		use_param_p = 1;
> +	      else if (strcmp (oo->name, "length") == 0
> +		       && oo->kind == OPTION_STRING)
> +		fieldlength = oo->info.string;
>  
>  	    if (skip_p)
>  	      continue;
> @@ -2774,16 +2833,24 @@ walk_type (type_p t, struct walk_type_da
>  			     "field `%s' is missing `tag' or `default' option",
>  			     f->name);
>  
> +	    if (fieldlength)
> +	      {
> +		d->loopcounter = endcounter - lengths_seen--;
> +	      }
> +
>  	    d->line = &f->line;
>  	    d->val = newval = xasprintf ("%s%s%s", oldval, dot, f->name);
>  	    d->opt = f->opt;
>  	    d->used_length = false;
> +	    d->in_record_p = !union_p;
>  
>  	    if (union_p && use_param_p && d->param == NULL)
>  	      oprintf (d->of, "%*sgcc_unreachable ();\n", d->indent, "");
>  	    else
>  	      walk_type (f->type, d);
>  
> +	    d->in_record_p = false;
> +
>  	    free (newval);
>  
>  	    if (union_p)
> @@ -2808,6 +2875,11 @@ walk_type (type_p t, struct walk_type_da
>  	    oprintf (d->of, "%*s}\n", d->indent, "");
>  	    d->indent -= 2;
>  	  }
> +	if (any_length_seen)
> +	  {
> +	    d->indent -= 2;
> +	    oprintf (d->of, "%*s}\n", d->indent, "");
> +	  }
>        }
>        break;
>  
> 
>