[patch] AVR: Let -mrelax elide global RJMP .+0

Georg-Johann Lay via Binutils <[email protected]>
Newsgroups gmane.comp.gnu.binutils
Message-ID <[email protected]>
This patch extends the avr-specific linker relaxation machine to
elide trivial RJMP instructions.  These are RJMP instructions
with a jump offset of 0 that are targeting a global symbol.

The use case is when a function is tail-calling some other function,
and the latter happens to be located right after the former, like in:

    rjmp  func     ;; from module A
    .global func   ;; from module B
    func:

Plain RJMP .+0 that don't target a global symbol are not touched
since they are emit by __builtin_avr_delay_cycles (2) etc.

The feature can be turned off with the new ld option --no-elide-rjmp0.

The patch passes without new regressions when run in the AVR GCC
testsuite with --tool_opts="-mrelax".

Ok for trunk?

Johann

--

bfd/
	* elf32-avr.c (avr_elide_rjmp0): New static variable.
	(elf32_avr_setup_params): Set it.
	(elf32_avr_relax_section) <sym_is_global>: New variable.
	[avr_elide_rjmp0 && avr_is_RJMP]: Elide global RJMPs with
	a jump offset of 0.
	* elf32-avr.h (elf32_avr_setup_params): Adjust interface.
ld/
	* ldlex.h (enum option_values) [emultempl/avrelf.em]: Add
	OPTION_NO_ELIDE_RJMP0.
	* emultempl/avrelf.em (avr_elide_rjmp0): New static variable.
	(avr_elf_set_global_bfd_parameters): Pass it.
	(PARSE_AND_LIST_LONGOPTS) <--no-elide-rjmp0>: Add and document.
	(PARSE_AND_LIST_ARGS_CASES) [OPTION_NO_ELIDE_RJMP0]: Set
	avr_elide_rjmp0 to false.
binutils-elide-rjmp0.diff (text/x-patch, 9.7 KB)
    AVR: Let -mrelax elide global RJMP .+0
    
    This patch extends the avr-specific linker relaxation machine to
    elide trivial RJMP instructions.  These are RJMP instructions
    with a jump offset of 0 that are targeting a global symbol.
    
    The use case is when a function is tail-calling some other function,
    and the latter happens to be located right after the former, like in:
    
       rjmp  func    ; from module A
       .global func  ; from module B
       func:
    
    Plain RJMP .+0 that don't target a global symbol are not touched
    since they are emit by __builtin_avr_delay_cycles (2) etc.
    
    The feature can be turned off with the new ld option --no-elide-rjmp0.
    
    bfd/
            * elf32-avr.h (elf32_avr_setup_params): Adjust interface.
            * elf32-avr.c (avr_elide_rjmp0): New static variable.
            (elf32_avr_setup_params): Set it.
            (elf32_avr_relax_section) <sym_is_global>: New variable.
            [avr_elide_rjmp0 && avr_is_RJMP]: Elide global RJMPs with
            a jump offset of 0.
    ld/
            * ldlex.h (enum option_values) [emultempl/avrelf.em]: Add
            OPTION_NO_ELIDE_RJMP0.
            * emultempl/avrelf.em (avr_elide_rjmp0): New static variable.
            (avr_elf_set_global_bfd_parameters): Pass it.
            (PARSE_AND_LIST_LONGOPTS) <--no-elide-rjmp0>: Add and document.
            (PARSE_AND_LIST_ARGS_CASES) [OPTION_NO_ELIDE_RJMP0]: Set
            avr_elide_rjmp0 to false.

diff --git a/bfd/elf32-avr.c b/bfd/elf32-avr.c
index 1d0298d57c3..70d2b347ba0 100644
--- a/bfd/elf32-avr.c
+++ b/bfd/elf32-avr.c
@@ -729,6 +729,12 @@ static bfd_vma avr_pc_wrap_around = 0x10000000;
    machine will try to optimize CALL/RET sequences by a single jump
    instruction. This can be switched off by --no-call-ret-replacement.  */
 static bool avr_replace_call_ret_sequences = true;
+
+/* If this variable holds true, the linker relaxation machine will
+   try to remove RJMP instructions that are void.  This does not
+   include plain RJMP .+0 which is used by GCC to delay 2 cycles.
+   This can be switched off by --no-elide-rjmp0.  */
+static bool avr_elide_rjmp0 = true;
 
 
 /* Per-section relaxation related information for avr.  */
