[patch,avr] PR81075: Move jump-tables out of .text

[patch,avr] PR81075: Move jump-tables out of .text

[Georg-Johann Lay]

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

Hi,

Since PR71151 we have jump-tables in .text so that branches
crossing the tables have longer offsets that needed.

This moves jump-tables out of test again, but not into
.progmem.gcc_sw_tables like before PR71151, but into
the currently unused but existing .jumptables.

Since PR63223 there is no restriction on the location
of jump-tables, they can even reside above 128KiB without
problems.

Also adds -mlog=insn_addresses to dump insn addresses
as asm comments before respective instruction.

The patch implements ASM_OUTPUT_ADDR_VEC so that avr.c
gains full control over the table generation.

Tested on ATmega2560.

Ok to apply?

Johann


gcc/
	Move jump-tables out of .text again.

	PR target/81075
	* config/avr/avr.c (ASM_OUTPUT_ADDR_VEC_ELT): Remove function.
	(ASM_OUTPUT_ADDR_VEC): New function.
	(avr_adjust_insn_length) [JUMP_TABLE_DATA_P]: Return 0.
	(avr_final_prescan_insn) [avr_log.insn_addresses]: Dump
	INSN_ADDRESSes as asm comment.
	* config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Adjust comment.
	(ASM_OUTPUT_ADDR_VEC_ELT): Remove define.
	(ASM_OUTPUT_ADDR_VEC): Define to avr_output_addr_vec.
	* config/avr/avr.md (*tablejump): Adjust comment.
	* config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Remove.
	* config/avr/avr-log.c (avr_log_set_avr_log) <insn_addresses>:
	New detail.
	* config/avr/avr-protos.h (avr_output_addr_vec_elt): Remove proto.
	(avr_output_addr_vec): New proto.
	(avr_log_t) <insn_addresses>: New field.

[-- Attachment #2: pr81075-jumptable.diff --]
[-- Type: text/x-patch, Size: 7759 bytes --]

Index: config/avr/avr-log.c
===================================================================
--- config/avr/avr-log.c	(revision 249121)
+++ config/avr/avr-log.c	(working copy)
@@ -302,6 +302,7 @@ avr_log_set_avr_log (void)
       SET_DUMP_DETAIL (address_cost);
       SET_DUMP_DETAIL (builtin);
       SET_DUMP_DETAIL (constraints);
+      SET_DUMP_DETAIL (insn_addresses);
       SET_DUMP_DETAIL (legitimate_address_p);
       SET_DUMP_DETAIL (legitimize_address);
       SET_DUMP_DETAIL (legitimize_reload_address);
Index: config/avr/avr-protos.h
===================================================================
--- config/avr/avr-protos.h	(revision 249121)
+++ config/avr/avr-protos.h	(working copy)
@@ -87,7 +87,7 @@ extern bool avr_emit_movmemhi (rtx*);
 extern int avr_epilogue_uses (int regno);
 extern int avr_starting_frame_offset (void);
 
-extern void avr_output_addr_vec_elt (FILE *stream, int value);
+extern void avr_output_addr_vec (rtx_insn*, rtx);
 extern const char *avr_out_sbxx_branch (rtx_insn *insn, rtx operands[]);
 extern const char* avr_out_bitop (rtx, rtx*, int*);
 extern const char* avr_out_plus (rtx, rtx*, int* =NULL, int* =NULL, bool =true);
@@ -175,6 +175,7 @@ typedef struct
   unsigned address_cost :1;
   unsigned builtin :1;
   unsigned constraints :1;
+  unsigned insn_addresses :1;
   unsigned legitimate_address_p :1;
   unsigned legitimize_address :1;
   unsigned legitimize_reload_address :1;
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 249124)
+++ config/avr/avr.c	(working copy)
@@ -3088,6 +3088,10 @@ avr_final_prescan_insn (rtx_insn *insn,
                  rtx_cost (PATTERN (insn), VOIDmode, INSN, 0,
                            optimize_insn_for_speed_p()));
     }
