Re: MIPS ELF embedded-pic support.

[Ian Lance Taylor <ian at zembu dot com>]

   Date: Fri, 15 Oct 1999 16:08:18 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   I've tested this pretty extensively, including the gcc testsuite, on
   both mips32 (actually mipstx39), and mips64.  I haven't checked to see
   if the old COFF support is still working, although I did try
   not to break it.

The changes to BFD look fine to me.

   --- /sloth/disk0/co/binutils-mainline/binutils/gas/config/tc-mips.c	Tue Sep 28 14:05:41 1999
   +++ binutils/gas/config/tc-mips.c	Fri Oct 15 15:42:53 1999
   @@ -2062,9 +2062,11 @@ append_insn (place, ip, address_expr, re
		     && insn_uses_reg (&prev_insn, RA, MIPS_GR_REG))
		 /* If we are generating embedded PIC code, the branch
		    might be expanded into a sequence which uses $at, so
   -		 we can't swap with an instruction which reads it.  */
   +		 we can't swap with an instruction which reads it.  
   +		 The ELF linker doesn't do this at present.  */
		 || (mips_pic == EMBEDDED_PIC
   -		  && insn_uses_reg (&prev_insn, AT, MIPS_GR_REG))
   +		  && insn_uses_reg (&prev_insn, AT, MIPS_GR_REG)
   +		  && OUTPUT_FLAVOR != bfd_target_elf_flavour)
		 /* If the previous previous instruction has a load
		    delay, and sets a register that the branch reads, we
		    can not swap.  */

Is this wise?  If the ELF linker will ever perform this optimization,
which as I recall is required to support embedded PIC for large
programs, then old object files may break.  I would recommend against
checking bfd_target_elf_flavour in this context.

   @@ -4725,9 +4725,10 @@ macro (ip)
	  else if (mips_pic == EMBEDDED_PIC)
	   {
	     macro_build ((char *) NULL, &icnt, &offset_expr, "bal", "p");
   -	  /* The linker may expand the call to a longer sequence which
   -	     uses $at, so we must break rather than return.  */
   -	  break;
   +	  if (OUTPUT_FLAVOR != bfd_target_elf_flavour)
   +	    /* The linker may expand the call to a longer sequence which
   +	       uses $at, so we must break rather than return.  */
   +	    break;
	   }
	  else
	   abort ();

The same issue appears here as well.

   @@ -9470,29 +9476,28 @@ md_apply_fix (fixP, valueP)
	 symbol, we need to adjust the value.  */
    #ifdef OBJ_ELF
      if (fixP->fx_addsy != NULL && OUTPUT_FLAVOR == bfd_target_elf_flavour)
   -    if (S_GET_OTHER (fixP->fx_addsy) == STO_MIPS16 
   -        || S_IS_WEAK (fixP->fx_addsy)
   -        || (symbol_used_in_reloc_p (fixP->fx_addsy)
   -            && (((bfd_get_section_flags (stdoutput,
   -					 S_GET_SEGMENT (fixP->fx_addsy))
   -		  & SEC_LINK_ONCE) != 0)
   -		|| !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
   -			     ".gnu.linkonce",
   -			     sizeof (".gnu.linkonce") - 1))))
   +    {
   +      /* `*valuep' may contain the value of the symbol on which the reloc
   +	 will be based; we have to remove it.  */
   +      if (symbol_used_in_reloc_p (fixP->fx_addsy)
   +	  && S_GET_SEGMENT (fixP->fx_addsy) != absolute_section
   +	  && S_GET_SEGMENT (fixP->fx_addsy) != undefined_section
   +	  && ! bfd_is_com_section (S_GET_SEGMENT (fixP->fx_addsy)))
   +	value -= S_GET_VALUE (fixP->fx_addsy);

   -      {
   -        value -= S_GET_VALUE (fixP->fx_addsy);
   -        if (value != 0 && ! fixP->fx_pcrel)
   -          {
   -            /* In this case, the bfd_install_relocation routine will
   -               incorrectly add the symbol value back in.  We just want
   -               the addend to appear in the object file.  */
   -            value -= S_GET_VALUE (fixP->fx_addsy);
   -          }
   -      }
   +      if (fixP->fx_pcrel)
   +	{
   +	  value += fixP->fx_frag->fr_address + fixP->fx_where;
   +	  /* BFD's REL handling, for MIPS, is _very_ weird.
   +	     This gives the right results, but it can't possibly
   +	     be the way things are supposed to work.  */
   +	  if (fixP->fx_r_type != BFD_RELOC_16_PCREL_S2
   +	      || S_GET_SEGMENT (fixP->fx_addsy) != undefined_section)
   +	      value += fixP->fx_frag->fr_address + fixP->fx_where;
   +	}
   +    }
    #endif

I'm not comfortable with these changes.  You appear to be changing the
behaviour for STO_MIPS16, S_IS_WEAK, and linkonce symbols, and perhaps
others.  Is that wise?  Have you tested those cases thoroughly?

You can not apply logic to this code.  The whole gas relocation system
is completely and horribly broken, as discussed in the internal
documentation.  I get scared by any change to this code.

   @@ -10991,6 +11011,8 @@ tc_gen_reloc (section, fixp)
	   as_fatal (_("Double check fx_r_type in tc-mips.c:tc_gen_reloc"));
	  fixp->fx_r_type = BFD_RELOC_GPREL32;
	}
   +  else if (fixp->fx_pcrel == 0 || OUTPUT_FLAVOR == bfd_target_elf_flavour)
   +    reloc->addend = fixp->fx_addnumber;
      else if (fixp->fx_r_type == BFD_RELOC_PCREL_LO16)
	{
	  /* We use a special addend for an internal RELLO reloc.  */

   +++ binutils/gas/testsuite/gas/mips/mips.exp	Fri Oct 15 15:42:53 1999
   @@ -107,5 +107,6 @@ if [istarget mips*-*-*] then {
	   } {
	       run_dump_test "e32-rel2" 
	   } 
   +	run_dump_test "empic"
	}
    }

Can you make a version of this test which works for both ECOFF and
ELF?

   --- /sloth/disk0/co/binutils-mainline/binutils/include/elf/mips.h	Mon Jun 21 16:57:35 1999
   +++ binutils/include/elf/mips.h	Fri Oct 15 15:42:53 1999
   @@ -78,6 +78,12 @@ START_RELOC_NUMBERS (elf_mips_reloc_type
      /* These are GNU extensions to enable C++ vtable garbage collection.  */
      RELOC_NUMBER (R_MIPS_GNU_VTINHERIT, 253)
      RELOC_NUMBER (R_MIPS_GNU_VTENTRY, 254)
   +  /* These are GNU extensions to handle embedded-pic.  */
   +  RELOC_NUMBER (R_MIPS_GNU_REL_HI16, 252)
   +  RELOC_NUMBER (R_MIPS_GNU_REL_LO16, 251)
   +  RELOC_NUMBER (R_MIPS_GNU_REL16_S2, 250)
   +  RELOC_NUMBER (R_MIPS_PC64, 249)
   +  RELOC_NUMBER (R_MIPS_PC32, 248)
    END_RELOC_NUMBERS

I think it's a bit nicer if the relocations are sorted in numeric
order.

Ian