Hi David,
On 12/1/2022 下午 10:44, David Edelsohn wrote:
> On Wed, Jan 12, 2022 at 7:22 AM HAO CHEN GUI <guih...@linux.ibm.com> wrote:
>>
>> Hi,
>> This patch enables absolute jump table by default on rs6000. The relative
>> jump tables are used when
>> it's explicit set by "rs6000_relative_jumptables",
>> or jump tables are placed in text section but global relocation is
>> required.
>>
>> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
>> Is this okay for trunk?
>> Any recommendations? Thanks a lot.
>>
>> ChangeLog
>> 2022-01-12 Haochen Gui <guih...@linux.ibm.com>
>>
>> gcc/
>> * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Define.
>> * config/rs6000/rs6000.c (rs6000_gen_pic_addr_diff_vec): Return
>> true when relative jump table is explicit required or jump tables
>> have
>> to be put in text section but global relocation is also required.
>> * config/rs6000/rs6000.opt (rs6000_relative_jumptables): Disable.
>>
>> patch.diff
>> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
>> index d617f346f81..2e257c60f8c 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.c b/gcc/config/rs6000/rs6000.c
>> index 319182e94d9..9fba893a27a 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -28465,7 +28465,9 @@ rs6000_emit_xxspltidp_v2df (rtx dst, long value)
>> static bool
>> rs6000_gen_pic_addr_diff_vec (void)
>> {
>> - return rs6000_relative_jumptables;
>> + return rs6000_relative_jumptables
>> + || (JUMP_TABLES_IN_TEXT_SECTION
>> + && targetm.asm_out.reloc_rw_mask () == 3);
>> }
>
> This seems like contorted logic and overriding the
> rs6000_relative_jumptables option change. The later part of the patch
> overrides rs6000_relative_jumptables for all rs6000 configurations,
> and then changes this one use of rs6000_relative_jumptables to add
> more logic to revert to the old meaning for some targets.
>
> What about all of the other uses of rs6000_relative_jumptables in the
> target? What about rs6000.md?
>
> I highly doubt that this patch is correct.
>
> Why not override rs6000_relative_jumptables for PPC64 Linux instead of
> changing its value globally and then trying to fix it up?
>
> Thanks, David
Thanks for your comments.
In this patch, I tried to enable absolute jump table on all rs6000 targets.
For PPC64 Linux, it supports RELRO section (e.g. .data.rel.ro.local) as
"JUMP_TABLES_IN_TEXT_SECTION" is set to 0. So, absolute jump tables could be
placed
in RELRO section whatever global relocation is required or not. The absolute
jump
table can't be placed in text section when global relocation is required as text
section is not writable. So for other rs6000 targets, absolute jump table can't
be
used if the target doesn't support RELRO and global relocation is required also.
Looking forward to your advice. Thanks again.
>
>>
>> void
>> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
>> index c2a77182a9e..75e3fa86829 100644
>> --- a/gcc/config/rs6000/rs6000.opt
>> +++ b/gcc/config/rs6000/rs6000.opt
>> @@ -630,7 +630,7 @@ Target Mask(MMA) Var(rs6000_isa_flags)
>> Generate (do not generate) MMA instructions.
>>
>> mrelative-jumptables
>> -Target Undocumented Var(rs6000_relative_jumptables) Init(1) Save
>> +Target Undocumented Var(rs6000_relative_jumptables) Init(0) Save
>>
>> mrop-protect
>> Target Var(rs6000_rop_protect) Init(0)