+
+  if (avr_log.insn_addresses)
+    fprintf (asm_out_file, ";; ADDR = %d\n",
+             (int) INSN_ADDRESSES (INSN_UID (insn)));
 }
 
 /* Return 0 if undefined, 1 if always true or always false.  */
@@ -9138,6 +9142,12 @@ avr_adjust_insn_length (rtx_insn *insn,
   rtx *op = recog_data.operand;
   enum attr_adjust_len adjust_len;
 
+  /* As we pretend jump tables in .text, fix branch offsets crossing jump
+     tables now.  */
+
+  if (JUMP_TABLE_DATA_P (insn))
+    return 0;
+
   /* Some complex insns don't need length adjustment and therefore
      the length need not/must not be adjusted for these insns.
      It is easier to state this in an insn attribute "adjust_len" than
@@ -12309,17 +12319,88 @@ avr_out_reload_inpsi (rtx *op, rtx clobb
 }
 
 
-/* Worker function for `ASM_OUTPUT_ADDR_VEC_ELT'.  */
+/* Worker function for `ASM_OUTPUT_ADDR_VEC'.  */
+/* Emit jump tables out-of-line so that branches crossing the table
+   get shorter offsets.  If we have JUMP + CALL, then put the tables
+   in a dedicated non-.text section so that CALLs get better chance to
+   be relaxed to RCALLs.
+
+   We emit the tables by hand because `function_rodata_section' does not
+   work as expected, cf. PR71151, and we do *NOT* want the table to be
+   in .rodata, hence setting JUMP_TABLES_IN_TEXT_SECTION = 0 is of limited
+   use; and setting it to 1 attributes table lengths to branch offsets...
+   Moreover, fincal.c keeps switching section before each table entry
+   which we find too fragile as to rely on section caching.  */
 
 void
-avr_output_addr_vec_elt (FILE *stream, int value)
+avr_output_addr_vec (rtx_insn *labl, rtx table)
 {
-  if (AVR_HAVE_JMP_CALL)
-    fprintf (stream, "\t.word gs(.L%d)\n", value);
+  FILE *stream = asm_out_file;
+
+  app_disable();
+
+  // Switch to appropriate (sub)section.
+
+  if (DECL_SECTION_NAME (current_function_decl)
+      && symtab_node::get (current_function_decl)
+      && ! symtab_node::get (current_function_decl)->implicit_section)
+    {
+      // .subsection will emit the code after the function and in the
+      // section as chosen by the user.
+
+      switch_to_section (current_function_section ());
+      fprintf (stream, "\t.subsection\t1\n");
+    }
   else
-    fprintf (stream, "\trjmp .L%d\n", value);
+    {
+      // Since PR63223 there is no restriction where to put the table; it
+      // may even reside above 128 KiB.  We put it in a section as high as
+      // possible and avoid progmem in order not to waste flash <= 64 KiB.
+
+      const char *sec_name = ".jumptables.gcc";
+
+      // The table belongs to its host function, therefore use fine
+      // grained sections so that, if that function is removed by
+      // --gc-sections, the child table(s) may also be removed.  */
+
+      tree asm_name = DECL_ASSEMBLER_NAME (current_function_decl);
+      const char *fname = IDENTIFIER_POINTER (asm_name);
+      fname = targetm.strip_name_encoding (fname);
+      sec_name = ACONCAT ((sec_name, ".", fname, NULL));
+
+      fprintf (stream, "\t.section\t%s,\"%s\",@progbits\n", sec_name,
+               AVR_HAVE_JMP_CALL ? "a" : "ax");
+    }
+
+  // Output the label that preceeds the table.
+
+  ASM_OUTPUT_ALIGN (stream, 1);
+  targetm.asm_out.internal_label (stream, "L", CODE_LABEL_NUMBER (labl));
+
+  // Output the table's content.
+
+  int vlen = XVECLEN (table, 0);
+
+  for (int idx = 0; idx < vlen; idx++)
+    {
+      int value = CODE_LABEL_NUMBER (XEXP (XVECEXP (table, 0, idx), 0));
+
+      if (AVR_HAVE_JMP_CALL)
+        fprintf (stream, "\t.word gs(.L%d)\n", value);
+      else
+        fprintf (stream, "\trjmp .L%d\n", value);
+    }
+
+  // Switch back to original section.  As we clobbered the section above,
+  // forget the current section before switching back.
+
+  in_section = NULL;
+  switch_to_section (current_function_section ());
 }
 
+
+/* Implement `TARGET_CONDITIONAL_REGISTER_USAGE'.  */
+
 static void
 avr_conditional_register_usage (void)
 {
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 249124)
+++ config/avr/avr.h	(working copy)
@@ -398,6 +398,10 @@ typedef struct avr_args
 
 #define SUPPORTS_INIT_PRIORITY 0
 
+/* We pretend jump tables are in text section because otherwise,
+   final.c will switch to .rodata before jump tables and thereby
+   triggers __do_copy_data.  As we implement ASM_OUTPUT_ADDR_VEC,
+   we still have full control over the jump tables themselves.  */
 #define JUMP_TABLES_IN_TEXT_SECTION 1
 
 #define ASM_COMMENT_START " ; "
@@ -447,8 +451,8 @@ typedef struct avr_args
   fprintf (STREAM, "\tpop\tr%d", REGNO);	\
 }
 
