This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Add MIPS32r2/MT ASE pause instruction
"Maciej W. Rozycki" <macro@linux-mips.org> writes:
>> Index: opcodes/mips-opc.c
>> ===================================================================
>> RCS file: /cvs/src/src/opcodes/mips-opc.c,v
>> retrieving revision 1.89
>> diff -u -p -r1.89 mips-opc.c
>> --- opcodes/mips-opc.c 29 Nov 2011 20:28:55 -0000 1.89
>> +++ opcodes/mips-opc.c 7 Dec 2011 22:39:50 -0000
>> @@ -1169,6 +1169,7 @@ const struct mips_opcode mips_builtin_op
>> {"ori", "t,r,i", 0x34000000, 0xfc000000, WR_t|RD_s, 0, I1 },
>> {"pabsdiff.ob", "X,Y,Q",0x78000009, 0xfc20003f, WR_D|RD_S|RD_T|FP_D, 0, SB1 },
>> {"pabsdiffc.ob", "Y,Q", 0x78000035, 0xfc2007ff, RD_S|RD_T|FP_D, WR_MACC, SB1 },
>> +{"pause", "", 0x00000140, 0xffffffff, TRAP, 0, I33|MT32},
>> {"pavg.ob", "X,Y,Q", 0x78000008, 0xfc20003f, WR_D|RD_S|RD_T|FP_D, 0, SB1 },
>> {"pickf.ob", "X,Y,Q", 0x78000002, 0xfc20003f, WR_D|RD_S|RD_T|FP_D, 0, MX|SB1 },
>> {"pickf.ob", "D,S,T", 0x4ac00002, 0xffe0003f, WR_D|RD_S|RD_T, 0, N54 },
>
> I think it's redundant to say |MT32 here as MT32 already implies I33 --
> the MT ASE requires rev. 2 of the architecture.
>
> Also please use NODS rather than TRAP -- although both macros expand to
> the same numerical value, they have been semantically split so that the
> former refers to instructions that must not be scheduled into a delay slot
> for architectural reasons (undefined behaviour) and the latter is for
> instructions that are meant to generate a trap and we chose not to
> schedule these instructions into a delay slot merely to make life of other
> software easier (delay-slot execution is just fine).
>
> Yes, I know most MT instructions use the TRAP flag, but that's because
> there's really no reason I know of they should have this flag at all in
> the first place, so they have been left as classified "for someone's
> convenience" (some can legitimately trap indeed, i.e. FORK when run out of
> thread contexts). PAUSE's delay-slot operation is on the other hand
> undefined (which actually brings a concern about forward compatibility of
> old software, hmm...).
>
> I can't approve the change, but it looks good to me otherwise.
OK with Maciej's changes (i.e. MT32 rather than "I33|MT32" and
NODS rather than TRAP).
Richard