This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS EVA ASE Support
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: "Moore\, Catherine" <Catherine_Moore at mentor dot com>, "binutils\ at sourceware dot org" <binutils at sourceware dot org>
- Date: Mon, 10 Jun 2013 17:08:27 +0100
- Subject: Re: [PATCH] MIPS EVA ASE Support
- References: <FD3DCEAC5B03E9408544A1E416F11242F8FC6EE7 at NA-MBX-01 dot mgc dot mentorg dot com> <87fvwz9hsg dot fsf at talisman dot default> <FD3DCEAC5B03E9408544A1E416F11242F8FC9639 at NA-MBX-01 dot mgc dot mentorg dot com> <87bo7g99yn dot fsf at talisman dot default> <alpine dot DEB dot 1 dot 10 dot 1306091647520 dot 16287 at tp dot orcam dot me dot uk>
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Sat, 8 Jun 2013, Richard Sandiford wrote:
>> Yeah, that's quite the patch. Thanks for doing this. The opcode table
>> changes were pretty much impossible to review manually, so I opted for
>> review by script instead. This threw up the changes below. Some of them
>> are just minor formatting tweaks: the IOCT|IOCTP|IOCT2 lines are removing
>> an excess tab that was carried over from the original, the SMT lines are
>> using a tab rather than a space, and the IL2F|IL3A lines are to avoid
>> whitespace changes on lines that don't need to be touched. I think the
>> rest are real fixes though. Still, if you wrote the changes by hand,
>> the number of differences is impressively small :-)
>
> Great work indeed -- thanks, Catherine!
>
>> I've applied the patch with these changes and with a minor tweak to the
>> mips-dis.c formatting of the mips32r2 and mips64r2 entries.
>
> A small nit below:
>
>> Index: opcodes/micromips-opc.c
>> ===================================================================
>> --- opcodes/micromips-opc.c 2013-06-08 10:18:33.894842596 +0100
>> +++ opcodes/micromips-opc.c 2013-06-08 10:18:44.227957964 +0100
>> @@ -115,7 +115,7 @@ const struct mips_opcode micromips_opcod
>> /* These instructions appear first so that the disassembler will find
>> them first. The assemblers uses a hash table based on the
>> instruction name anyhow. */
>> -/* name, args, match, mask, pinfo, pinfo2, membership, [exclusions] */
>> +/* name, args, match, mask, pinfo, pinfo2, membership, [ase], [exclusions] */
>
> I think this should be:
>
> /* name, args, match, mask, pinfo, pinfo2, membership[[, ase], exclusions] */
>
> or suchlike as the use of "exclusions" requires "ase" to have been set
> too (possibly to 0). Likewise in opcodes/mips-opc.c.
Don't you mean:
[, ase[, exclusions]]
? But the original seems clearer to me.
Richard