On Mon, 2014-05-05 at 10:18 -0600, Jeff Law wrote: > On 04/30/14 21:07, David Malcolm wrote: > > Currently, gengtype does not support template arguments that are > > explicitly pointers, such as: > > static GTY(()) vec<gimple_statement_base *> test_gimple; giving this > > error: > > ../../src/gcc/gimple-expr.c:902: parse error: expected a string constant > > or ',', have '*' instead > > requiring them to be typedefs. > > > > This patch removes this restriction, supporting up to a single pointer > > in each template argument. > > > > It also adds support for inheritance, allowing: > > static GTY(()) vec<gimple_statement_phi *, va_gc> *test_phis; > > where the generated gt-FOO.c file contains: > > > > void > > gt_ggc_mx (struct gimple_statement_phi *& x) > > { > > if (x) > > gt_ggc_mx_gimple_statement_base ((void *) x); > > } > > > > i.e. handling the subclass by calling the marking function for the base > > class. > > > > Doing so uncovered an issue within write_user_func_for_structure_ptr, > > which would unconditionally write out a func. This would lead to > > gt-FOO.c containing duplicate functions e.g.: > > gtype-desc.c:280: multiple definition of > > `gt_ggc_mx(gimple_statement_base*&)' > > if more than one GTY template type referenced such a type; for example like > > this: > > > > static GTY(()) vec<gimple, va_gc> *test_old_style_gimple; > > static GTY(()) vec<gimple_statement_base *, va_gc> > > *test_new_style_gimple; > > > > where "gimple" is just an alias for "gimple_statement_base *". This > > could be worked around by ensuring that the source tree consistently > > uses vec<> of just one kind, but for robustness the patch also makes > > write_user_func_for_structure_ptr idempotent, so it only writes out the > > func once for each (struct, which-of-ggc/pch) combination. > > > > Motivation: > > In the "Compile-time gimple-checking" discussion, Richi asked me to look > > at eliminating the const_gimple typedef in favor of having "gimple" be > > the struct, thus making the pointer to a gimple be explicit in the type: > > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html > > as in: > > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01632.html > >> [...] And I never liked the > >> const_ typedef variants that were introduced. The main reason for > >> them was probably to avoid all the churn of replacing the tree pointer > >> typedef with a tree (non-pointer) typedef. The mistake was to > >> repeat that for 'gimple' ... > > > > I'm attempting a patch for that, but in the meantime, this patch adds > > the needed support to gengtype. > > > > Successfully bootstrapped®tested on x86_64-unknown-linux-gnu (Fedora > > 20). > > > > OK for trunk? > Basically OK. Per the wide-int folks request, please hold off until the > wide-int merge is done. > > If after wide-int, the only changes are trivial, then consider the patch > pre-approved (post the final version here so that its archived). > > If after wide-int merges the changes to this patch are non-trivial, then > repost the RFA. > > Obviously, you get to exercise some judgement as to what constitutes > trivial vs non-trivial.
Thanks. The changes required after the wide-int merge were trivial. Successfully rebootstrapped®rtested the patch, both individually, and followed up with v4 of the proposed gimple class patch series from: http://gcc.gnu.org/ml/gcc-patches/2014-05/msg00887.html in both cases with equal testcase results compared to a control build of r210222. Committed to trunk as r210379; attached for reference.
Index: gcc/gengtype.c =================================================================== --- gcc/gengtype.c (revision 210378) +++ gcc/gengtype.c (revision 210379) @@ -145,6 +145,14 @@ s = s->u.s.base_class; return s; } + +static type_p +get_ultimate_base_class (type_p s) +{ + while (s->u.s.base_class) + s = s->u.s.base_class; + return s; +} /* Input file handling. */ @@ -589,7 +597,7 @@ /* We only accept simple template declarations (see require_template_declaration), so we only need to parse a comma-separated list of strings, implicitly assumed to - be type names. */ + be type names, potentially with "*" characters. */ char *arg = open_bracket + 1; char *type_id = strtok (arg, ",>"); pair_p fields = 0; @@ -597,8 +605,28 @@ { /* Create a new field for every type found inside the template parameter list. */ - const char *field_name = xstrdup (type_id); - type_p arg_type = resolve_typedef (field_name, pos); + + /* Support a single trailing "*" character. */ + const char *star = strchr (type_id, '*'); + int is_ptr = (star != NULL); + size_t offset_to_star = star - type_id; + if (is_ptr) + offset_to_star = star - type_id; + + char *field_name = xstrdup (type_id); + + type_p arg_type; + if (is_ptr) + { + /* Strip off the first '*' character (and any subsequent text). */ + *(field_name + offset_to_star) = '\0'; + + arg_type = find_structure (field_name, TYPE_STRUCT); + arg_type = create_pointer (arg_type); + } + else + arg_type = resolve_typedef (field_name, pos); + fields = create_field_at (fields, arg_type, field_name, 0, pos); type_id = strtok (0, ",>"); } @@ -2461,6 +2489,7 @@ const char *reorder_note_routine; const char *comment; int skip_hooks; /* skip hook generation if non zero */ + enum write_types_kinds kind; }; static void output_escaped_param (struct walk_type_data *d, @@ -2537,7 +2566,8 @@ size_t i; char *s = xstrdup (type_name); for (i = 0; i < strlen (s); i++) - if (s[i] == '<' || s[i] == '>' || s[i] == ':' || s[i] == ',') + if (s[i] == '<' || s[i] == '>' || s[i] == ':' || s[i] == ',' + || s[i] == '*') s[i] = '_'; return s; } @@ -3501,10 +3531,10 @@ /* Write on OF a user-callable routine to act as an entry point for the marking routine for S, generated by write_func_for_structure. - PREFIX is the prefix to use to distinguish ggc and pch markers. */ + WTD distinguishes between ggc and pch markers. */ static void -write_user_func_for_structure_ptr (outf_p of, type_p s, const char *prefix) +write_user_func_for_structure_ptr (outf_p of, type_p s, const write_types_data *wtd) { /* Parameterized structures are not supported in user markers. There is no way for the marker function to know which specific type @@ -3534,13 +3564,23 @@ break; } + DBGPRINTF ("write_user_func_for_structure_ptr: %s %s", s->u.s.tag, + wtd->prefix); + + /* Only write the function once. */ + if (s->u.s.wrote_user_func_for_ptr[wtd->kind]) + return; + s->u.s.wrote_user_func_for_ptr[wtd->kind] = true; + oprintf (of, "\nvoid\n"); - oprintf (of, "gt_%sx (", prefix); + oprintf (of, "gt_%sx (", wtd->prefix); write_type_decl (of, s); oprintf (of, " *& x)\n"); oprintf (of, "{\n"); oprintf (of, " if (x)\n "); - write_marker_function_name (of, alias_of ? alias_of : s, prefix); + write_marker_function_name (of, + alias_of ? alias_of : get_ultimate_base_class (s), + wtd->prefix); oprintf (of, " ((void *) x);\n"); oprintf (of, "}\n"); } @@ -3578,7 +3618,8 @@ which just marks the fields of T. */ static void -write_user_marking_functions (type_p s, const char *prefix, +write_user_marking_functions (type_p s, + const write_types_data *w, struct walk_type_data *d) { gcc_assert (s->kind == TYPE_USER_STRUCT); @@ -3590,10 +3631,10 @@ { type_p pointed_to_type = fld_type->u.p; if (union_or_struct_p (pointed_to_type)) - write_user_func_for_structure_ptr (d->of, pointed_to_type, prefix); + write_user_func_for_structure_ptr (d->of, pointed_to_type, w); } else if (union_or_struct_p (fld_type)) - write_user_func_for_structure_body (fld_type, prefix, d); + write_user_func_for_structure_body (fld_type, w->prefix, d); } } @@ -3791,7 +3832,7 @@ oprintf (d.of, "}\n"); if (orig_s->kind == TYPE_USER_STRUCT) - write_user_marking_functions (orig_s, wtd->prefix, &d); + write_user_marking_functions (orig_s, wtd, &d); } @@ -3969,7 +4010,7 @@ static const struct write_types_data ggc_wtd = { "ggc_m", NULL, "ggc_mark", "ggc_test_and_set_mark", NULL, "GC marker procedures. ", - FALSE + FALSE, WTK_GGC }; static const struct write_types_data pch_wtd = { @@ -3976,7 +4017,7 @@ "pch_n", "pch_p", "gt_pch_note_object", "gt_pch_note_object", "gt_pch_note_reorder", "PCH type-walking procedures. ", - TRUE + TRUE, WTK_PCH }; /* Write out the local pointer-walking routines. */ Index: gcc/gengtype.h =================================================================== --- gcc/gengtype.h (revision 210378) +++ gcc/gengtype.h (revision 210379) @@ -127,8 +127,16 @@ extern type_p param_structs; extern pair_p variables; +/* An enum for distinguishing GGC vs PCH. */ +enum write_types_kinds +{ + WTK_GGC, + WTK_PCH, + NUM_WTK +}; + /* Discrimating kind of types we can understand. */ enum typekind { @@ -302,6 +310,10 @@ type_p first_subclass; /* The next in that list. */ type_p next_sibling_class; + + /* Have we already written ggc/pch user func for ptr to this? + (in write_user_func_for_structure_ptr). */ + bool wrote_user_func_for_ptr[NUM_WTK]; } s; /* when TYPE_SCALAR: */ Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 210378) +++ gcc/ChangeLog (revision 210379) @@ -1,3 +1,32 @@ +2014-05-13 David Malcolm <dmalc...@redhat.com> + + * gengtype-parse.c (require3): Eliminate in favor of... + (require4): New. + (require_template_declaration): Update to support optional single * + on a type. + + * gengtype.c (get_ultimate_base_class): Add a non-const overload. + (create_user_defined_type): Handle a single level of explicit + pointerness within template arguments. + (struct write_types_data): Add field "kind". + (filter_type_name): Handle "*" character. + (write_user_func_for_structure_ptr): Require a write_types_data + rather than just a prefix string, so that we can look up the kind + of the wtd and use it as an index into wrote_user_func_for_ptr, + ensuring that such functions are written at most once. Support + subclasses by invoking the marking function of the ultimate base + class. + (write_user_func_for_structure_body): Require a write_types_data + rather than just a prefix string, so that we can pass this to + write_user_func_for_structure_ptr. + (write_func_for_structure): Likewise. + (ggc_wtd): Add initializer of new "kind" field. + (pch_wtd): Likewise. + + * gengtype.h (enum write_types_kinds): New. + (struct type): Add field wrote_user_func_for_ptr to the "s" + union member. + 2014-05-13 Richard Sandiford <r.sandif...@uk.ibm.com> * fold-const.c (optimize_bit_field_compare): Use wi:: operations Index: gcc/gengtype-parse.c =================================================================== --- gcc/gengtype-parse.c (revision 210378) +++ gcc/gengtype-parse.c (revision 210379) @@ -197,18 +197,19 @@ return v; } -/* If the next token does not have one of the codes T1, T2 or T3, report a +/* If the next token does not have one of the codes T1, T2, T3 or T4, report a parse error; otherwise return the token's value. */ static const char * -require3 (int t1, int t2, int t3) +require4 (int t1, int t2, int t3, int t4) { int u = token (); const char *v = advance (); - if (u != t1 && u != t2 && u != t3) + if (u != t1 && u != t2 && u != t3 && u != t4) { - parse_error ("expected %s, %s or %s, have %s", + parse_error ("expected %s, %s, %s or %s, have %s", print_token (t1, 0), print_token (t2, 0), - print_token (t3, 0), print_token (u, v)); + print_token (t3, 0), print_token (t4, 0), + print_token (u, v)); return 0; } return v; @@ -245,7 +246,9 @@ /* The caller has detected a template declaration that starts with TMPL_NAME. Parse up to the closing '>'. This recognizes - simple template declarations of the form ID<ID1,ID2,...,IDn>. + simple template declarations of the form ID<ID1,ID2,...,IDn>, + potentially with a single level of indirection e.g. + ID<ID1 *, ID2, ID3 *, ..., IDn>. It does not try to parse anything more sophisticated than that. Returns the template declaration string "ID<ID1,ID2,...,IDn>". */ @@ -254,6 +257,7 @@ require_template_declaration (const char *tmpl_name) { char *str; + int num_indirections = 0; /* Recognize the opening '<'. */ require ('<'); @@ -294,9 +298,21 @@ depth -= 1; continue; } - const char *id = require3 (SCALAR, ID, ','); + const char *id = require4 (SCALAR, ID, '*', ','); if (id == NULL) - id = ","; + { + if (T.code == '*') + { + id = "*"; + if (num_indirections++) + parse_error ("only one level of indirection is supported" + " in template arguments"); + } + else + id = ","; + } + else + num_indirections = 0; str = concat (str, id, (char *) 0); } return str;