This is the mail archive of the
mailing list for the binutils project.
RE: [PATCH][MIPS] Cleanup expected output for several MIPS targets
- From: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: "Maciej W. Rozycki" <macro at codesourcery dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>, "Eric Christopher (echristo at gmail dot com)" <echristo at gmail dot com>, Alan Modra <amodra at gmail dot com>, Takashi Yoshii <yoshii dot takashi at renesas dot com>
- Date: Wed, 3 Sep 2014 21:05:20 +0000
- Subject: RE: [PATCH][MIPS] Cleanup expected output for several MIPS targets
- Authentication-results: sourceware.org; auth=none
- References: <6D39441BF12EF246A7ABCE6654B0235320F0301C at LEMAIL01 dot le dot imgtec dot org> <87r3zstuit dot fsf at googlemail dot com>
Richard Sandiford <firstname.lastname@example.org> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> > Attached is a testsuite-only patch to ensure all the tests added in the
> > MIPS O32 FPXX patch will pass on all MIPS targets.
> > This probably amounts to an obvious fix even though it is quite large
> > but I would like to double check the changes I made to the mips.exp and
> > mips-elf.exp files are OK.
> Not sure about the obviousness :-) There seem to be several different
My reasoning was that you took the tests I added in the FPXX commit mostly
on faith so fixes to them for other configurations seemed obvious. Any which
way the changes are here for review now.
> types of change here. Please could you split the patch up so that there's
> one type of change per patch? E.g. adding extra sections is simple enough,
If it better to split the patch that's fine. I did one big patch to match the
one which introduced the tests. I'll post them tomorrow.
> but it's less obvious that:
> diff --git a/gas/testsuite/gas/mips/attr-gnu-abi-fp-1.d
> index ce5bbc2..17218d3 100644
> --- a/gas/testsuite/gas/mips/attr-gnu-abi-fp-1.d
> +++ b/gas/testsuite/gas/mips/attr-gnu-abi-fp-1.d
> @@ -9,14 +9,14 @@ File Attributes
> MIPS ABI Flags Version: 0
> -ISA: MIPS1
> +ISA: MIPS.*
> GPR size: 32
> CPR1 size: 32
> CPR2 size: 0
> FP ABI: Hard float \(double precision\)
> -ISA Extension: None
> +ISA Extension: .*
> -FLAGS 1: 00000000
> +FLAGS 1: 0000000.
> FLAGS 2: 00000000
> is a good idea, since in some ways it weakens the test. Another option
> would be to force MIPS I on the command line.
The tests are about floating point ABI markers rather than flags1 or ISA. The
FP flags should be entirely independent of ISA, ISA extension and flags1 so
testing that the abis are marked correctly regardless of other info is
actually an improvement here.
> The mips.exp change is OK as a stand-alone patch.
> Why is:
> @@ -60,7 +60,7 @@ set linux_gnu [expr [istarget mips*-*-linux*]]
> set embedded_elf [expr [istarget mips*-*-elf]]
> # Set defaults.
> -set abi_asflags(o32) ""
> +set abi_asflags(o32) "-32"
> set abi_asflags(n32) "-march=from-abi -n32 -EB"
> set abi_asflags(n64) "-march=from-abi -64 -EB"
> set abi_ldflags(o32) ""
> needed now and not before?
The abiflags logic is not defined for 'NO_ABI' and for mips_elf the default
abi is NO_ABI. As such when the linker tests for O32 run with mips-elf (and
a number of other configurations) then the O32 ABI extensions fail as the
assembler is actually not targeting O32 with abi_asflags(o32). Given the
name of the option it seems valid to state -32 instead of leaving it as