[PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[HAO CHEN GUI]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

Hi,

This patch adds non-relative jump table support for 64bit rs6000. It 
implements ASM_OUTPUT_ADDR_VEC_ELT and corresponding expansion of jump 
table instruction for 64bit rs6000. We want to put non-relative jump 
table in data.rel.ro section for rs6000. So I add a new target hook - 
TARGET_ASM_SELECT_JUMPTABLE_SECTION to choose which section jump table 
should be put in. It puts the jump table in data.rel.ro for 64bit 
rs6000. For other platforms, it calls 
targetm.asm_out.function_rodata_section, just as before. We want to have 
an option to use non-relative jump table even when PIC flag is set. So I 
add a new option - "mforce-nonrelative-jumptables" for rs6000.  It 
enables the feature by default. The option takes effect in target hook - 
TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC.

static bool
rs6000_gen_pic_addr_diff_vec (void)
{
   return TARGET_32BIT || !rs6000_force_nonrelative_jumptables;
}

The attachments are the patch diff file and change log file.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  
Is this okay for trunk? Any recommendations? Thanks a lot.


[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 1040 bytes --]

2019-11-30  Haochen Gui  <guihaoc@gcc.gnu.org>

        * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Redefine.
        * config/rs6000/rs6000.c (TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC,TARGET_ASM_SELECT_JUMPTABLE_SECTION): Implement two hooks.
		* config/rs6000/rs6000.h (CASE_VECTOR_PC_RELATIVE,CASE_VECTOR_MODE) Redefine.
		* config/rs6000/rs6000.md (nonrelative_tablejumpdi, nonrelative_tablejumpdi_nospec) Add two expansions.
		* config/rs6000/rs6000.opt (mforce-nonrelative-jumptables) Add a new options for rs6000.
        * doc/tm.texi.in (TARGET_ASM_SELECT_JUMPTABLE_SECTION): Likewise.
        * doc/tm.texi: Regenerate.
        * final.c (targetm.asm_out.select_jumptable_section): Replace targetm.asm_out.function_rodata_section with targetm.asm_out.select_jumptable_section in jumptable section selection.
        * output.h (default_select_jumptable_section): Declare.
        * target.def (default_select_jumptable_section): Likewise
        * varasm.c (default_select_jumptable_section): Implementation.

[-- Attachment #3: patch.diff --]
[-- Type: text/plain, Size: 11649 bytes --]

diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
index 34776c8421e..e9a1fed43cd 100644
--- a/gcc/config/rs6000/linux64.h
+++ b/gcc/config/rs6000/linux64.h
@@ -324,7 +324,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 (TARGET_64BIT && flag_pic && !rs6000_force_nonrelative_jumptables)
 
 /* 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 58f5d780603..d0c0ac6529a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1369,6 +1369,12 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #undef TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA
 #define TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA rs6000_output_addr_const_extra
 
+#undef  TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC
+#define TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC rs6000_gen_pic_addr_dif_vec
+
+#undef TARGET_ASM_SELECT_JUMPTABLE_SECTION
+#define TARGET_ASM_SELECT_JUMPTABLE_SECTION rs6000_select_jumptable_section
+
 #undef TARGET_LEGITIMIZE_ADDRESS
 #define TARGET_LEGITIMIZE_ADDRESS rs6000_legitimize_address
 
@@ -26494,6 +26500,68 @@ rs6000_cannot_substitute_mem_equiv_p (rtx mem)
   return false;
 }
 
+/* Implement TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC. Return false if rs6000_force_nonrelative_jumptables is set and target is 64bit. */
+
+static bool
+rs6000_gen_pic_addr_dif_vec (void)
+{
+  return !rs6000_force_nonrelative_jumptables;
+}
+
+/* Implement TARGET_ASM_FUNCTION_RODATA_SECTION. Put non-relative jump table in data.rel.ro section */
+
+section *
+rs6000_select_jumptable_section (tree decl)
+{
+  if (TARGET_32BIT)
+    return default_function_rodata_section (decl);
+
+  if (decl != NULL_TREE && DECL_SECTION_NAME (decl))
+    {
+      const char *name = DECL_SECTION_NAME (decl);
+
+      if (DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP)
+        {
+          const char *dot;
+          size_t len;
+          char* rname;
+
+          dot = strchr (name + 1, '.');
+          if (!dot)
+            dot = name;
+          len = strlen (dot) + 13;
+          rname = (char *) alloca (len);
+
+          strcpy (rname, ".data.rel.ro");
+          strcat (rname, dot);
+          return get_section (rname, SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE, decl);
+        }
+        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
+         else if (DECL_COMDAT_GROUP (decl)
+               && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
+        {
+          size_t len = strlen (name) + 1;
+          char *rname = (char *) alloca (len);
+
+          memcpy (rname, name, len);
+          rname[14] = 'r';
+          return get_section (rname, SECTION_LINKONCE, decl);
+        }
+        /* For .text.foo we want to use .data.rel.ro.foo.  */
+         else if (flag_function_sections && flag_data_sections
+              && strncmp (name, ".text.", 6) == 0)
+        {
+          size_t len = strlen (name) + 1;
+          char *rname = (char *) alloca (len + 7);
+
+          memcpy (rname, ".data.rel.ro", 12);
+          memcpy (rname + 12, name + 5, len - 5);
+          return get_section (rname, SECTION_WRITE | SECTION_RELRO, decl);
+        }
+    }
+  return get_section (".data.rel.ro", SECTION_WRITE | SECTION_RELRO, decl);
+}
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-rs6000.h"
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 1209a33173e..2a148aff9e4 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -1752,15 +1752,15 @@ typedef struct rs6000_args
 
 /* #define LEGITIMATE_PIC_OPERAND_P (X) */
 \f
-/* Specify the machine mode that this machine uses
-   for the index in the tablejump instruction.  */
-#define CASE_VECTOR_MODE SImode
-
 /* 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 TARGET_32BIT
+
+/* Specify the machine mode that this machine uses
+   for the index in the tablejump instruction.  */
+#define CASE_VECTOR_MODE (TARGET_32BIT || (flag_pic && !rs6000_force_nonrelative_jumptables) ? SImode : DImode)
 
 /* Define this as 1 if `char' should by default be signed; else as 0.  */
 #define DEFAULT_SIGNED_CHAR 0
@@ -2190,6 +2190,15 @@ extern char rs6000_reg_names[][8];	/* register names (0 vs. %r0).  */
        putc ('\n', FILE);				\
      } while (0)
 
+/* This is how to output an element of a case-vector that is non-relative. */
+#define ASM_OUTPUT_ADDR_VEC_ELT(FILE, VALUE) \
+  do { char buf[100];                                   \
+       fputs ("\t.quad ", FILE);                        \
+       ASM_GENERATE_INTERNAL_LABEL (buf, "L", VALUE);   \
+       assemble_name (FILE, buf);                       \
+       putc ('\n', FILE);                              \
+     } while (0)
+
 /* This is how to output an assembler line
    that says to advance the location counter
    to a multiple of 2**LOG bytes.  */
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 0aa5265d199..76b9f185e88 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -12670,8 +12670,10 @@
     {
       if (TARGET_32BIT)
       	emit_jump_insn (gen_tablejumpsi (operands[0], operands[1]));
-      else
+      else if (flag_pic && !rs6000_force_nonrelative_jumptables)
 	emit_jump_insn (gen_tablejumpdi (operands[0], operands[1]));
+      else 
+        emit_jump_insn (gen_nonrelative_tablejumpdi (operands[0], operands[1]));
     }
   else
     {
@@ -12679,8 +12681,10 @@
       rtx jump;
       if (TARGET_32BIT)
 	jump = gen_tablejumpsi_nospec (operands[0], operands[1], ccreg);
-      else
+      else if (flag_pic && !rs6000_force_nonrelative_jumptables)
 	jump = gen_tablejumpdi_nospec (operands[0], operands[1], ccreg);
+      else
+	jump = gen_nonrelative_tablejumpdi_nospec (operands[0], operands[1], ccreg);
       emit_jump_insn (jump);
     }
   DONE;
@@ -12731,6 +12735,17 @@
   operands[4] = gen_reg_rtx (DImode);
 })
 
