This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH][MIPS] Cleanup expected output for several MIPS targets


Richard Sandiford <rdsandiford@googlemail.com> 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
> b/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: .*
>  ASEs:
>  	None
> -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
NO_ABI.

Thanks,
Matthew


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]