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] Fix Branches with a constant offset


Richard,

[Removed David from the cc list as he's not been with MIPS Technologies 
for many years now and is not available to comment anymore.]

On Wed, 6 Jun 2012, Richard Sandiford wrote:

> >   After:
> > 2011-04-20  Catherine Moore  <clm@codesourcery.com>
> >             David Ung <davidu@mips.com>..
> >         * config/mips.c (mips_cl_insn): Add new field complete_p
> > ...
> >         (append_insn): Move O_constant expression handling.
> >
> > branches with a constant expression are broken by adding a relocation to them.
> > The relocation is totally wrong as it is saying it is an absolute
> > address while what we have really is an offset.
> >
> > This patch fixes the issue with how complete_p is handled for
> > O_constant expression handling just like it was handled before this
> > patch.
> >
> > OK?  Tested on mips64-linux-gnu with no regressions.
> 
> Oops.  It's certainly unfortunate that we've changed the meaning
> of this case.  TBH though, the new version makes more sense to me.
> If you really want a constant offset, ". + X" is (and IMO always was)
> the right way to write it.  The old behaviour gave oddities where:
> 
> 	.equ	x,0x1000
>         beq     $4,$5,x
> 
> would treat "x" as an offset while:
> 
>         beq     $4,$5,x
> 	.equ	x,0x1000
> 
> would treat it as an address.  The new behaviour also seems to be
> consistent with other targets.
> 
> None of which would be a convincing argument to change the behaviour
> for its own sake.  But now that we've changed it anyway -- in at least
> two releases -- I'm not sure we should go back.

 FWIW, I think semantics should be:

1.
	beq	$4, $5, 0x1000

   -- branch to the absolute address 0x1000.  That needs a relocation of 
   course and may overflow.

2.
	
	beq	$4, $5, . + 0x1000

   -- branch to the relative address 0x1000 bytes away up from the address 
   of the branch instruction.  No relocation needed.

That, of course, however the third operand is evaluated to its final 
expression.  Anything else is going to be pure confusion.  My observation 
is this is what GAS currently does.  This is also how J interprets its 
argument (except of course a relocation is also needed in the second case; 
I have no idea why one isn't produced to catch overflows in the first case 
though -- this is probably a bug).

 I could envisage constructs like:

	beq	$4, $5, -0x1000

and:

	beq	$4, $5, +0x1000

where "-" and "+" are not the usual arithmetic unary operators and that 
would express relative offsets just as with #2 above, but that IMHO would 
only make sense where a new assembly syntax was being defined and in our 
case would also cause more confusion than possible gain.

 Andrew, is the current implementation problematic for you for any reason?

  Maciej


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