This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MIPS/binutils: microMIPS linker relaxation fixes


On Oct 27, 2011, at 11:02 PM, Maciej W. Rozycki wrote:

> Hi,
> 
> Here are the long-promised microMIPS linker relaxation fixes -- I had to 
> go through another round of checking everything is right before submission 
> and I believe this version addresses all the concerns I meant to address 
> and in a way I am satisfied with.
> 
> Here's a summary of what this piece changes:
> 
> * relaxation is made in two phases so that alignment changes coming from
>  32-bit instructions being replaced with their 16-bit counterparts do
>  not make ADDIUPC offsets invalid (they require 4-byte alignment); 
>  further 32-bit->16-bit instruction relaxation is not made even if 
>  enabled by code shrinkage after ADDIUPC relaxation,
> 
> * HI16/LO16 relocation pairs against local symbols are not converted to
>  refer to the corresponding section symbol on REL targets as that breaks 
>  addend recalculation that may overflow if they are replaced with a 
>  PC23_S2 relocation used with ADDIUPC (arguably this is a shortcoming of 
>  BFD that could be fixed by using RELA reloc representation internally 
>  throughout for REL targets too, but there you go -- not to be fixed 
>  with this change); a test case is included to cover it,
> 
> * all addends of relocations against the section symbol of a section being 
>  relaxed are recalculated according to code being moved (a similar 
>  recalculation should be made for other symbols; right now code only
>  recalculates symbol addresses disregarding the addend used -- I have 
>  deliberately not addressed this shortcoming at this stage as text 
>  symbols typically are not offsetted and it only triggers when applying 
>  the addend makes the reference cross the place of code shrinkage),
> 
> * any addend used with instructions being relaxed is taken into account 
>  while determining whether relaxation is possible,
> 
> * any addend used with instructions being relaxed on REL targets is
>  reformatted according to the replacement instructions used and retained,
> 
> * several issues related to ADDIUPC relaxation around branches and jumps 
>  have been fixed per earlier discussion,
> 
> * code references to microMIPS symbols are not considered for ADDIUPC 
>  relaxation -- the ISA bit being set makes that an unlikely corner case 
>  any effort to handle which seems unjustified.
> 
> Code generated may still be broken if microMIPS and standard MIPS code is 
> interlinked in the same section -- in this case 32-bit->16-bit instruction 
> relaxation may cause standard MIPS code misalignment.  I have deliberately 
> not addressed this shortcoming at this stage.  The workaround is to 
> refrain from linker relaxation when mixing microMIPS and standard MIPS 
> code.
> 
> As it fixes a number of cases where bad code is generated, I recommend it 
> for the 2.22 branch too -- without this fix microMIPS relaxation is pretty 
> much unusable for any non-trivial case.  I'll follow up with a relaxation 
> test case, assuming we'll sort out with Alan how to address the .reloc 
> regression.

Ok too (unless Richard Sandifort objects).

Tristan.

> 
> Tested with mips-sde-elf and mips-linux-gnu, no regressions.  OK to 
> apply?
> 
> 2011-10-27  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	bfd/
> 	* elfxx-mips.c (mips_elf_release_contents): New function.
> 	(mips_elf_write_rel_addend): Likewise.
> 	(mips_elf_relax_reloc_adjust_addend): Likewise.
> 	(mips_elf_relax_delete_bytes): Also adjust addends of relocs
> 	against the section being handled.
> 	(relax_phase_1): New macro.
> 	(_bfd_mips_elf_relax_section): Don't relax IRIX objects.  Split
> 	handling into two phases.  Reimplement ADDIUPC relaxation.
> 	Recalculate and retain in-place addends of instructions relaxed.
> 
> 	gas/
> 	* config/tc-mips.c (SY_MIPS16_26, SY_MICROMIPS_HI16): New macros.
> 	(append_insn): Use them for symbol tagging.
> 	(mips_fix_adjustable): On REL targets reject microMIPS HI16
> 	relocations as well as matching LO16 and GOT16 relocations.  
> 	Update to use SY_MIPS16_26.
> 
> 	gas/testsuite/
> 	* gas/mips/l.d: Update for microMIPS HI16/LO16 relocation
> 	changes.
> 	* gas/mips/l_d.d: Likewise.
> 	* gas/mips/sd.d: Likewise.
> 	* gas/mips/s_d.d: Likewise.
> 	* gas/mips/elf-rel29.d: New test.
> 	* gas/mips/elf-rel29-n32.d: Likewise.
> 	* gas/mips/elf-rel29-n64.d: Likewise.
> 	* gas/mips/mips16@elf-rel29.d: Likewise.
> 	* gas/mips/micromips@elf-rel29.d: Likewise.
> 	* gas/mips/micromips@elf-rel29-n32.d: Likewise.
> 	* gas/mips/micromips@elf-rel29-n64.d: Likewise.
> 	* gas/mips/elf-rel29.s: Source for the new tests.
> 	* gas/mips/mips.exp: Run the new tests.
> 
>  Maciej
> 
> binutils-umips-relax-fix.diff
> [Patch attached compressed due to its size.]<binutils-umips-relax-fix.diff.bz2>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]