David Malcolm <dmalc...@redhat.com> writes:
> @@ -2388,8 +2389,10 @@ gen_mnemonic_setattr (htab_t mnemonic_htab, rtx insn)
> static int
> mnemonic_htab_callback (void **slot, void *info ATTRIBUTE_UNUSED)
> {
> - obstack_grow (&string_obstack, (char*)*slot, strlen ((char*)*slot));
> - obstack_1grow (&string_obstack, ',');
> + struct obstack *string_obstack = rtx_reader_ptr->get_string_obstack ();
> +
> + obstack_grow (string_obstack, (char*)*slot, strlen ((char*)*slot));
> + obstack_1grow (string_obstack, ',');
The old code was missing a space after the cast. Might be good to
add it while you're there.
> @@ -2470,8 +2474,8 @@ gen_mnemonic_attr (void)
> htab_traverse (mnemonic_htab, mnemonic_htab_callback, NULL);
>
> /* Replace the last ',' with the zero end character. */
> - *((char *)obstack_next_free (&string_obstack) - 1) = '\0';
> - XSTR (mnemonic_attr, 1) = XOBFINISH (&string_obstack, char *);
> + *((char *)obstack_next_free (string_obstack) - 1) = '\0';
> + XSTR (mnemonic_attr, 1) = XOBFINISH (string_obstack, char *);
Same here.
> @@ -129,9 +105,9 @@ get_md_ptr_loc (const void *ptr)
> void
> copy_md_ptr_loc (const void *new_ptr, const void *old_ptr)
> {
> - const struct ptr_loc *loc = get_md_ptr_loc (old_ptr);
> + const struct ptr_loc *loc = rtx_reader_ptr->get_md_ptr_loc (old_ptr);
> if (loc != 0)
> - set_md_ptr_loc (new_ptr, loc->filename, loc->lineno);
> + rtx_reader_ptr->set_md_ptr_loc (new_ptr, loc->filename, loc->lineno);
> }
I suppose it's bikeshedding, but since you need access to an rtx_reader
to get and set the locations, it feels like the rtx_reader should be
a parameter here too.
> @@ -140,7 +116,7 @@ copy_md_ptr_loc (const void *new_ptr, const void *old_ptr)
> void
> fprint_md_ptr_loc (FILE *outf, const void *ptr)
> {
> - const struct ptr_loc *loc = get_md_ptr_loc (ptr);
> + const struct ptr_loc *loc = rtx_reader_ptr->get_md_ptr_loc (ptr);
> if (loc != 0)
> fprintf (outf, "#line %d \"%s\"\n", loc->lineno, loc->filename);
> }
Same here.
> +/* As rtx_reader::join_c_conditions above, but call it on the singleton
> + rtx_reader instance. */
> +
> +const char *
> +join_c_conditions (const char *cond1, const char *cond2)
> +{
> + return rtx_reader_ptr->join_c_conditions (cond1, cond2);
> +}
> +
I think it'd be better to have the callers use the rtx_reader_ptr
explicitly.
In general I think it'd be better if the read-md.h interface itself
didn't rely on there being a singleton, other than providing the
rtx_reader_ptr variable. It would then be easy to allow non-singleton
uses in future, without having to update gen* calls a second time.
> @@ -204,6 +189,15 @@ fprint_c_condition (FILE *outf, const char *cond)
> }
> }
>
> +/* As rtx_reader::fprint_c_condition above, but call it on the singleton
> + rtx_reader instance. */
> +
> +void
> +fprint_c_condition (FILE *outf, const char *cond)
> +{
> + rtx_reader_ptr->fprint_c_condition (outf, cond);
> +}
> +
> /* Special fprint_c_condition for writing to STDOUT. */
>
> void
Another instance of the above.
> @@ -808,7 +802,7 @@ handle_constants (void)
> void
> traverse_md_constants (htab_trav callback, void *info)
> {
> - htab_traverse (md_constants, callback, info);
> + htab_traverse (rtx_reader_ptr->get_md_constants (), callback, info);
> }
Here too.
I assume you didn't turn the functions above into member functions
because they aren't primitives. OTOH it might seem inconsistent
to have things like traverse_enum_types as member functions but
traverse_md_constants not.
Thanks,
Richard
