Hi!
Alan tells me -fpatchable-function-entry is inconsequential on PowerPC,
since the option is really only for the Linux kernel, and that uses
-mprofile-kernel for us, instead.
That option is only for 64 bit. The kernel does not support profiling
for 32-bit PowerPC. Curiously, the kernel also requires LE here. I
wonder why that is, GCC works fine for this in BE as well.
-mprofile-kernel works fine on ELFv2, with its dual entrypoints. It
also works fine on ELFv1, there are no dot symbol or official
procedure descriptor problems.
On Wed, Mar 31, 2021 at 02:49:29PM +0200, Jakub Jelinek wrote:
> I'm afraid I don't know what exactly should be done, whether e.g.
> it could use
> .section
> __patchable_function_entries,"awo",@progbits,.L._Z3foov
> instead, or whether the linker should be changed to handle it as is, or
> something else.
I have no idea either. The GAS documentation says (for ELF section
flags):
'o'
section references a symbol defined in another section (the
linked-to section) in the same file.
But GCC thinks it means something with "link order"?
> But because we have a P1 regression that didn't see useful progress over the
> 4 months since it has been filed and we don't really have much time, below
> is an attempt to do a targetted reversion of H.J's patch, basically act as
> if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1,
> but for 32-bit or 64-bit ELFv2 keep working as is.
> This would give us time to resolve it for GCC 12 properly.
It then still is broken for ELFv2? Do we need to open a separate bug
for that?
Disabling the testcase for PowerPC is fine, too, if you prefer that.
> +++ gcc/config/rs6000/rs6000.c 2021-03-30 15:59:10.299755166 +0200
> @@ -1341,6 +1341,9 @@ static const struct attribute_spec rs600
> #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
> #endif
>
> +#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
> +#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
> rs6000_print_patchable_function_entry
Line too long.
> +void
> +rs6000_print_patchable_function_entry (FILE *file,
> + unsigned HOST_WIDE_INT patch_area_size,
> + bool record_p)
> +{
> + unsigned int flags = SECTION_WRITE | SECTION_RELRO;
> + /* When .opd section is emitted, the function symbol
> + default_print_patchable_function_entry_1 is emitted into the .opd
> section
> + while the patchable area is emitted into the function section.
> + Don't use SECTION_LINK_ORDER in that case. */
> + if (!(TARGET_64BIT && DEFAULT_ABI != ABI_ELFv2)
> + && HAVE_GAS_SECTION_LINK_ORDER)
> + flags |= SECTION_LINK_ORDER;
Does this need a check that it is ELF at all?
> --- gcc/testsuite/g++.dg/pr93195a.C.jj 2020-12-02 14:42:52.216054386
> +0100
> +++ gcc/testsuite/g++.dg/pr93195a.C 2021-03-30 17:09:06.529574261 +0200
> @@ -1,4 +1,5 @@
> /* { dg-do link { target { ! { nvptx*-*-* visium-*-* } } } } */
> +/* { dg-skip-if "not supported yet" { { powerpc*-*-* } && lp64 } } */
> // { dg-require-effective-target o_flag_in_section }
> /* { dg-options "-O0 -fpatchable-function-entry=1" } */
> /* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
"not supported", instead.
Okay for trunk. Thank you!
(Sorry this took so long, has to do a lot of archaeology and research,
pretty much nothing here is documented :-/ )
Segher
