[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