Hi Haochen,
Some minor comments are inlined.
on 2022/2/16 下午4:42, HAO CHEN GUI via Gcc-patches wrote:
> Hi,
> This patch enables absolute jump tables on PPC AIX and Linux. For AIX, the
> jump
> table is placed in data section. For Linux, it is placed in RELRO section when
> relocation is needed.
>
> Bootstrapped and tested on AIX,Linux BE and LE with no regressions. Is
> this okay for trunk?
> Any recommendations? Thanks a lot.
>
I may miss something, but there seem no test cases in power target testsuite to
check expected
assembly for absolute and relative jump table. If so, maybe it's better to add
one or two?
> ChangeLog
> 2022-02-16 Haochen Gui <guih...@linux.ibm.com>
>
> gcc/
> * config/rs6000/aix.h (JUMP_TABLES_IN_TEXT_SECTION): Define.
> * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Likewise.
> * config/rs6000/rs6000.cc (rs6000_option_override_internal): Enable
> absolute jump tables for AIX and Linux.
> (rs6000_xcoff_function_rodata_section): Implement.
> * config/rs6000/xcoff.h (TARGET_ASM_FUNCTION_RODATA_SECTION): Define.
>
> patch.diff
> diff --git a/gcc/config/rs6000/aix.h b/gcc/config/rs6000/aix.h
> index ad3238bf09a..b52208c2ee7 100644
> --- a/gcc/config/rs6000/aix.h
> +++ b/gcc/config/rs6000/aix.h
> @@ -253,7 +253,7 @@
>
> /* Indicate that jump tables go in the text section. */
>
> -#define JUMP_TABLES_IN_TEXT_SECTION 1
> +#define JUMP_TABLES_IN_TEXT_SECTION 0
>
> /* Define any extra SPECS that the compiler needs to generate. */
> #undef SUBTARGET_EXTRA_SPECS
> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
> index b2a7afabc73..16df9ef167f 100644
> --- a/gcc/config/rs6000/linux64.h
> +++ b/gcc/config/rs6000/linux64.h
> @@ -239,7 +239,7 @@ extern int dot_symbols;
>
> /* Indicate that jump tables go in the text section. */
> #undef JUMP_TABLES_IN_TEXT_SECTION
> -#define JUMP_TABLES_IN_TEXT_SECTION TARGET_64BIT
> +#define JUMP_TABLES_IN_TEXT_SECTION 0
>
> /* The linux ppc64 ABI isn't explicit on whether aggregates smaller
> than a doubleword should be padded upward or downward. You could
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index bc3ef0721a4..e9c5552c082 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -4954,6 +4954,10 @@ rs6000_option_override_internal (bool global_init_p)
> warning (0, "%qs is deprecated and not recommended in any circumstances",
> "-mno-speculate-indirect-jumps");
>
> + /* Enable absolute jump tables for AIX and Linux. */
> + if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
> + rs6000_relative_jumptables = 0;
> +
> return ret;
> }
>
> @@ -28751,6 +28755,15 @@ constant_generates_xxspltidp (vec_const_128bit_type
> *vsx_const)
> return sf_value;
> }
>
> +section * rs6000_xcoff_function_rodata_section (tree decl ATTRIBUTE_UNUSED,
> + bool relocatable)
> +{
> + if (relocatable)
> + return data_section;
> + else
> + return readonly_data_section;
> +}
> +
>
There is one area to put xcoff related functions and guarded with "#if
TARGET_XCOFF",
it looks good to put this into? and could we make this function static?
Besides, it seems good to put some comments for this function to describe why we
choose to use the data_section.
> struct gcc_target targetm = TARGET_INITIALIZER;
>
> diff --git a/gcc/config/rs6000/xcoff.h b/gcc/config/rs6000/xcoff.h
> index cd0f99cb9c6..0dacd86eed9 100644
> --- a/gcc/config/rs6000/xcoff.h
> +++ b/gcc/config/rs6000/xcoff.h
> @@ -98,7 +98,7 @@
> #define TARGET_ASM_SELECT_SECTION rs6000_xcoff_select_section
> #define TARGET_ASM_SELECT_RTX_SECTION rs6000_xcoff_select_rtx_section
> #define TARGET_ASM_UNIQUE_SECTION rs6000_xcoff_unique_section
> -#define TARGET_ASM_FUNCTION_RODATA_SECTION default_no_function_rodata_section
> +#define TARGET_ASM_FUNCTION_RODATA_SECTION
> rs6000_xcoff_function_rodata_section
IIUC, the behavior of rs6000_xcoff_function_rodata_section isn't consistent
with what we have
in the hook description. If so, we need to update the hook description to
align with this change.
Otherwise, some future codes might expect this hook only return readonly
section (not possible
like data section) and get unexpected results.
BR,
Kewen
