Created attachment 6844 [details] patch that fixes encoding and disassembling of absdp, dpint, dpsp, dptrunc, rcpdp and rsqrdp instructions gas emits incorrect opcodes for absdp, dpint, dpsp, dptrunc, rcpdp and rsqrdp instructions as it encodes even register number of the reg pair into src2 field and 0 into src1 field whereas Ti assembler encodes odd register (msb) into src2 and even register (lsb) into src1. The proposed patch enables the generation of the same opcode as the Ti assembler. Also, Ti disassembler doesn't care what the value of the src1 field is when disassembling those instructions. The same patch enables the disassembling of such opcodes the same way Ti disassembler does.
This is tricky. Just crossed check and the opcode that binutils generates is correctly disassembled by Ti disassembler. It means there exist 2 variants for those instructions : variant 1 : the one described in SPRUFE8B.pdf and used by binutils with src2 = dreg lsb and src1 = 0 variant 2 : the one actually generated by Ti SDK with src2 = dreg msb ans src1 = dreg lsb I'll update the patch to keep both variants.
For the record here is an example for absdp instruction absdp.asm : .text nop absdp_variant1: ; dst src2 src1 x .word 0x0c180b20 ; 0000 11000 00110 00000 0 101100100000 absdp_variant2: .word 0x0c1ccb20 ; 0000 11000 00111 00110 0 101100100000 .word 0x0c1eeb20 ; 0000 11000 00111 10111 0 101100100000 .word 0x0c1feb20 ; 0000 11000 00111 11111 0 101100100000 $ cl6x -mv6740 absdp.asm && dis6x absdp.obj 00000000 .text: 00000000 00000000 NOP 00000004 0c180b20 ABSDP.S1 A7:A6,A25:A24 00000008 0c1ccb20 ABSDP.S1 A7:A6,A25:A24 0000000c 0c1eeb20 ABSDP.S1 A7:A6,A25:A24 00000010 0c1feb20 ABSDP.S1 A7:A6,A25:A24 00000014 00000000 NOP 00000018 00000000 NOP 0000001c 00000000 NOP
Created attachment 6853 [details] patch that fixes encoding and disassembling of absdp, dpint, dpsp, dptrunc, rcpdp and rsqrdp instructions
Patch updated against master : - generates the same opcodes as Ti SDK - keep old opcodes in gas testsuite as they seem perfectly legit - simplify the disassembly by only looking at src2 field for decoding the regpair register numbers
Hi Alexis, Thanks for the bug report and patch. The patch itself is OK, and I would be happy to apply it, but there are two problems: 1. You do not have a binutils copyright assignment on file with the FSF. Given that this patch is non-trivial, we do need such an assignment in place. You can find the form the start the assignment process here: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=doc/Copyright/request-assign.changes;hb=HEAD Or you could use the request-assign.future version of the form if you want to cover future changes to other parts of the binutils. 2. It would be helpful to have example changelog entries for the various parts of the patch. I could write them myself, but it is much easier (for me) if you write them yourself. Cheers Nick
I've just applied for the requested assignment. As I understand, you'd like individual changelog entries for the different parts of the patch. Is that right ? What about the following : tic6x - modify opcode generation of instructions using src1 port to load regpair lsb After cross checking, instructions that use src2 port to load msb and src1 to load lsb value of a double precision operand into a regpair are not encoded the same way as Ti assembler does. Corresponding instructions are: absdp, dpint, dpsp, dptrunc, rcpdp, rsqrdp. See SPRUFE8B.pdf p. 105, 258, 260, 262 and 418. Ti disassembler : - only bits 0-4 of src2 field is used to encode the regpair register numbers. - will correctly disassemble either Ti or binutils generated opcodes Ti assembler : - encodes odd register number of regpair in src2 and even register number of regpair in src1 Binutils strictly sticks to SPRUFE8B.pdf opcode definition and expects : - src1 field to be 0 - src2 field to be even With this patch binutils will generate the same opcode as Ti's assembler and all opcode variants will be correctly decoded : * include/opcode/tic6x.h: add tic6x_coding_dreg_(msb|lsb) field coding type in order to encode separately the msb and lsb of a register pair ; this will be needed to encode the opcodes the same way as Ti assembler does. * gas/config/tc-tic6x.c: handle tic6x_coding_dreg_(msb|lsb) field coding types and use it to encode register pair numbers when required. * opcodes/tic6x-dis.c: decodes opcodes that have individual msb and lsb halves in src1 & src2 fields ; discard the src1 (lsb) value and only use src2 (msb), discarding bit 0, to follow what Ti SDK does in that case as any value in the src1 field yields the same output with SDK disassembler. * include/opcode/tic6x-opcode-table.h: modify absdp, dpint, dpsp, dptrunc, rcpdp and rsqrdp opcodes to use the new field coding types. * gas/testsuite/gas/tic6x/insns-c674x.d, gas/testsuite/gas/tic6x/insns-c674x.s : add test case for the newly generated opcode but keep the old ones as they seem legit as per Ti disassembler output. Cheers, Alexis.
Hi Alexis, > I've just applied for the requested assignment. Great. > As I understand, you'd like individual changelog entries for the different > parts of the patch. Is that right ? What about the following : Perfect - thanks. I'll keep this email on file and when the copyright assignment comes through ping me and I will check the patch in. Cheers Nick
I have applied the patch and checked it in to the repository.