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

[Richard Sandiford <rdsandiford at googlemail dot com>]

Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> 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.

Thanks.

>> 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.

OK, fair enough.

>> 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.

Yeah, I'd forgotten about NO_ABI.  In that case the mips-elf.exp parts
are also OK as an independent patch.

Thanks,
Richard