This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch - gas] MIPS: fix problem with mips16 delay branchoptimization.
On Tue, 2005-09-06 at 13:48 +0100, Richard Sandiford wrote:
> David Ung <davidu@mips.com> writes:
> >> But the code quoted above is adding the delay instruction (the one
> >> which originally gets added before the branch). It shouldn't be
> >> used for the branch itself. Note that the condition guarding it is:
> >>
> >> else if (mips_opts.mips16
> >> && ! ip->use_extend
> >> && *reloc_type != BFD_RELOC_MIPS16_JMP)
> >> {
> >>
> >> The point of reserving 6 bytes is that:
> >>
> >> (a) we can only use 2-byte instructions to a fill a delay slot
> >> (b) branches are at most 4 bytes
> >> (c) adding the branch shouldn't grow the frag by more than 4 bytes
> >>
> >> So if we reserve 6 bytes, the branch should get placed in the same
> >> frag as the delay slot. That's the idea anyway.
> >
> > yeah, but the jump instruction "jr" will do this code aswell.
>
> Oh, so it was specifically "jr" that was the problem? That's the
> extra detail I was hoping for, thanks.
yeah. I thought I've mention that "jr" was creating the new fragment,
but obviously it wasn't clear enough.
>
> > Perhaps a better approach would be to extend the above check
> > to handle such cases.
>
> FWIW, I agree that that would be better. The reason I didn't like your
> original patch was that it made the proximity of the insn to the end
> of the frag a factor in deciding whether to fill a branch delay slot.
> That seems like a bad design IMO. It would be very confusing for a
> user who's looking at disassembly code if, for example, inserting a
> nop at some seemingly-unrelated point in the file made the difference
> between gas filling or not filling a slot. There's no way we should
> expect the user to understand stuff like gas frags.
>
> At first glance, it looks like it should just be a case of guarding
> "frag_grow (6);" with "(pinfo & INSN_UNCOND_BRANCH_DELAY) == 0".
yeah, something like that might do it.
else if (mips_opts.mips16
&& ! ip->use_extend
+ && (pinfo & INSN_UNCOND_BRANCH_DELAY) == 0
&& *reloc_type != BFD_RELOC_MIPS16_JMP)
I'll run the regressions and see.
David.