@@ -2563,7 +2569,11 @@ avr_reloc_at (bfd *abfd, Elf_Internal_Shdr *symtab_hdr,
 
    The .jumptables section is meant to be used for a future tablejump variant
    for the devices with 3-byte program counter where the table itself contains
-   4-byte jump instructions whose relative offset must not be changed.  */
+   4-byte jump instructions whose relative offset must not be changed.
+
+   Finally, we elide RJMP instructions that are void and not a delay.
+   This may occur when a function is tailcalling some other function,
+   and the latter happens to be located right after the former.  */
 
 static bool
 elf32_avr_relax_section (bfd *abfd,  asection *sec,
@@ -2657,6 +2667,7 @@ elf32_avr_relax_section (bfd *abfd,  asection *sec,
   for (irel = internal_relocs; irel < irelend; irel++)
     {
       bfd_vma symval;
+      bool sym_is_global = false;
 
       if (ELF32_R_TYPE (irel->r_info) != R_AVR_13_PCREL
 	  && ELF32_R_TYPE (irel->r_info) != R_AVR_7_PCREL
@@ -2720,6 +2731,7 @@ elf32_avr_relax_section (bfd *abfd,  asection *sec,
 	  symval = (h->root.u.def.value
 		    + h->root.u.def.section->output_section->vma
 		    + h->root.u.def.section->output_offset);
+	  sym_is_global = true;
 	}
 
       /* For simplicity of coding, we are going to modify the section
@@ -2900,6 +2912,84 @@ elf32_avr_relax_section (bfd *abfd,  asection *sec,
 		*again = true;
 		break;
 	      }
+	    else if (avr_elide_rjmp0
+		     // Elide no-op RJMP tail calls like in
+		     //    RJMP func     ;; in module A
+		     //    .global func  ;; in module B
+		     //    func:
+		     && avr_is_RJMP (code_word)
+		     // Plain RJMP .+0 is used by GCC to delay 2 cycles, thus
+		     // we are only interested in global jump targets...
+		     && sym_is_global
+		     // ...without offset, and...
+		     && irel->r_addend == 0
+		     // ...where the RJMP targets the insn directly after it.
+		     && symval + irel->r_addend == dot + 2)
+	      {
+		if (debug_relax)
+		  printf ("found rjmp .+0 at address 0x%x in section %s\n",
+			  (int) dot, sec->name);
+
+		const bool has_prev = irel->r_offset >= 2;
+		const uint16_t prev_word = has_prev
+		  ? avr_word (abfd, contents + irel->r_offset - 2)
+		  : 0;
+
+		// The assumption in the following condition is that there is
+		// no dangling skip at the end of a section.  Note that a skip
+		// insn at that place doesn't make sense in a real program.
+		if (has_prev
+		    && avr_is_skip (prev_word))
+		  {
+		    if (debug_relax)
+		      printf ("skip insn prevents deletion of rjmp .+0 at "
+			      "address 0x%x\n", (int) dot);
+		    break;
+		  }
+
+		// Avoid the paranoid case where the RJMP is at the end of
+		// the program memory and jumps to 0x0.  We don't have the
+		// flash size handy, so assume a size of 0.5 KiB.
+		if ((dot + 2) % 0x200 == 0)
+		  {
+		    if (debug_relax)
+		      printf ("not deleting rjmp .+0 at address 0x%x that may "
+			      "be at the end of program memory\n", (int) dot);
+		    break;
+		  }
+
+		// Ditch the RJMP.
+		// Notice that labels or relocs at the RJMP are no issue.
+
+		if (debug_relax)
+		  printf ("deleted rjmp .+0 instruction at address 0x%x\n",
+			  (int) dot);
+
+		// Read this BFD's local symbols if we haven't done so already.
+		if (isymbuf == NULL && symtab_hdr->sh_info != 0)
+		  {
+		    isymbuf = avr_read_symbuf (abfd, symtab_hdr);
+		    if (isymbuf == NULL)
+		      break;
+		  }
+
+		elf_section_data (sec)->relocs = internal_relocs;
+		elf_section_data (sec)->this_hdr.contents = contents;
+		symtab_hdr->contents = (unsigned char *) isymbuf;
+
+		// Delete the two RJMP bytes, and...
+		if (!elf32_avr_relax_delete_bytes (abfd, sec,
+						   irel->r_offset, 2, true))
+		  goto error_return;
+
+		// ...decommission the reloc.
+		irel->r_info = R_AVR_NONE;
+
+		// That will change things, so we should relax again.
+		// Note that this is not required, and it may be slow.
+		*again = true;
+		break;
+	      }
 	    else if (avr_is_RJMP (code_word)
 		     || avr_is_JMP (code_word))
 	      {
@@ -3327,7 +3417,8 @@ void
 elf32_avr_setup_params (struct bfd_link_info *info, bfd *avr_stub_bfd,
 			asection *avr_stub_section,
 			bool no_stubs, bool deb_stubs, bool deb_relax,
-			bfd_vma pc_wrap_around, bool call_ret_replacement)
+			bfd_vma pc_wrap_around, bool call_ret_replacement,
+			bool elide_rjmp0)
 {
   elf32_avr_link_hash_table_t *htab = avr_link_hash_table (info);
 
@@ -3341,6 +3432,7 @@ elf32_avr_setup_params (struct bfd_link_info *info, bfd *avr_stub_bfd,
   debug_stubs = deb_stubs;
   avr_pc_wrap_around = pc_wrap_around;
   avr_replace_call_ret_sequences = call_ret_replacement;
+  avr_elide_rjmp0 = elide_rjmp0;
 }
 
 
diff --git a/bfd/elf32-avr.h b/bfd/elf32-avr.h
index 2b9079bc11e..1f06e608402 100644
--- a/bfd/elf32-avr.h
+++ b/bfd/elf32-avr.h
@@ -24,7 +24,7 @@
 /* These four functions will be called from the ld back end.  */
 
 extern void elf32_avr_setup_params (struct bfd_link_info *, bfd *, asection *,
-				    bool, bool, bool, bfd_vma, bool);
+				    bool, bool, bool, bfd_vma, bool, bool);
 extern int elf32_avr_setup_section_lists (bfd *, struct bfd_link_info *);
 extern bool elf32_avr_size_stubs (bfd *, struct bfd_link_info *, bool);
 extern bool elf32_avr_build_stubs (struct bfd_link_info *);
diff --git a/ld/emultempl/avrelf.em b/ld/emultempl/avrelf.em
index 71e035b74a1..cd3c66a3563 100644
--- a/ld/emultempl/avrelf.em
+++ b/ld/emultempl/avrelf.em
@@ -43,6 +43,7 @@ static bool avr_no_stubs = false;
 static bool avr_debug_relax = false;
 static bool avr_debug_stubs = false;
 static bool avr_replace_call_ret_sequences = true;
+static bool avr_elide_rjmp0 = true;
 static bfd_vma avr_pc_wrap_around = 0x10000000;
 
 /* Transfers information to the bfd frontend.  */
@@ -57,7 +58,8 @@ avr_elf_set_global_bfd_parameters (void)
 			  avr_debug_stubs,
 			  avr_debug_relax,
 			  avr_pc_wrap_around,
-			  avr_replace_call_ret_sequences);
+			  avr_replace_call_ret_sequences,
+			  avr_elide_rjmp0);
 }
 
 
@@ -235,6 +237,8 @@ EOF
 PARSE_AND_LIST_LONGOPTS='
   { "no-call-ret-replacement", no_argument,
     NULL, OPTION_NO_CALL_RET_REPLACEMENT},
+  { "no-elide-rjmp0", no_argument,
+    NULL, OPTION_NO_ELIDE_RJMP0},
   { "pmem-wrap-around", required_argument,
     NULL, OPTION_PMEM_WRAP_AROUND},
   { "no-stubs", no_argument,
@@ -260,6 +264,14 @@ PARSE_AND_LIST_OPTIONS='
 		   "  instructions by a single jump instruction.\n"
 		   "                              "
 		   "  This option disables this optimization.\n"));
