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 - 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.


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