On Mon, Oct 4, 2021 at 1:13 PM Martin Liška <mli...@suse.cz> wrote:
>
> On 9/22/21 11:59, Richard Biener wrote:
> > On Thu, Sep 16, 2021 at 3:12 PM Martin Liška <mli...@suse.cz> wrote:
> >>
> >> This patch comes up with asm_out_state and a new global variable casm.
> >>
> >> Tested on all cross compilers.
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > * output.h (struct asm_out_file): New struct that replaces a
> > ^^^
> > asm_out_state?
>
> Yes, sure!
>
> >
> > You replace a lot of asm_out_file - do we still need the macro then?
> > (I'd have simplified the patch leaving that replacement out at first)
>
> Well, I actually replaced only a very small fraction of the usage of
> asm_out_file.
>
> $ git grep asm_out_file | grep -v ChangeLog | wc -l
>
> 1601
>
>
> So I think we should live with the macro for some time ...
>
> >
> > You leave quite some global state out of the struct that is obviously
> > related, in the diff I see object_block_htab for example.
>
> Yes, I'm aware of it. Can we do that incrementally?
>
> > Basically
> > everything initialized in init_varasm_once is a candidate (which
> > then shows const_desc_htab and shared_constant_pool as well
> > as pending_assemble_externals_set).
>
> Well, these would probably need a different header file (or another #include
> ... must
> be added before output.h :// ).
>
> > For the goal of outputting
> > early DWARF to another file the state CTOR could have a mode
> > to not initialize those parts or we could have asm-out-state-with-sections
> > as base of asm-out-state.
>
> Yes, right now asm_out_state ctor is minimal:
>
> asm_out_state (): out_file (NULL), in_section (NULL),
> sec ({}), anchor_labelno (0), in_cold_section_p (false)
> {
> section_htab = hash_table<section_hasher>::create_ggc (31);
> }
>
> >
> > In the end there will be a target part of the state so I think
> > construction needs to be defered to the target which can
> > derive from asm-out-state and initialize the part it needs.
> > That's currently what targetm.asm_out.init_sections () does
> > and we'd transform that to a targetm.asm_out.create () or so.
> > That might already be necessary for the DWARF stuff.
>
> So what do you want to with content of init_varasm_once function?
It goes away ;) Or rather becomes the invocation of the
asm-out-state CTOR via the target hook.
> >
> > That said, dealing with the target stuff piecemail is OK, but maybe
> > try to make sure that init_varasm_once is actually identical
> > to what the CTOR does?
>
> So you want asm_out_state::asm_out_state doing what we current initialize
> in init_varasm_once, right?
Yes, asm_out_state should cover everything initialized in init_varasm_once
(and the called targetm.asm_out.init_sections).
targetm.asm_out.init_sections would become
asm_out_state *init_sections ();
where the target can derive from asm_out_state (optionally) and
allocates the asm-out-state & invokes the CTORs. The middle-end
visible asm_out_state would contain all the tables initialized in
init_varasm_once.
I'm not sure if doing it piecemail is good - we don't want to touch
things multiple times without good need and it might be things
turn out not be as "simple" as the above plan sounds.
I would suggest to try the conversion with powerpc since it
seems to be the only target with two different implementations
for the target hook.
The
static GTY(()) section *read_only_data_section;
static GTY(()) section *private_data_section;
static GTY(()) section *tls_data_section;
static GTY(()) section *tls_private_data_section;
static GTY(()) section *read_only_private_data_section;
static GTY(()) section *sdata2_section;
section *toc_section = 0;
could become #defines to static_cast<rs6000_asm_out_state *>
(casm)->section_name
and there'd be
class rs6000_asm_out_state : public asm_out_state
{
... add stuff ...
}
in rs6000/xcoff.h and rs6000/sysv4.h and some generic rs6000 header as well
(adding no fields) and the target hook would basically do
return ggc_new rs6000_asm_state ();
(OK, ggc_alloc + placement new I guess). The default hook just
creates asm_out_state.
Richard.
> Thanks,
> Cheers,
> Martin
>
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
>