-#define ASM_OUTPUT_ADDR_VEC_ELT(STREAM, VALUE)  \
-  avr_output_addr_vec_elt (STREAM, VALUE)
+#define ASM_OUTPUT_ADDR_VEC(TLABEL, TDATA)      \
+  avr_output_addr_vec (TLABEL, TDATA)
 
 #define ASM_OUTPUT_ALIGN(STREAM, POWER)                 \
   do {                                                  \
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 249121)
+++ config/avr/avr.md	(working copy)
@@ -5185,7 +5185,7 @@ (define_insn "*indirect_jump"
    (set_attr "cc" "none")])
 
 ;; table jump
-;; For entries in jump table see avr_output_addr_vec_elt.
+;; For entries in jump table see avr_output_addr_vec.
 
 ;; Table made from
 ;;    "rjmp .L<n>"   instructions for <= 8K devices
Index: config/avr/elf.h
===================================================================
--- config/avr/elf.h	(revision 249121)
+++ config/avr/elf.h	(working copy)
@@ -31,11 +31,6 @@
 #undef STRING_LIMIT
 #define STRING_LIMIT ((unsigned) 64)
 
-/* Output alignment 2**1 for jump tables.  */
-#undef ASM_OUTPUT_BEFORE_CASE_LABEL
-#define ASM_OUTPUT_BEFORE_CASE_LABEL(FILE, PREFIX, NUM, TABLE) \
-  ASM_OUTPUT_ALIGN (FILE, 1);
-
 /* Be conservative in crtstuff.c.  */
 #undef INIT_SECTION_ASM_OP
 #undef FINI_SECTION_ASM_OP

