Re: [PATCH] Convert MIPS JR t9 to B

[Richard Sandiford <rdsandiford at googlemail dot com>]

Looks good.  One thing though...

"Fu, Chao-Ying" <fu@mips.com> writes:
> @@ -4788,6 +4793,7 @@ mips_elf_calculate_relocation (bfd *abfd
>  			       Elf_Internal_Sym *local_syms,
>  			       asection **local_sections, bfd_vma *valuep,
>  			       const char **namep, bfd_boolean *require_jalxp,
> +			       bfd_boolean *require_jalrxp,
>  			       bfd_boolean save_addend)
>  {
>    /* The eventual value we will return.  */
> @@ -5080,6 +5086,10 @@ mips_elf_calculate_relocation (bfd *abfd
>    *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)));
> +  /* Calls from 32-bit code to 16-bit code and require the jalr instruction
> +     to switch the mode.  */
> +  *require_jalrxp = (!info->relocatable
> +                     && r_type == R_MIPS_JALR && target_is_16_bit_code_p);

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