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;
>
>
>
--
Richard Guenther <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
