On Thu, Oct 21, 2021 at 5:42 PM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
>
> On Thu, Oct 21, 2021 at 02:42:10PM +0200, Richard Biener wrote:
> > +#define rs6000_casm static_cast<rs6000_asm_out_state *> (casm)
> >
> > maybe there's a better way?  Though I can't think of one at the moment.
> > There are only 10 uses so eventually we can put the
> > static_cast into all places.  Let's ask the powerpc maintainers (CCed).
>
> It's disgusting, and fragile.  The define is slightly better than having
> to write it out every time.  But can this not be done properly?
>
> If you use object-oriented stuff and need casts for that, you are doing
> something wrong.

I think the "proper" fix would be to make 'casm' have the correct type
in the first place - of course that would either mean that the target
needs to provide the (possibly derived) type, for example via a typedef
in the target structure or a classical target macro.  If gengtype would
know about inheritance that would also fix the GC marking issue.
Of course encoding a static type in the target structure is the
wrong direction from making targets "switchable".

Other than that my C++ fu is too weak to suggest the correct "pattern"
here.

> > Note you do
> >
> > +/* Implement TARGET_ASM_INIT_SECTIONS.  */
> > +
> > +static asm_out_state *
> > +rs6000_elf_asm_init_sections (void)
> > +{
> > +  rs6000_asm_out_state *target_state
> > +    = new (ggc_alloc<rs6000_asm_out_state> ()) rs6000_asm_out_state ();
> > +  target_state->init_elf_sections ();
> > +  target_state->init_sections ();
> > +
> > +  return target_state;
> > +}
> >
> > If you'd have made init_sections virtual the flow would be more
> > natural and we could separate section init from casm construction
> > (and rs6000 would override init_sections but call the base function
> > from the override).
>
> Yeah.
>
>
> Segher

Reply via email to