[patch,avr][ping #1] PR81075: Move jump-tables out of .text

[Georg-Johann Lay]

Ping #1

http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html

Georg-Johann Lay schrieb:
> Hi,
> 
> Since PR71151 we have jump-tables in .text so that branches
> crossing the tables have longer offsets that needed.
> 
> This moves jump-tables out of test again, but not into
> .progmem.gcc_sw_tables like before PR71151, but into
> the currently unused but existing .jumptables.
> 
> Since PR63223 there is no restriction on the location
> of jump-tables, they can even reside above 128KiB without
> problems.
> 
> Also adds -mlog=insn_addresses to dump insn addresses
> as asm comments before respective instruction.
> 
> The patch implements ASM_OUTPUT_ADDR_VEC so that avr.c
> gains full control over the table generation.
> 
> Tested on ATmega2560.
> 
> Ok to apply?
> 
> Johann
> 
> 
> gcc/
>     Move jump-tables out of .text again.
> 
>     PR target/81075
>     * config/avr/avr.c (ASM_OUTPUT_ADDR_VEC_ELT): Remove function.
>     (ASM_OUTPUT_ADDR_VEC): New function.
>     (avr_adjust_insn_length) [JUMP_TABLE_DATA_P]: Return 0.
>     (avr_final_prescan_insn) [avr_log.insn_addresses]: Dump
>     INSN_ADDRESSes as asm comment.
>     * config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Adjust comment.
>     (ASM_OUTPUT_ADDR_VEC_ELT): Remove define.
>     (ASM_OUTPUT_ADDR_VEC): Define to avr_output_addr_vec.
>     * config/avr/avr.md (*tablejump): Adjust comment.
>     * config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Remove.
>     * config/avr/avr-log.c (avr_log_set_avr_log) <insn_addresses>:
>     New detail.
>     * config/avr/avr-protos.h (avr_output_addr_vec_elt): Remove proto.
>     (avr_output_addr_vec): New proto.
>     (avr_log_t) <insn_addresses>: New field.
> 

[patch,avr][Ping #2] PR81075: Move jump-tables out of .text

[Georg-Johann Lay]

Ping #2

http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html

On 14.06.2017 14:03, Georg-Johann Lay wrote:
> Hi,
> 
> Since PR71151 we have jump-tables in .text so that branches
> crossing the tables have longer offsets that needed.
> 
> This moves jump-tables out of test again, but not into
> .progmem.gcc_sw_tables like before PR71151, but into
> the currently unused but existing .jumptables.
> 
> Since PR63223 there is no restriction on the location
> of jump-tables, they can even reside above 128KiB without
> problems.
> 
> Also adds -mlog=insn_addresses to dump insn addresses
> as asm comments before respective instruction.
> 
> The patch implements ASM_OUTPUT_ADDR_VEC so that avr.c
> gains full control over the table generation.
> 
> Tested on ATmega2560.
> 
> Ok to apply?
> 
> Johann
> 
> 
> gcc/
>      Move jump-tables out of .text again.
> 
>      PR target/81075
>      * config/avr/avr.c (ASM_OUTPUT_ADDR_VEC_ELT): Remove function.
>      (ASM_OUTPUT_ADDR_VEC): New function.
>      (avr_adjust_insn_length) [JUMP_TABLE_DATA_P]: Return 0.
>      (avr_final_prescan_insn) [avr_log.insn_addresses]: Dump
>      INSN_ADDRESSes as asm comment.
>      * config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Adjust comment.
>      (ASM_OUTPUT_ADDR_VEC_ELT): Remove define.
>      (ASM_OUTPUT_ADDR_VEC): Define to avr_output_addr_vec.
>      * config/avr/avr.md (*tablejump): Adjust comment.
>      * config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Remove.
>      * config/avr/avr-log.c (avr_log_set_avr_log) <insn_addresses>:
>      New detail.
>      * config/avr/avr-protos.h (avr_output_addr_vec_elt): Remove proto.
>      (avr_output_addr_vec): New proto.
>      (avr_log_t) <insn_addresses>: New field.

Re: [patch,avr][Ping #3] PR81075: Move jump-tables out of .text

[Georg-Johann Lay]

Ping #3

http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html

As avr maintainers are off-line, would a global maintainer have
a look at this?

Thanks,

Johann



On 27.06.2017 12:01, Georg-Johann Lay wrote:
> Ping #2
> 
> http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html
> 
> On 14.06.2017 14:03, Georg-Johann Lay wrote:
>> Hi,
>>
>> Since PR71151 we have jump-tables in .text so that branches
>> crossing the tables have longer offsets that needed.
>>
>> This moves jump-tables out of test again, but not into
>> .progmem.gcc_sw_tables like before PR71151, but into
>> the currently unused but existing .jumptables.
>>
>> Since PR63223 there is no restriction on the location
>> of jump-tables, they can even reside above 128KiB without
>> problems.
>>
>> Also adds -mlog=insn_addresses to dump insn addresses
>> as asm comments before respective instruction.
>>
>> The patch implements ASM_OUTPUT_ADDR_VEC so that avr.c
>> gains full control over the table generation.
>>
>> Tested on ATmega2560.
>>
>> Ok to apply?
>>
>> Johann
>>
>>
>> gcc/
>>      Move jump-tables out of .text again.
>>
>>      PR target/81075
>>      * config/avr/avr.c (ASM_OUTPUT_ADDR_VEC_ELT): Remove function.
>>      (ASM_OUTPUT_ADDR_VEC): New function.
>>      (avr_adjust_insn_length) [JUMP_TABLE_DATA_P]: Return 0.
>>      (avr_final_prescan_insn) [avr_log.insn_addresses]: Dump
>>      INSN_ADDRESSes as asm comment.
>>      * config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Adjust comment.
>>      (ASM_OUTPUT_ADDR_VEC_ELT): Remove define.
>>      (ASM_OUTPUT_ADDR_VEC): Define to avr_output_addr_vec.
>>      * config/avr/avr.md (*tablejump): Adjust comment.
>>      * config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Remove.
>>      * config/avr/avr-log.c (avr_log_set_avr_log) <insn_addresses>:
>>      New detail.
>>      * config/avr/avr-protos.h (avr_output_addr_vec_elt): Remove proto.
>>      (avr_output_addr_vec): New proto.
>>      (avr_log_t) <insn_addresses>: New field.
> 
> 

Re: [patch,avr][Ping #3] PR81075: Move jump-tables out of .text

[Denis Chertykov]

I'm sorry for so long delay.

Please apply the patch.

2017-07-05 14:19 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> Ping #3
>
> http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html
>
> As avr maintainers are off-line, would a global maintainer have
> a look at this?
>
> Thanks,
>
> Johann
>
>
>
>
> On 27.06.2017 12:01, Georg-Johann Lay wrote:
>>
>> Ping #2
>>
>> http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01029.html
>>
>> On 14.06.2017 14:03, Georg-Johann Lay wrote:
>>>
>>> Hi,
>>>
>>> Since PR71151 we have jump-tables in .text so that branches
>>> crossing the tables have longer offsets that needed.
>>>
>>> This moves jump-tables out of test again, but not into
>>> .progmem.gcc_sw_tables like before PR71151, but into
>>> the currently unused but existing .jumptables.
>>>
>>> Since PR63223 there is no restriction on the location
>>> of jump-tables, they can even reside above 128KiB without
>>> problems.
>>>
>>> Also adds -mlog=insn_addresses to dump insn addresses
>>> as asm comments before respective instruction.
>>>
>>> The patch implements ASM_OUTPUT_ADDR_VEC so that avr.c
>>> gains full control over the table generation.
>>>
>>> Tested on ATmega2560.
>>>
>>> Ok to apply?
>>>
>>> Johann
>>>
>>>
>>> gcc/
>>>      Move jump-tables out of .text again.
>>>
>>>      PR target/81075
>>>      * config/avr/avr.c (ASM_OUTPUT_ADDR_VEC_ELT): Remove function.
>>>      (ASM_OUTPUT_ADDR_VEC): New function.
>>>      (avr_adjust_insn_length) [JUMP_TABLE_DATA_P]: Return 0.
>>>      (avr_final_prescan_insn) [avr_log.insn_addresses]: Dump
>>>      INSN_ADDRESSes as asm comment.
>>>      * config/avr/avr.h (JUMP_TABLES_IN_TEXT_SECTION): Adjust comment.
>>>      (ASM_OUTPUT_ADDR_VEC_ELT): Remove define.
>>>      (ASM_OUTPUT_ADDR_VEC): Define to avr_output_addr_vec.
>>>      * config/avr/avr.md (*tablejump): Adjust comment.
>>>      * config/avr/elf.h (ASM_OUTPUT_BEFORE_CASE_LABEL): Remove.
>>>      * config/avr/avr-log.c (avr_log_set_avr_log) <insn_addresses>:
>>>      New detail.
>>>      * config/avr/avr-protos.h (avr_output_addr_vec_elt): Remove proto.
>>>      (avr_output_addr_vec): New proto.
>>>      (avr_log_t) <insn_addresses>: New field.
>>
>>
>>
>