On Mon, Aug 24, 2020 at 03:48:43PM +0800, HAO CHEN GUI wrote:
> >I'll try to be quicker at reviewing iterations of this -- there is quite
> >some way to go, without me slowing things down!
Sigh :-(
> * config/rs6000/linux.h (rs6000_relative_jumptables): Define.
That macro looks like it is variable (or function). *Make* it a
variable, please?
> * config/rs6000/rs6000.c (TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC):
> Define
Period?
> (rs6000_gen_pic_addr_diff_vec, rs6000_output_addr_vec_elt): Implement.
"New function."
> * config/rs6000/rs6000.md (absolute_tablejumpsi,
> absolute_tablejumpsi_nospec, absolute_tablejumpdi,
> absolute_tablejumpdi_nospec): Add four new expansions.
"New define_expands." or "New expanders."
> * config/rs6000/rs6000.opt (mrelative-jumptables): Add a new option and
> set rs6000_relative_jumptables to true by default.
"rs6000.opt: Add -mrelative-jumptables."
> +/* Disable relative jump tables for Power Linux. */
> +#undef rs6000_relative_jumptables
> +#define rs6000_relative_jumptables 0
Why?
> +/* Disable relative jump tables for Power Linux64. */
> +#undef rs6000_relative_jumptables
> +#define rs6000_relative_jumptables 0
(That's not what it's called... Just don't say the "for..." at all?
It is clear from what file it is in.)
> /* 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 rs6000_relative_jumptables
Not sure that is correct. Maybe the patch using rodata (.data.rel.ro)
should be a separate patch?
> /* Define as C expression which evaluates to nonzero if the tablejump
> instruction expects the table to contain offsets from the address of the
> table.
> Do not define this if the table should contain absolute addresses. */
> -#define CASE_VECTOR_PC_RELATIVE 1
> +#define CASE_VECTOR_PC_RELATIVE 0
This should depend on the new flag?
> +/* Specify the machine mode that this machine uses
> + for the index in the tablejump instruction. */
> +#define CASE_VECTOR_MODE \
> + (TARGET_32BIT || rs6000_relative_jumptables ? SImode : DImode)
rs6000_relative_jumptables ? SImode : Pmode;
> + if (rs6000_relative_jumptables)
> + {
> + if (TARGET_32BIT)
> + emit_jump_insn (gen_tablejumpsi (operands[0], operands[1]));
> + else
> + emit_jump_insn (gen_tablejumpdi (operands[0], operands[1]));
> + }
Hrm, I guess we should make that a parameterized name (future work,
don't do it now :-) )
> +(define_expand "absolute_tablejumpsi"
Don't prefix names; it should start with "tablejump".
Segher
