Re: [PATCH] MIPS/GAS: Fix NewABI reloc handling with the LD/SD macro

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
>  All the infrastructure is in place though and here is my proposal to fix 
> the problem.  A side effect is these macros now handle relocations other 
> than LO16 even on o32.  While easily doable I think it makes no sense to 
> insist on the previous behaviour of these macros on o32 -- I think where 
> the address offset is hardware-expressable (i.e. matches the "o(b)" 
> format) it makes perfect sense to respect percent-ops even in macros.  
> What do you think?

Yeah, the previous o32 behaviour was a silent wrong-code bug.

>  Tested with the usual set of targets: mips-linux, mips64-linux, 
> mipstx39-elf, mipsisa64-elf and mips-ecoff targets and their respective 
> little-endian counterparts.  At least a smoke test for the regression 
> caused here would be a good idea; I'll think of something.

Yeah, I think the patch does need a testcase.  Like you say, it doesn't
have to be anything fancy.

> 2010-10-24  Maciej W. Rozycki  <macro@linux-mips.org>
>
> 	gas/
> 	* config/tc-mips.c (macro)[M_LD_OB, M_SD_OB]: Use any offset
> 	reloc supplied.

Looks good, but I think it would be easier to make the 'o' case in
mips_ip set offset_expr appropriately, so that we can use it
unconditionally.  I.e.:

	    case 'o':		/* 16 bit offset */
	      offset_reloc[0] = BFD_RELOC_LO16;
	      offset_reloc[1] = BFD_RELOC_UNUSED;
	      offset_reloc[2] = BFD_RELOC_UNUSED;

This won't affect non-macro uses, since 'o' only accepts non-reloc
constants that don't overflow a LO16.

(As far as setting [1] and [2] goes: I think it would be better if all
the cases in mips_ip set all three reloc elements, rather than just the
first as some do.  That's a separate change though.)

With the change to 'o', let's leave "r" as a scalar.

Also, while we're here, would you mind fixing the unaligned load/store
macros in the same way?

This may be a known bug, and is certainly nothing to do with your patches,
but I notice:

	ld	$4,0x7ffc($5)

fails to work correctly (or trigger a diagnostic) in o32 mode.
0x8000 works fine of course.

Ah, the fun of macros.

Richard