[RFA] [MIPS] Don't use $gp if abicalls are in effect

Maciej W. Rozycki macro@imgtec.com
Fri Jan 22 19:59:00 GMT 2016


On Tue, 19 Jan 2016, Moore, Catherine wrote:

> > > diff --git a/gas/testsuite/gas/mips/mips.exp
> > > b/gas/testsuite/gas/mips/mips.exp index 6645e83..c0a65f1 100644
> > > --- a/gas/testsuite/gas/mips/mips.exp
> > > +++ b/gas/testsuite/gas/mips/mips.exp
> > > @@ -741,6 +741,7 @@ if { [istarget mips*-*-vxworks*] } {
> > >      run_dump_test_arches "rol64-hw"	[mips_arch_list_matching
> > gpr64 ror]
> > >
> > >      run_dump_test "sb"
> > > +    run_dump_test_arches "sdata-gp"	[mips_arch_list_matching
> > mips3]
> > 
> >  Why is running this test limited to MIPS III+ ISAs only?
> 
> I was avoiding errors from the mips64-linux configuration:
> 
> Error: -march=mips1 is not compatible with the selected ABI
> /scratch/cmoore/9101/patched2/binutils-gdb/gas/testsuite/gas/mips/sdata-gp.s:7: Error: `gp=32' used with a 64-bit ABI
> /scratch/cmoore/9101/patched2/binutils-gdb/gas/testsuite/gas/mips/sdata-gp.s:7: Warning: `fp=32' used with a 64-bit ABI

 Ah indeed, I keep forgetting of this failure mode.

> This version of the patch adds -32 to the assembler command and runs the test for MIPS1 +
> I take it you prefer this?

 This was really a question only, not a statement of preference.  I think 
diversity in testing helps, having coverage for ABIs other than o32 in 
tests that are not ABI-specific has its benefit.  OTOH, I think perhaps 
modifying `run_dump_test_arch' so that it adds -32 automagically for 
32-bit architectures is the way to go.

 That said since you've made the switch to -32 already I'm fine with your 
updated change, on the basis that I'll go and look into the change to 
`run_dump_test_arch' outlined above sometime and then review all the test 
cases and remove -32 where it isn't actually needed for the matter of the 
test case itself.

 So I only have this small nit still remaining:

> diff --git a/gas/testsuite/gas/mips/sdata-gp.s b/gas/testsuite/gas/mips/sdata-gp.s
> new file mode 100644
> index 0000000..daca04c
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/sdata-gp.s
> @@ -0,0 +1,7 @@
> +	.sdata
> +c0101:	.word	0xabcd
> +
> +	.text
> +	.align	4
> +test:
> +	lw      $2, c0101

-- you still have spaces between `lw' and its operands here.  Please 
replace them with a tab.

 Please go ahead and commit your change with this update applied, no need 
to repost.  Thanks for your work!

  Maciej



More information about the Binutils mailing list