On 10/5/21 13:54, Richard Biener wrote: > On Mon, Oct 4, 2021 at 1:13 PM Martin Liška wrote: >> >> On 9/22/21 11:59, Richard Biener wrote: >>> On Thu, Sep 16, 2021 at 3:12 PM Martin Liška 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::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 > (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. Hello. All right, I made a patch candidate that does the suggested steps and I ported rs6000 target (both ELF and XCOFF sub-targets). So far I was able to bootstrap on ppc6le-linux-gnu. Thoughts? Thanks, Martin > > Richard. > >> Thanks, >> Cheers, >> Martin >> >> >>> >>> Richard. >>> >>>> Thanks, >>>> Martin >>