+  fprintf (file, _("  --no-elide-rjmp0   "
+		   "The relaxation machine normally will\n"
+		   "                              "
+		   "  remove an rjmp instruction when it targets a\n"
+		   "                              "
+		   "  global symbol at a jump offset of 0.\n"
+		   "                              "
+		   "  This option disables this optimization.\n"));
   fprintf (file, _("  --no-stubs                  "
 		   "If the linker detects to attempt to access\n"
 		   "                              "
@@ -310,6 +322,13 @@ PARSE_AND_LIST_ARGS_CASES='
 	avr_replace_call_ret_sequences = false;
       }
       break;
+
+    case OPTION_NO_ELIDE_RJMP0:
+      {
+	/* This variable is defined in the bfd library.  */
+	avr_elide_rjmp0 = false;
+      }
+      break;
 '
 
 #
diff --git a/ld/ldlex.h b/ld/ldlex.h
index 43b0d9d41d1..b4f1bffd2a7 100644
--- a/ld/ldlex.h
+++ b/ld/ldlex.h
@@ -270,6 +270,7 @@ enum option_values
   OPTION_IN_IMPLIB,
   /* Used by emultempl/avrelf.em.  */
   OPTION_NO_CALL_RET_REPLACEMENT,
+  OPTION_NO_ELIDE_RJMP0,
   OPTION_PMEM_WRAP_AROUND,
   OPTION_NO_STUBS,
   OPTION_DEBUG_STUBS,
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.