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