This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS: microMIPS compact branch linker relaxation check
- From: Richard Sandiford <richard dot sandiford at linaro dot org>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: binutils at sourceware dot org, Chao-ying Fu <fu at mips dot com>, Rich Fuhler <rich at mips dot com>, David Lau <davidlau at mips dot com>, Kevin Mills <kevinm at mips dot com>, Ilie Garbacea <ilie at mips dot com>, Catherine Moore <clm at codesourcery dot com>, Nathan Sidwell <nathan at codesourcery dot com>, Joseph Myers <joseph at codesourcery dot com>
- Date: Mon, 01 Aug 2011 16:28:40 +0100
- Subject: Re: [PATCH] MIPS: microMIPS compact branch linker relaxation check
- References: <alpine.DEB.1.10.1107292358340.4083@tp.orcam.me.uk> <87bowcnvcr.fsf@firetop.home> <alpine.DEB.1.10.1108011533350.4083@tp.orcam.me.uk>
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> > @@ -12474,18 +12480,18 @@ _bfd_mips_elf_relax_section (bfd *abfd,
>> > && ELF32_R_SYM (irel[2].r_info) == r_symndx)
>> > continue;
>> >
>> > - /* See if the LUI instruction *might* be in a branch delay slot. */
>> > + /* See if the LUI instruction *might* be in a branch delay slot.
>> > + We check whether what looks like a 16-bit branch or jump is
>> > + actually an immediate argument to a compact branch, and let
>> > + it through if so. */
>> > if (irel->r_offset >= 2
>> > && check_br16_dslot (abfd, ptr - 2)
>> > && !(irel->r_offset >= 4
>> > - /* If the instruction is actually a 4-byte branch,
>> > - the value of check_br16_dslot doesn't matter.
>> > - We should use check_br32_dslot to check whether
>> > - the branch has a delay slot. */
>> > - && check_4byte_branch (internal_relocs, irelend,
>> > - irel->r_offset - 4)))
>> > + && (bzc = check_bzc (abfd, ptr - 4, irel->r_offset - 4,
>> > + internal_relocs, irelend))))
>> > continue;
>> > if (irel->r_offset >= 4
>> > + && !bzc
>> > && check_br32_dslot (abfd, ptr - 4))
>> > continue;
>> >
>>
>> I think the flow is more obvious as:
>>
>> /* See if the LUI instruction *might* be in a branch delay slot. */
>> if (rel->r_offset >= 4
>> && check_relocated_bzc (...))
>> /* The previous instruction looks like a compact branch,
>> and the relocations confirm that it is one. We can be
>> confident that there is no delay slot. */
>> ;
>> else if ((rel->r_offset >= 2 && check_br16_slot (abfd, ptr - 2)
>> || (rel->r_offset >= 4 && check_br32_dslot (abfd, ptr - 4)))
>> continue;
>>
>> OK with that change.
>
> I would have done it originally myself, except that I have chosen the
> current flow deliberately. Please note that (unlike check_br16_slot() or
> check_br32_dslot()) check_relocated_bzc() is *expensive* in that it
> requires iterating over the relocation table, making it O(n) (as opposed
> to O(1)). Therefore I've chosen to check for it only if check_br16_slot()
> indicates the preceding instruction would otherwise be a 16-bit ordinary
> branch/jump.
OK, fair enough. Approved with just the other changes then.
Richard