On 10/26/21 09:45, Richard Biener wrote:
On Mon, Oct 25, 2021 at 6:32 PM Segher Boessenkool
<seg...@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
