| Message ID | c9def580-6777-66c8-2623-1b8df8c61079@gjlay.de |
|---|---|
| State | New |
| Headers | show |
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. >
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.
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. > >
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. >> >> >> >
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