This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] MIPS/binutils: microMIPS linker relaxation fixes
- From: Tristan Gingold <gingold at adacore dot com>
- To: Maciej W. Rozycki <macro at codesourcery dot com>
- Cc: <binutils at sourceware dot org>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Wed, 2 Nov 2011 12:52:18 +0100
- Subject: Re: [PATCH] MIPS/binutils: microMIPS linker relaxation fixes
- References: <alpine.DEB.email@example.com>
On Oct 27, 2011, at 11:02 PM, Maciej W. Rozycki wrote:
> 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
> 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
Ok too (unless Richard Sandifort objects).
> Tested with mips-sde-elf and mips-linux-gnu, no regressions. OK to
> 2011-10-27 Maciej W. Rozycki <firstname.lastname@example.org>
> * 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.
> * 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/mips/l.d: Update for microMIPS HI16/LO16 relocation
> * 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.
> * email@example.com: Likewise.
> * firstname.lastname@example.org: Likewise.
> * email@example.com: Likewise.
> * firstname.lastname@example.org: Likewise.
> * gas/mips/elf-rel29.s: Source for the new tests.
> * gas/mips/mips.exp: Run the new tests.
> [Patch attached compressed due to its size.]<binutils-umips-relax-fix.diff.bz2>