+(define_expand "nonrelative_tablejumpdi"
+  [(set (match_dup 2)
+        (match_operand:DI 0 "gpc_reg_operand" "r"))
+   (parallel [(set (pc)
+                   (match_dup 2))
+              (use (label_ref (match_operand 1)))])]
+   "TARGET_64BIT && rs6000_speculate_indirect_jumps"
+ {
+  operands[2] = gen_reg_rtx (DImode);
+ })
+
 (define_expand "tablejumpdi_nospec"
   [(set (match_dup 5)
         (sign_extend:DI (match_operand:SI 0 "lwa_operand")))
@@ -12748,6 +12763,19 @@
   operands[5] = gen_reg_rtx (DImode);
 })
 
+(define_expand "nonrelative_tablejumpdi_nospec"
+  [(set (match_dup 3)
+        (match_operand:SI 0 "gpc_reg_operand" "r"))
+   (parallel [(set (pc)
+                   (match_dup 3))
+              (use (label_ref (match_operand 1)))
+              (clobber (match_operand 2))])]
+  "TARGET_64BIT && !rs6000_speculate_indirect_jumps"
+{
+  operands[3] = gen_reg_rtx (DImode);
+})
+
+
 (define_insn "*tablejump<mode>_internal1"
   [(set (pc)
 	(match_operand:P 0 "register_operand" "c,*l"))
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index f95b8279270..b3f69a73053 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -578,3 +578,6 @@ Generate (do not generate) prefixed memory instructions.
 mpcrel
 Target Report Mask(PCREL) Var(rs6000_isa_flags)
 Generate (do not generate) pc-relative memory addressing.
+
+mforce-nonrelative-jumptables
+Target Undocumented Var(rs6000_force_nonrelative_jumptables) Init(1) Save
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6e7d9dc54a9..28e2afabe7e 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7712,6 +7712,12 @@ if function is in @code{.text.name}, and the normal readonly-data section
 otherwise.
 @end deftypefn
 
+@deftypefn {Target Hook} {section *} TARGET_ASM_SELECT_JUMPTABLE_SECTION (tree @var{decl})
+Return the section of jump table associated with
+@samp{DECL_SECTION_NAME (@var{decl})}.
+The default version of this function selects the rodata section.
+@end deftypefn
+
 @deftypevr {Target Hook} {const char *} TARGET_ASM_MERGEABLE_RODATA_PREFIX
 Usually, the compiler uses the prefix @code{".rodata"} to construct
 section names for mergeable constant data.  Define this macro to override
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 3be984bbd5c..173a8dfaee8 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5007,6 +5007,8 @@ it is unlikely to be called.
 
 @hook TARGET_ASM_FUNCTION_RODATA_SECTION
 
+@hook TARGET_ASM_SELECT_JUMPTABLE_SECTION
+
 @hook TARGET_ASM_MERGEABLE_RODATA_PREFIX
 
 @hook TARGET_ASM_TM_CLONE_TABLE_SECTION
diff --git a/gcc/final.c b/gcc/final.c
index a3601964a8d..18ed1f6788c 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -2492,7 +2492,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 	    {
 	      int log_align;
 
-	      switch_to_section (targetm.asm_out.function_rodata_section
+	      switch_to_section (targetm.asm_out.select_jumptable_section
 				 (current_function_decl));
 
 #ifdef ADDR_VEC_ALIGN
@@ -2571,7 +2571,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 #endif
 
 	    if (! JUMP_TABLES_IN_TEXT_SECTION)
-	      switch_to_section (targetm.asm_out.function_rodata_section
+	      switch_to_section (targetm.asm_out.select_jumptable_section
 				 (current_function_decl));
 	    else
 	      switch_to_section (current_function_section ());
diff --git a/gcc/output.h b/gcc/output.h
index eb253c50329..b01228b224c 100644
--- a/gcc/output.h
+++ b/gcc/output.h
@@ -573,6 +573,7 @@ extern section *default_elf_select_section (tree, int, unsigned HOST_WIDE_INT);
 extern void default_unique_section (tree, int);
 extern section *default_function_rodata_section (tree);
 extern section *default_no_function_rodata_section (tree);
+extern section *default_select_jumptable_section (tree);
 extern section *default_clone_table_section (void);
 extern section *default_select_rtx_section (machine_mode, rtx,
 					    unsigned HOST_WIDE_INT);
diff --git a/gcc/target.def b/gcc/target.def
index 07059a87caf..32a8523ae16 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -561,6 +561,15 @@ otherwise.",
  section *, (tree decl),
  default_function_rodata_section)
 
+/* Return the section of jump table associated with function DECL.  */
+DEFHOOK
+(select_jumptable_section,
+ "Return the section of jump table associated with\n\
+@samp{DECL_SECTION_NAME (@var{decl})}.\n\
+The default version of this function selects the rodata section.",
+ section *, (tree decl),
+ default_select_jumptable_section)
+
 /* Nonnull if the target wants to override the default ".rodata" prefix
    for mergeable data sections.  */
 DEFHOOKPOD
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 4070f9c17e8..8cdcfbca4f5 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -788,6 +788,14 @@ default_no_function_rodata_section (tree decl ATTRIBUTE_UNUSED)
   return readonly_data_section;
 }
 
+/* Return target hook function_rotdata_section by default.  */
+
+section *
+default_select_jumptable_section (tree decl)
+{
+  return targetm.asm_out.function_rodata_section(decl);
+}
+
 /* A subroutine of mergeable_string_section and mergeable_constant_section.  */
 
 static const char *

Re: [PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[Kewen.Lin]

Added more CCs.

Kewen

on 2020/7/28 下午1:25, HAO CHEN GUI via Gcc-patches wrote:
> Hi,
> 
> This patch adds non-relative jump table support for 64bit rs6000. It implements ASM_OUTPUT_ADDR_VEC_ELT and corresponding expansion of jump table instruction for 64bit rs6000. We want to put non-relative jump table in data.rel.ro section for rs6000. So I add a new target hook - TARGET_ASM_SELECT_JUMPTABLE_SECTION to choose which section jump table should be put in. It puts the jump table in data.rel.ro for 64bit rs6000. For other platforms, it calls targetm.asm_out.function_rodata_section, just as before. We want to have an option to use non-relative jump table even when PIC flag is set. So I add a new option - "mforce-nonrelative-jumptables" for rs6000.  It enables the feature by default. The option takes effect in target hook - TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC.
> 
> static bool
> rs6000_gen_pic_addr_diff_vec (void)
> {
>   return TARGET_32BIT || !rs6000_force_nonrelative_jumptables;
> }
> 
> The attachments are the patch diff file and change log file.
> 
> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this okay for trunk? Any recommendations? Thanks a lot.
> 

Re: [PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[David Edelsohn]

On 2020/7/28 下午1:25, HAO CHEN GUI via Gcc-patches wrote:
> Hi,
>
> This patch adds non-relative jump table support for 64bit rs6000. It implements ASM_OUTPUT_ADDR_VEC_ELT and corresponding expansion of jump table instruction for 64bit rs6000. We want to put non-relative jump table in data.rel.ro section for rs6000. So I add a new target hook - TARGET_ASM_SELECT_JUMPTABLE_SECTION to choose which section jump table should be put in. It puts the jump table in data.rel.ro for 64bit rs6000. For other platforms, it calls targetm.asm_out.function_rodata_section, just as before. We want to have an option to use non-relative jump table even when PIC flag is set. So I add a new option - "mforce-nonrelative-jumptables" for rs6000.  It enables the feature by default. The option takes effect in target hook - TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC.
>
> static bool
> rs6000_gen_pic_addr_diff_vec (void)
> {
>   return TARGET_32BIT || !rs6000_force_nonrelative_jumptables;
> }
>
> The attachments are the patch diff file and change log file.
>
> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this okay for trunk? Any recommendations? Thanks a lot.

It seems that this will break AIX, especially the select section logic.

Thanks, David

Re: [PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[Segher Boessenkool]

Hi!

As I explained in the other mail, please split this into two: one the
core thing, adding the flag and all code generation (maybe using some
new macros or such for section selection, for example); and the second
patch hooking it up and enabling this by default for powerpc64* (or only
for powerpc64le?)

The first patch should work on all OSes, all endians, all word sizes.

On Tue, Jul 28, 2020 at 01:25:36PM +0800, HAO CHEN GUI via Gcc-patches wrote:
> We want to put non-relative jump 
> table in data.rel.ro section for rs6000. So I add a new target hook - 
> TARGET_ASM_SELECT_JUMPTABLE_SECTION to choose which section jump table 
> should be put in.

That sounds like a good idea, but please put it in a separate patch as
well?  Before everything else.

> It puts the jump table in data.rel.ro for 64bit 
> rs6000. For other platforms, it calls 
> targetm.asm_out.function_rodata_section, just as before.

Maybe that hook should get another parameter, instead?

> We want to have 
> an option to use non-relative jump table even when PIC flag is set. So I 
> add a new option - "mforce-nonrelative-jumptables" for rs6000.

"force" isn't good in a name, and no negatives please (what would
-mno-force-norelative-jumptables mean?)  So just -mrelative-jumptables
then?

> static bool
> rs6000_gen_pic_addr_diff_vec (void)
> {
>   return TARGET_32BIT || !rs6000_force_nonrelative_jumptables;
> }

There is no reason why it cannot work on 32-bit?  It even is more
obviously a win there (for 64-bit the jump table elements are just 32
bits for relative, but 64 otherwise; for 32-bit, you get 32 bits either
way, so that isn't an advantage for relative there).

>         * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Redefine.
>         * config/rs6000/rs6000.c (TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC,TARGET_ASM_SELECT_JUMPTABLE_SECTION): Implement two hooks.

(Space after comma.  Line too long.)

The target macros are not very directly related, so put them in separate
entries please?

> 		* config/rs6000/rs6000.h (CASE_VECTOR_PC_RELATIVE,CASE_VECTOR_MODE) Redefine.

Space after comma, colon after closing paren here.

"Redefine"?  The patch doesn't do that (which is good, we don't want
that).  Ah you mean the patch changes the value it is defined as; your
changelog sounds like it now #undefs it first, which would be nasty :-)

> 		* config/rs6000/rs6000.md (nonrelative_tablejumpdi, nonrelative_tablejumpdi_nospec) Add two expansions.
> 		* config/rs6000/rs6000.opt (mforce-nonrelative-jumptables) Add a new options for rs6000.

Colon, line too long.  Changelog lines are 80 chars max, including the
initial tab (which counts as 8 spaces).

>         * doc/tm.texi.in (TARGET_ASM_SELECT_JUMPTABLE_SECTION): Likewise.

Likewise?  It doesn't do the same thing as the previous line.

>         * doc/tm.texi: Regenerate.
>         * final.c (targetm.asm_out.select_jumptable_section): Replace targetm.asm_out.function_rodata_section with targetm.asm_out.select_jumptable_section in jumptable section selection.
>         * output.h (default_select_jumptable_section): Declare.
>         * target.def (default_select_jumptable_section): Likewise
>         * varasm.c (default_select_jumptable_section): Implementation.


> --- a/gcc/config/rs6000/linux64.h
> +++ b/gcc/config/rs6000/linux64.h
> @@ -324,7 +324,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 (TARGET_64BIT && flag_pic && !rs6000_force_nonrelative_jumptables)

I'm not sure where flag_pic matters here?

> +/* Implement TARGET_ASM_GENERATE_PIC_ADDR_DIFF_VEC. Return false if rs6000_force_nonrelative_jumptables is set and target is 64bit. */

Line too long.  Lines of code are at most 80 chars.

> +/* Implement TARGET_ASM_FUNCTION_RODATA_SECTION. Put non-relative jump table in data.rel.ro section */
> +
> +section *
> +rs6000_select_jumptable_section (tree decl)
> +{
> +  if (TARGET_32BIT)
> +    return default_function_rodata_section (decl);

This shouldn't exclude 32 bit?

> +  if (decl != NULL_TREE && DECL_SECTION_NAME (decl))

Just "if (decl && DECL_SECTION_NAME (decl))" if you ask me...  but some
people disagree, I guess.

> +    {
> +      const char *name = DECL_SECTION_NAME (decl);
> +
> +      if (DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP)
> +        {
> +          const char *dot;
> +          size_t len;
> +          char* rname;
> +
> +          dot = strchr (name + 1, '.');
> +          if (!dot)
> +            dot = name;
> +          len = strlen (dot) + 13;
> +          rname = (char *) alloca (len);
> +
> +          strcpy (rname, ".data.rel.ro");
> +          strcat (rname, dot);
> +          return get_section (rname, SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE, decl);
> +        }
> +        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
> +         else if (DECL_COMDAT_GROUP (decl)
> +               && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
> +        {
> +          size_t len = strlen (name) + 1;
> +          char *rname = (char *) alloca (len);
> +
> +          memcpy (rname, name, len);
> +          rname[14] = 'r';
> +          return get_section (rname, SECTION_LINKONCE, decl);
> +        }
> +        /* For .text.foo we want to use .data.rel.ro.foo.  */
> +         else if (flag_function_sections && flag_data_sections
> +              && strncmp (name, ".text.", 6) == 0)
> +        {
> +          size_t len = strlen (name) + 1;
> +          char *rname = (char *) alloca (len + 7);
> +
> +          memcpy (rname, ".data.rel.ro", 12);
> +          memcpy (rname + 12, name + 5, len - 5);
> +          return get_section (rname, SECTION_WRITE | SECTION_RELRO, decl);
> +        }
> +    }
> +  return get_section (".data.rel.ro", SECTION_WRITE | SECTION_RELRO, decl);
> +}

So this is hugely complicated.  I think most of it is copy/paste from
somewhere else?  Can this be refactored, instead?

> > It puts the jump table in data.rel.ro for 64bit 
> > rs6000. For other platforms, it calls 
> > targetm.asm_out.function_rodata_section, just as before.
> 
> Maybe that hook should get another parameter, instead?

^^^ Perhaps via that?

> -/* Specify the machine mode that this machine uses
> -   for the index in the tablejump instruction.  */
> -#define CASE_VECTOR_MODE SImode

> +#define CASE_VECTOR_MODE (TARGET_32BIT || (flag_pic && !rs6000_force_nonrelative_jumptables) ? SImode : DImode)

That will be just

#define CASE_VECTOR_MODE (rs6000_relative_jumptables ? SImode : Pmode)

or similar?

>  /* 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 TARGET_32BIT

#define CASE_VECTOR_PC_RELATIVE rs6000_relative_jumptables

> +/* This is how to output an element of a case-vector that is non-relative. */
> +#define ASM_OUTPUT_ADDR_VEC_ELT(FILE, VALUE) \
> +  do { char buf[100];                                   \
> +       fputs ("\t.quad ", FILE);                        \
> +       ASM_GENERATE_INTERNAL_LABEL (buf, "L", VALUE);   \
> +       assemble_name (FILE, buf);                       \
> +       putc ('\n', FILE);                              \
> +     } while (0)

That's not how we indent stuff.  We shouldn't have big macros like this
anyway (make a function for it).

Use fprintf please, not the various "put" things.

Why would 100 be enough?

And see David's comment about .quad not existing on all assemblers we
support.


> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -12670,8 +12670,10 @@
>      {
>        if (TARGET_32BIT)
>        	emit_jump_insn (gen_tablejumpsi (operands[0], operands[1]));
> -      else
> +      else if (flag_pic && !rs6000_force_nonrelative_jumptables)
>  	emit_jump_insn (gen_tablejumpdi (operands[0], operands[1]));
> +      else 

No space at end of line please.

> +        emit_jump_insn (gen_nonrelative_tablejumpdi (operands[0], operands[1]));

I don't see why flag_pic matters here, and it should just work for 32-bit
as well.

> +(define_expand "nonrelative_tablejumpdi"
> +  [(set (match_dup 2)
> +        (match_operand:DI 0 "gpc_reg_operand" "r"))
> +   (parallel [(set (pc)
> +                   (match_dup 2))

(Use a tab instead of every 8 leading spaces).

> +mforce-nonrelative-jumptables
> +Target Undocumented Var(rs6000_force_nonrelative_jumptables) Init(1) Save

"Undocumented", good, let's not make this an official option (for always
and ever) unless there turns out to be a reason for that.


This is a pretty big thing to do as your first patch :-)  It will take a
few iterations to get right, don't let it discourage you!

Oh, btw, you don't have an entry in MAINTAINERS yet?


Segher

Re: [PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[Segher Boessenkool]

Hi!

On Thu, Jul 30, 2020 at 04:35:57AM -0400, David Edelsohn wrote:
> The purpose should not be to exclude AIX.  If there is no fundamental
> limitation with use of the new tablejump design on AIX then the patch
> is not acceptable without AIX support.

The patch should work on all subtargets.  It will be controlled by an
(undocumented?) command line option, for easy experimentation, and to
make it easy to (temporarily) disable it for some subtarget.

This change will probably cause some problems on different OSes and with
different binary file formats, so it is useful to be able to control the
default separately for each, to keep things working while we figure
things out.  (It is not just AIX and Darwin, but also 32-bit Linux, the
several BSDs (which are different), bare-metal ports, the embedded OSes,
etc.)

But the goal is for this to work everywhere, yes.

We could default it to non-relative, to see what breaks ;-)

> The patch should use DOUBLE_INT_ASM_OP, not explicit ".quad".  AIX
> always is PIC.  It's not obvious to me why the patch should limit
> support to PPC64 Linux.

It's the only thing that was tested so far (I'm not sure if BE was
tested even there?)

> The section selection seems Linux/ELF
> specific, but the rest seems like a general design optimization for
> all PowerPC-based operating systems.

Yup, needs some work.


This patch should be split into the core piece, which creates the flag
and adds infrastructure etc., and patches for the subtarget-specific
pieces.  It is likely that different subtargets will want to choose
different sections for the jump tables?  But the basic things can work
everywhere, in principle.


Segher

Re: [PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[David Edelsohn]

On Thu, Jul 30, 2020 at 1:22 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>
> David,
>
> Seems there is something wrong with my email box.  I lost your email. I
> reconfigured the box and it should be OK now.
>
> Could you inform me how to exclude AIX from the condition testing? By
> the ABI? Thanks a lot.

The purpose should not be to exclude AIX.  If there is no fundamental
limitation with use of the new tablejump design on AIX then the patch
is not acceptable without AIX support.

The patch should use DOUBLE_INT_ASM_OP, not explicit ".quad".  AIX
always is PIC.  It's not obvious to me why the patch should limit
support to PPC64 Linux.  The section selection seems Linux/ELF
specific, but the rest seems like a general design optimization for
all PowerPC-based operating systems.

If you have questions about AIX details, that's fine. But limiting the
implementation to Linux is not acceptable.  All other ISA and
optimization features added to GCC that are supported on all OSes are
implemented on all OSes.  This tablejump change should be no
different.

Thanks, David


Thanks, David


Thanks, David

[PATCH, rs6000] Add non-relative jump table support for 64bit rs6000

[HAO CHEN GUI]

David,

Seems there is something wrong with my email box.  I lost your email. I 
reconfigured the box and it should be OK now.

Could you inform me how to exclude AIX from the condition testing? By 
the ABI? Thanks a lot.

Haochen Gui