[PATCH 3/N] Come up with casm global state.

Martin Liška mliska@suse.cz
Fri Nov 5 14:27:37 GMT 2021


On 10/26/21 09:45, Richard Biener wrote:
> On Mon, Oct 25, 2021 at 6:32 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>>
>> Hi!
>>
>> On Mon, Oct 25, 2021 at 03:36:25PM +0200, Martin Liška wrote:
>>> --- a/gcc/config/rs6000/rs6000-internal.h
>>> +++ b/gcc/config/rs6000/rs6000-internal.h
>>> @@ -189,4 +189,13 @@ extern bool rs6000_passes_vector;
>>>   extern bool rs6000_returns_struct;
>>>   extern bool cpu_builtin_p;
>>>
>>> +struct rs6000_asm_out_state : public asm_out_state
>>> +{
>>> +  /* Initialize ELF sections. */
>>> +  void init_elf_sections ();
>>> +
>>> +  /* Initialize XCOFF sections. */
>>> +  void init_xcoff_sections ();
>>> +};
>>
>> Our coding convention says to use "class", not "struct" (since this
>> isn't valid C code at all).
>>
>>> -  sdata2_section
>>> + sec.sdata2
>>>       = get_unnamed_section (SECTION_WRITE, output_section_asm_op,
>>>                           SDATA2_SECTION_ASM_OP);
>>
>> (broken indentation)
>>
>>> +/* Implement TARGET_ASM_INIT_SECTIONS.  */
>>
>> That comment is out-of-date.
>>
>>> +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 ();
>>
>> Hrm, maybe we can have a macro or function that does this, ggc_new or
>> something?
>>
>>> +/* Implement TARGET_ASM_INIT_SECTIONS.  */
>>> +
>>> +static asm_out_state *
>>> +rs6000_xcoff_asm_init_sections (void)
>>
>> Here, too.  Both implementations are each one of several functions that
>> together implement the target macro.
>>
>>> +    /* The section that holds the DWARF2 frame unwind information, when known.
>>> +       The section is set either by the target's init_sections hook or by the
>>> +       first call to switch_to_eh_frame_section.  */
>>> +    section *eh_frame;
>>> +
>>> +    /* RS6000 sections.  */
>>
>> Nothing here?  Just remove the comment header?
>>
>> The idea looks fine to me.
> 
> Yeah, of course then the target hook does not need to do the allocation
> and we could simply keep the current init_sections hook but change it
> to take the asm_out_state to initialize as argument.

Makes sense.

> 
> Note that I'd put
> 
> +    /* RS6000 sections.  */
> +
> +    /* ELF sections.  */
> +    section *toc;
> +    section *sdata2;
> +
> +    /* XCOFF sections.  */
> +    section *read_only_data;
> +    section *private_data;
> +    section *tls_data;
> +    section *tls_private_data;
> +    section *read_only_private_data;
> 
> into a union, thus
> 
>     union {
>         struct /* RS6000 sections */ {
>             /* ELF sections.  */
>            section *toc;
> ...
>         } rs6000;
>         struct /* darwin sections */ {
>   ...
>     };

Union is a bit tricky for GGC marking script, but we can manage that.

> 
> not sure whether we need some magic GTY marking here to make
> it pick up the 'correct' set.  Another alternative would be
> 
>           section *target[MAX_TARGET_SECTIONS];
> 
> and #defines in the targets mapping the former global variables to
> indices in that array.
> 
> All of this isn't "nice C++" of course, but well ... I'm not the one
> to insist ;)

Anyway, I took a look at targets that do call the init_sections hook and I noticed
Darwin uses pretty many sections and comes up with an array that is defined here:
./gcc/config/darwin-sections.def

I tend to creating all sections in asm_out_state with DEF_SECTION, where the list
will be extensible with a target-specific definition list.

What do you think Richi?

Thanks,
Martin

> 
> Richard.
> 
>>
>> Segher



More information about the Gcc-patches mailing list