Re: [PATCH] Support VU0 on MIPS R5900

[JÃrgen Urban <JuergenUrban at gmx dot de>]

Am 31.07.2013 um 10:13 schrieb Richard Sandiford <rdsandiford@googlemail.com>:

> "JÃrgen Urban" <JuergenUrban@gmx.de> writes:
>> ragnarok2040 is busy and wasn't able to finish the work. So I took over
>> the work. The binutils changed in the meantime. So the old patch doesn't
>> apply and your questions are no longer applicable (the patch is
>> completely changed). I couldn't find a way to work the old stuff in,
>> because the new binutils are very different. So I decided to add it
>> without special support for suffixes. All suffixes are listed instead in
>> the mips opcode table, so the suffixes will work without special suffix
>> support. I think this was the intented way that binutils was designed
>> for.
> 
> Well, I'm not sure there's really a precedent either way.  These VU0
> instructions are pretty idiosyncratic.  Things like .ob vs. .qh for MDMX
> and .s/.d/.ps for FP are similar, but there are different requirements
> for when you can use those (no .qh for VR5400, no .ps until MIPS V, etc.)
> In this case the suffix is really just an operand that happens to be part
> of the mnemonic, so I preferred your original approach of dealing with the
> suffixes programmatically.  Certainly....
> 
>> The result is that the patch adds 1527 instructions.
> 
> ...this seems far too many :-)
> 
> The easiest way of dealing with it would be to have a pinfo/pinfo2 bit
> to say that the suffix is required.  Unfortunately there are none left
> that we can use.
> 
> I'm close to finishing a series of patches to further rework the opcode
> table and free up more bits.  Those patches again interfere with yours,
> sorry.  Rather than ask you to make another wholesale change, I've locally
> reworked your patch to apply on top of the other ones and to make it
> use the pinfo2 approach.

I would appreciate it. I am hoping that it gets finally in.

>> I did the same for register suffixes, so all combinations are part of
>> the internal symbol table for registers, i.e. 1043 registers.
> 
> Here too I think your original approach was better, sorry.  Although the
> syntax glues the suffix directly to the register name, I think it's
> logically a separate token.  The fact that the designers chose the syntax
> "$vf0xyz" rather than "$vf0.xyz" (or "$vf0[xyz]", or whatever) isn't
> particularly important.  In the new parsing scheme that means the suffix
> should be a separate OT_*.
> 
> The same principle applies to the operands.  The fields specified by the
> suffixes are not contiguous with the register fields, so I think things
> like "$vf0xyz" should be two separate mips_operands, one normal register
> operand and one new type for just the suffix.

You need to know that there are special cases for instructions with brackets:

vlqd.xyz $vf0xyz, (--$vi3)xyz
vlqi.xyz $vf1xyz, ($vi2++)xyz
vsqd.x $vf3x, (--$vi4)x
vsqi.y $vf7y, ($vi6++)y
vilwr.w $vi7, ($vi8)w
viswr.z $vi1, ($vi15)z

The suffix is behind the closing bracket. My tests didn't test for the last suffix, because I wasn't able to handle this with my approach. I want that the disassembler prints the last suffix, but this means that the test fails and need to be updated.

> I've done this locally and it seems to be working well.
> 
> BTW, out of interest, I notice the syntax allows "$0" instead of "$vf0"
> or "$vi0", but not "$0xyz" instead of "$vf0xyz" or "$vi0xyz".  Is that
> historical?  I've no problem with keeping it that way, I was just curious.

I think "$0xyz" is confusing and it seems to be never used, so I decided to forbid it. Some source code uses "$0" because there are some old versions of binutils which don't have support for the real register name. I want to have support for "$0" because it makes it easier to port existing code to the new GCC and binutils.

>> I added all tests which are needed to test it and to show that is
>> reliable and everything is correct. This increased the patch to 1
>> MByte. I know this is big, but I think this is needed for testing it
>> correctly.
> 
> Yeah, these tests look great, thanks, and were a big help while making
> the changes above.
> 
>> The VU0 supports vcallmsr only with register $vi27. This is currently
>> not checked by the assembler, so other registers can be used. A fix for
>> this will be later submitted when I found out how to fix this.
> 
> Hmm, probably the best place would be check_completed_insn.  Thanks for
> making it a separate patch though.
> 
> I'm hoping to commit the patches over the weekend.  I just wanted to reply
> earlier than that rather than leave the thread hanging.

Thanks
JÃrgen