[PATC] MIPS16/GAS: Permit branch swapping with PC-relative insns

[Richard Sandiford]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Thu, 11 Aug 2011, Richard Sandiford wrote:
>
>> >  Hmm, I have troubles buying that -- that can be true about any address 
>> > calculation that gets swapped into a branch delay slot.  And about the 
>> > only use of such calculation, requiring the resulting address to point to 
>> > the instruction doing it, is to patch it with something else in 
>> > self-modifying code, which sounds useless, but let it be that I may be 
>> > overlooking something here.
>> >
>> >  What I do not overlook, and neither you do, is that such code requires a 
>> > noreorder block, or otherwise GAS is essentially *by definition* free to 
>> > shuffle code around, transform instructions, add NOPs and do all kinds of 
>> > weird stuff, so I fail to see why we should be refraining from making this 
>> > particular change.
>> 
>> So you're agreeing with me that it is very unlikely that such code will
>> ever be written.  And yet you insist on changing the behaviour for it
>> (and for nothing else) anyway.
>
>  I only wrote that such code shouldn't depend on exact instruction 
> scheduling.  It's perfectly OK to write e.g. such a piece:
>
> 	.ent	foo
> foo:
> 0:
> 	[...]
> 	li	$5, 2f - 0b
> 1:
> 	addiu	$4, $pc, 0b - 1b
> 	jal	bar			# do something about foo
> 	[...]
> 2:
> 	.end	foo
>
> That'll work the same if ADDIU and JAL are swapped.

But that ADDIUPC can't be swapped.  The offset is negative, so you need
the extended rather than 2-byte instruction.

My point is that the only case in which you can swap is:

(a) The instruction refers to the start of the ADDIUPC instruction itself,
    perhaps using a constant, or perhaps using symbolic arithmetic.
    In that case, swapping seems counter-intuitive at best.  We'd be
    changing the ADDIUPC so that it refers to a jump, even though the
    source "obviously" refers to the ADDIUPC.

(b) The instruction has a hard-coded, nonzero, 8-bit unsigned offset
    (so that no relaxation is required).  We both agree that such an
    offset shouldn't appear in a reorder block.

The only effect of your patch is to change (a) and (b) in a
backwards-incompatible way.  It brings no benefit, and IMO even
makes things conceptually worse.  I say that because you're proposing
that we remove the traditional treatment of ADDIUPC for MIPS16 just
after we have added code to make microMIPS ADDIUPC follow the traditional
MIPS16 behaviour (rather than, say, allowing the instructions to be
swapped for microMIPS, and adjusting the offset to compensate).

So not only would the patch be gratuitously breaking backwards
compatibility, it would also introduce a new inconsistency between
MIPS16 and microMIPS.

Richard