RE: [PATCH] Convert MIPS JR t9 to B

["Fu, Chao-Ying" <fu at mips dot com>]

Richard Sandiford wrote:
> 
> Looks good.  One thing though...
> 
> I think the name here is confusing, since there's no JALRX 
> instruction.
> How about renaming require_jalxp to something like 
> "cross_mode_jump_p",
> and including the new R_MIPS_JALR check in there?  (Bonus points for
> removing the redundant brackets around the existing "(r_type 
> == R_MIPS16_26)"
> and "(r_type == R_MIPS_26)" checks while you're there.)
> 
> Then add:
> 
> static inline bfd_boolean
> jal_reloc_p (int r_type)
> {
>   return r_type == R_MIPS_26 || r_type == R_MIPS16_26;
> }
> 
> before _bfd_mips16_elf_reloc_unshuffle and change:
> 
>   /* If required, turn JAL into JALX.  */
>   if (require_jalx)
>     {
> 
> to:
> 
>   /* If required, turn JAL into JALX.  */
>   if (cross_mode_jump_p && jal_reloc_p (r_type))
>     {
> 
> You can then continue to use a single mode-change check for all
> three transformations (JAL->BAL, JALR->BAL, JR->B), rather than
> having to check different booleans for each case.
> 
> The comment:
> 
>    REQUIRE_JALXP indicates whether or not the opcode used with this
>    relocation must be JALX.
> 
> can become something like:
> 
>    On exit, set *CROSS_MODE_JUMP_P to true if the relocation field
>    is a MIPS16 jump to non-MIPS16 code, or vice versa.
> 
> And you could change the comment:
> 
>    If REQUIRE_JALX is TRUE, then the opcode used
>    for the relocation must be either JAL or JALX, and it is
>    unconditionally converted to JALX.
> 
> to:
> 
>    CROSS_MODE_P is true if the relocation field
>    is a MIPS16 jump to non-MIPS16 code, or vice versa.
> 
> Be sure to update the local variable name in
> _bfd_mips_elf_relocate_section too.
> 
> OK with those changes, thanks.
> 
> Richard
> 

  Thanks a lot!
  Here is the updated patch for elfxx-mips.c
Retested Binutils/GAS/LD on Debain (mipsel-linux-gnu).
Ok to apply?

Regards,
Chao-ying


bfd/ChangeLog

2010-01-12  Chao-ying Fu  <fu@mips.com>

	* elfxx-mips.c (JR_TO_B_P): New define to transform JR to B.
	It is true for all CPUs.
	(jal_reloc_p): New function.
	(mips_elf_calculate_relocation): Rename require_jalxp to cross_mode_jump_p.
	Update comment for CROSS_MODE_JUMP_P.
	Set up cross_mode_jump_p based on the mode change.
	(mips_elf_perform_relocation): Rename require_jalx to cross_mode_jump_p.
	Update comment for CROSS_MODE_JUMP_P.
	Test cross_mode_jump_p and jal_reloc_p to turn jal to jalx.
	Use !cross_mode_jump_p to guard conversion.
	Convert "jr t9" to "b", if possible.
	(_bfd_mips_elf_relocate_section): Rename require_jalx to cross_mode_jump_p.
	Pass &cross_mode_jump_p to call mips_elf_calculate_relocation.
	Pass cross_mode_jump_p to call mips_elf_perform_relocation.

Index: elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.264
diff -u -p -r1.264 elfxx-mips.c
--- elfxx-mips.c	11 Dec 2009 13:42:04 -0000	1.264
+++ elfxx-mips.c	12 Jan 2010 22:07:23 -0000
@@ -680,6 +680,11 @@ static bfd *reldyn_sorting_bfd;
    all CPUs.  */
 #define JALR_TO_BAL_P(abfd) 1
 
+/* True if ABFD is for CPUs that are faster if JR is converted to B.
+   This should be safe for all architectures.  We enable this predicate for
+   all CPUs.  */
+#define JR_TO_B_P(abfd) 1
+
 /* True if ABFD is a PIC object.  */
 #define PIC_OBJECT_P(abfd) \
   ((elf_elfheader (abfd)->e_flags & EF_MIPS_PIC) != 0)
@@ -1871,6 +1876,12 @@ mips16_call_reloc_p (int r_type)
   return r_type == R_MIPS16_26 || r_type == R_MIPS16_CALL16;
 }
 
+static inline bfd_boolean
+jal_reloc_p (int r_type)
+{
+  return r_type == R_MIPS_26 || r_type == R_MIPS16_26;
+}
+
 void
 _bfd_mips16_elf_reloc_unshuffle (bfd *abfd, int r_type,
 				 bfd_boolean jal_shuffle, bfd_byte *data)
@@ -4771,8 +4782,8 @@ mips_elf_relocation_needs_la25_stub (bfd
    RELOCATION; RELOCATION->R_ADDEND is ignored.
 
    The result of the relocation calculation is stored in VALUEP.
-   REQUIRE_JALXP indicates whether or not the opcode used with this
-   relocation must be JALX.
+   On exit, set *CROSS_MODE_JUMP_P to true if the relocation field
+   is a MIPS16 jump to non-MIPS16 code, or vice versa.
 
    This function returns bfd_reloc_continue if the caller need take no
    further action regarding this relocation, bfd_reloc_notsupported if
@@ -4787,7 +4798,8 @@ mips_elf_calculate_relocation (bfd *abfd
 			       bfd_vma addend, reloc_howto_type *howto,
 			       Elf_Internal_Sym *local_syms,
 			       asection **local_sections, bfd_vma *valuep,
-			       const char **namep, bfd_boolean *require_jalxp,
+			       const char **namep,
+			       bfd_boolean *cross_mode_jump_p,
 			       bfd_boolean save_addend)
 {
   /* The eventual value we will return.  */
@@ -5076,10 +5088,11 @@ mips_elf_calculate_relocation (bfd *abfd
 	      + h->la25_stub->offset);
 
   /* Calls from 16-bit code to 32-bit code and vice versa require the
-     special jalx instruction.  */
-  *require_jalxp = (!info->relocatable
-                    && (((r_type == R_MIPS16_26) && !target_is_16_bit_code_p)
-                        || ((r_type == R_MIPS_26) && target_is_16_bit_code_p)));
+     mode change.  */
+  *cross_mode_jump_p = !info->relocatable
+		       && ((r_type == R_MIPS16_26 && !target_is_16_bit_code_p)
+			   || ((r_type == R_MIPS_26 || r_type == R_MIPS_JALR)
+			       && target_is_16_bit_code_p));
 
   local_p = mips_elf_local_relocation_p (input_bfd, relocation,
 					 local_sections, TRUE);
@@ -5533,9 +5546,9 @@ mips_elf_obtain_contents (reloc_howto_ty
 /* It has been determined that the result of the RELOCATION is the
    VALUE.  Use HOWTO to place VALUE into the output file at the
    appropriate position.  The SECTION is the section to which the
-   relocation applies.  If REQUIRE_JALX is TRUE, then the opcode used
-   for the relocation must be either JAL or JALX, and it is
-   unconditionally converted to JALX.
+   relocation applies.  
+   CROSS_MODE_JUMP_P is true if the relocation field
+   is a MIPS16 jump to non-MIPS16 code, or vice versa.
 
    Returns FALSE if anything goes wrong.  */
 
@@ -5545,7 +5558,7 @@ mips_elf_perform_relocation (struct bfd_
 			     const Elf_Internal_Rela *relocation,
 			     bfd_vma value, bfd *input_bfd,
 			     asection *input_section, bfd_byte *contents,
-			     bfd_boolean require_jalx)
+			     bfd_boolean cross_mode_jump_p)
 {
   bfd_vma x;
   bfd_byte *location;
@@ -5566,7 +5579,7 @@ mips_elf_perform_relocation (struct bfd_
   x |= (value & howto->dst_mask);
 
   /* If required, turn JAL into JALX.  */
-  if (require_jalx)
+  if (cross_mode_jump_p && jal_reloc_p (r_type))
     {
       bfd_boolean ok;
       bfd_vma opcode = x >> 26;
@@ -5600,15 +5613,19 @@ mips_elf_perform_relocation (struct bfd_
       x = (x & ~(0x3f << 26)) | (jalx_opcode << 26);
     }
 
-  /* Try converting JAL and JALR to BAL, if the target is in range.  */
+  /* Try converting JAL to BAL and J(AL)R to B(AL), if the target is in
+     range.  */
   if (!info->relocatable
-      && !require_jalx
+      && !cross_mode_jump_p
       && ((JAL_TO_BAL_P (input_bfd)
 	   && r_type == R_MIPS_26
 	   && (x >> 26) == 0x3)		/* jal addr */
 	  || (JALR_TO_BAL_P (input_bfd)
 	      && r_type == R_MIPS_JALR
-	      && x == 0x0320f809)))	/* jalr t9 */
+	      && x == 0x0320f809)	/* jalr t9 */
+	  || (JR_TO_B_P (input_bfd)
+	      && r_type == R_MIPS_JALR
+	      && x == 0x03200008)))	/* jr t9 */
     {
       bfd_vma addr;
       bfd_vma dest;
@@ -5624,7 +5641,12 @@ mips_elf_perform_relocation (struct bfd_
 	dest = value;
       off = dest - addr;
       if (off <= 0x1ffff && off >= -0x20000)
-	x = 0x04110000 | (((bfd_vma) off >> 2) & 0xffff);   /* bal addr */
+	{
+	  if (x == 0x03200008)	/* jr t9 */
+	    x = 0x10000000 | (((bfd_vma) off >> 2) & 0xffff);   /* b addr */
+	  else
+	    x = 0x04110000 | (((bfd_vma) off >> 2) & 0xffff);   /* bal addr */
+	}
     }
 
   /* Put the value into the output.  */
@@ -8886,7 +8908,7 @@ _bfd_mips_elf_relocate_section (bfd *out
       const char *name;
       bfd_vma value = 0;
       reloc_howto_type *howto;
-      bfd_boolean require_jalx;
+      bfd_boolean cross_mode_jump_p;
       /* TRUE if the relocation is a RELA relocation, rather than a
          REL relocation.  */
       bfd_boolean rela_relocation_p = TRUE;
@@ -9086,7 +9108,7 @@ _bfd_mips_elf_relocate_section (bfd *out
 					     input_section, info, rel,
 					     addend, howto, local_syms,
 					     local_sections, &value,
-					     &name, &require_jalx,
+					     &name, &cross_mode_jump_p,
 					     use_saved_addend_p))
 	{
 	case bfd_reloc_continue:
@@ -9198,7 +9220,7 @@ _bfd_mips_elf_relocate_section (bfd *out
       /* Actually perform the relocation.  */
       if (! mips_elf_perform_relocation (info, howto, rel, value,
 					 input_bfd, input_section,
-					 contents, require_jalx))
+					 contents, cross_mode_jump_p))
 	return FALSE;
     }