On Fri, 2021-05-07 at 12:19 +0930, Alan Modra via Gcc-patches wrote:
> On PowerPC64 ELFv1 function symbols are defined on function
> descriptors in an .opd section rather than in the function code.
> .opd is not split up by the PowerPC64 backend for comdat groups or
> other situations where per-function sections are required. Thus
> SECTION_LINK_ORDER can't use the function name to reference a
> suitable
> section for ordering: The .opd section might contain many other
> function descriptors and they may be in a different order to the
> final
> function code placement. This patch arranges to use a code label
> instead of the function name symbol.
>
> I chose to emit the label inside default_elf_asm_named_section,
> immediately before the .section directive using the label, and in
> case
> someone uses .previous or the like, need to save and restore the
> current section when switching to the function code section to emit
> the label. That requires a tweak to switch_to_section in order to
> get
> the current section. I checked all the TARGET_ASM_NAMED_SECTION
> functions and unnamed.callback functions and it appears none will be
> affected by that tweak.
Hi,
good description. thanks :-)
>
> PR target/98125
> * varasm.c (default_elf_asm_named_section): Use a function
> code label rather than the function symbol as the "o" argument.
> (switch_to_section): Don't set in_section until section
> directive has been emitted.
>
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index 97c1e6fff25..5f95f8cfa75 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -6866,6 +6866,26 @@ default_elf_asm_named_section (const char
> *name, unsigned int flags,
> *f = '\0';
> }
>
> + char func_label[256];
> + if (flags & SECTION_LINK_ORDER)
> + {
> + static int recur;
> + if (recur)
> + gcc_unreachable ();
Interesting.. Is there any anticipation of re-entry or parallel runs
through this function that requires the recur lock/protection?
> + else
> + {
> + ++recur;
> + section *save_section = in_section;
> + static int func_code_labelno;
> + switch_to_section (function_section (decl));
> + ++func_code_labelno;
> + ASM_GENERATE_INTERNAL_LABEL (func_label, "LPFC",
> func_code_labelno);
> + ASM_OUTPUT_LABEL (asm_out_file, func_label);
> + switch_to_section (save_section);
> + --recur;
> + }
> + }
ok
> +
> fprintf (asm_out_file, "\t.section\t%s,\"%s\"", name, flagchars);
>
> /* default_section_type_flags (above) knows which flags need
> special
> @@ -6893,11 +6913,8 @@ default_elf_asm_named_section (const char
> *name, unsigned int flags,
> fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
> if (flags & SECTION_LINK_ORDER)
> {
> - tree id = DECL_ASSEMBLER_NAME (decl);
> - ultimate_transparent_alias_target (&id);
> - const char *name = IDENTIFIER_POINTER (id);
> - name = targetm.strip_name_encoding (name);
> - fprintf (asm_out_file, ",%s", name);
> + fputc (',', asm_out_file);
> + assemble_name_raw (asm_out_file, func_label);
ok as far as I can tell :-) assemble_name_raw is an if/else that
outputs 'name' or a LABELREF based on the file & name. It's not an
obvious analog to the untimate_transparent_alias_target() and name
processing that is being replaced, but seems to fit the changes as
described.
> }
> if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
> {
> @@ -7821,11 +7838,6 @@ switch_to_section (section *new_section, tree
> decl)
> else if (in_section == new_section)
> return;
>
> - if (new_section->common.flags & SECTION_FORGET)
> - in_section = NULL;
> - else
> - in_section = new_section;
> -
> switch (SECTION_STYLE (new_section))
> {
> case SECTION_NAMED:
> @@ -7843,6 +7855,11 @@ switch_to_section (section *new_section, tree
> decl)
> break;
> }
>
> + if (new_section->common.flags & SECTION_FORGET)
> + in_section = NULL;
> + else
> + in_section = new_section;
> +
> new_section->common.flags |= SECTION_DECLARED;
OK.
lgtm, thx
-Will
> }
>
