[PATCH/MIPS] Fix Branches with a constant offset

[Andrew Pinski]

On Wed, Jun 6, 2012 at 5:35 PM, Maciej W. Rozycki
<macro@codesourcery.com> wrote:
> 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?

Just the MIPS AVP testsuite for verify the architecture depends on the
older (incorrect also in my mind also) behavior.  I don't know who to
talk to at MIPS have this fixed in that testsuite.

Thanks,
Andrew Pinski