[CCing Richard; this is re:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00273.html
]
Essentially I want to split class rtx_reader into two parts: a base class
covering the things implemented in read-md.o, and a subclass implemented in
read-rtl.o.
The motivation is that I want to make some of the rtx-reading functions in
read-rtl.c virtual in a followup, so that they can be overridden by the
function_reader subclass in my RTL frontend. Hence the ctor needs access to
these functions in order to access the vtable - or the link fails.
It appears that I can't simply use read-rtl.o everywhere due to
read-rtl.c needing insn-constants.h, which is built by genconstants, which
would be a circular dependency, hence the split between BUILD_MD vs BUILD_RTL
in Makefile.in to break this cycle.
On Tue, 2016-10-11 at 17:53 +0200, Bernd Schmidt wrote:
> On 10/05/2016 06:15 PM, David Malcolm wrote:
> > - rtx_reader_ptr->get_top_level_filename ());
> > + base_rtx_reader_ptr->get_top_level_filename ());
>
> I wonder if the number of changes could be minimized by retaining the
> name rtx_reader for the base class, and using something more specific
> for the derived ones. Or would that require renaming a bunch of other
> locations?
The number of changes got rather larger with r241293
("[PATCH] read-md.c: Move various state to within class rtx_reader
(v3)"; https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01323.html
), so I'd rather just "bite the bullet" and do the renamings.
I'm not in love with the names I chose in this patch. It does seem odd
having an "rtx_reader" class that can't actually read hierarchical rtx.
How about "md_reader" as the base class (with responsibility for the
things in read-md.o), and "rtx_reader" for the subclass (adding the
things in read-rtl.o)?
> > -static void
> > -read_rtx_operand (rtx return_rtx, int idx)
> > +rtx
> > +rtx_reader::read_rtx_operand (rtx return_rtx, int idx)
> > {
> > RTX_CODE code = GET_CODE (return_rtx);
> > const char *format_ptr = GET_RTX_FORMAT (code);
>
> > +
> > + return return_rtx;
> > }
> >
>
> This appears to be an unrelated change. The return value needs to be
> documented.
Indeed. I'll split that change out into a followup.
Thanks
Dave
