public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
@ 2010-05-20 18:00 Catherine Moore
  2010-05-23 21:47 ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Catherine Moore @ 2010-05-20 18:00 UTC (permalink / raw)
  To: binutils; +Cc: Catherine Moore

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

This patch changes the opcode membership for the ssnop instruction to 
I1 from I32 | N55.  It also changes the opcode membership of the ehb 
instruction to I32 from I33.  For those ASE's which don't support ehb 
and/or ssnop, the instruction executes as a nop.  This change make the 
disassembly read more cleanly.  Does this look okay to install?

Thanks,
Catherine

2010-05-20  Catherine Moore  <clm@codesourcery.com>

	opcodes/
	* mips-opc.c (mips_builtin_op):  Change membership for
	ehb and ssnop instructions.

	gas/testsuite
	* gas/mips/set-arch.d: Update expected output.


[-- Attachment #2: ehb-ssnop-patch --]
[-- Type: text/plain, Size: 2740 bytes --]

Index: gas/testsuite/gas/mips/set-arch.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/set-arch.d,v
retrieving revision 1.5
diff -p -r1.5 set-arch.d
*** gas/testsuite/gas/mips/set-arch.d	4 May 2006 10:47:05 -0000	1.5
--- gas/testsuite/gas/mips/set-arch.d	20 May 2010 17:51:37 -0000
*************** Disassembly of section \.text:
*** 165,171 ****
  00000274 <[^>]*> 7000003f 	sdbbp
  00000278 <[^>]*> 7000003f 	sdbbp
  0000027c <[^>]*> 7159e27f 	sdbbp	0x56789
! 00000280 <[^>]*> 000000c0 	sll	zero,zero,0x3
  00000284 <[^>]*> 7ca43980 	0x7ca43980
  00000288 <[^>]*> 7ca46984 	0x7ca46984
  0000028c <[^>]*> 0100fc09 	jalr.hb	t0
--- 165,171 ----
  00000274 <[^>]*> 7000003f 	sdbbp
  00000278 <[^>]*> 7000003f 	sdbbp
  0000027c <[^>]*> 7159e27f 	sdbbp	0x56789
! 00000280 <[^>]*> 000000c0 	ehb
  00000284 <[^>]*> 7ca43980 	0x7ca43980
  00000288 <[^>]*> 7ca46984 	0x7ca46984
  0000028c <[^>]*> 0100fc09 	jalr.hb	t0
Index: opcodes/mips-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-opc.c,v
retrieving revision 1.75
diff -p -r1.75 mips-opc.c
*** opcodes/mips-opc.c	2 Sep 2009 07:20:30 -0000	1.75
--- opcodes/mips-opc.c	20 May 2010 17:51:37 -0000
*************** const struct mips_opcode mips_builtin_op
*** 188,195 ****
  {"pref",    "k,o(b)",   0xcc000000, 0xfc000000, RD_b,           	0,		I4_32|G3	},
  {"prefx",   "h,t(b)",	0x4c00000f, 0xfc0007ff, RD_b|RD_t|FP_S,		0,		I4_33	},
  {"nop",     "",         0x00000000, 0xffffffff, 0,              	INSN2_ALIAS,	I1      }, /* sll */
! {"ssnop",   "",         0x00000040, 0xffffffff, 0,              	INSN2_ALIAS,	I32|N55	}, /* sll */
! {"ehb",     "",         0x000000c0, 0xffffffff, 0,              	INSN2_ALIAS,	I33	}, /* sll */
  {"li",      "t,j",      0x24000000, 0xffe00000, WR_t,			INSN2_ALIAS,	I1	}, /* addiu */
  {"li",	    "t,i",	0x34000000, 0xffe00000, WR_t,			INSN2_ALIAS,	I1	}, /* ori */
  {"li",      "t,I",	0,    (int) M_LI,	INSN_MACRO,		0,		I1	},
--- 188,195 ----
  {"pref",    "k,o(b)",   0xcc000000, 0xfc000000, RD_b,           	0,		I4_32|G3	},
  {"prefx",   "h,t(b)",	0x4c00000f, 0xfc0007ff, RD_b|RD_t|FP_S,		0,		I4_33	},
  {"nop",     "",         0x00000000, 0xffffffff, 0,              	INSN2_ALIAS,	I1      }, /* sll */
! {"ssnop",   "",         0x00000040, 0xffffffff, 0,              	INSN2_ALIAS,	I1	}, /* sll */
! {"ehb",     "",         0x000000c0, 0xffffffff, 0,              	INSN2_ALIAS,	I32	}, /* sll */
  {"li",      "t,j",      0x24000000, 0xffe00000, WR_t,			INSN2_ALIAS,	I1	}, /* addiu */
  {"li",	    "t,i",	0x34000000, 0xffe00000, WR_t,			INSN2_ALIAS,	I1	}, /* ori */
  {"li",      "t,I",	0,    (int) M_LI,	INSN_MACRO,		0,		I1	},

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
  2010-05-20 18:00 [Patch] [MIPS] Change opcode membership for the ssnop and ehb instructions Catherine Moore
@ 2010-05-23 21:47 ` Richard Sandiford
  2010-05-25  2:08   ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2010-05-23 21:47 UTC (permalink / raw)
  To: Catherine Moore; +Cc: binutils

Catherine Moore <clm@codesourcery.com> writes:
> This patch changes the opcode membership for the ssnop instruction to 
> I1 from I32 | N55.  It also changes the opcode membership of the ehb 
> instruction to I32 from I33.  For those ASE's which don't support ehb 
> and/or ssnop, the instruction executes as a nop.  This change make the 
> disassembly read more cleanly.  Does this look okay to install?

I'm not convinced this is a good idea.  Do we really want:

	.set mips32
        ehb

to be acceptable?

Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
  2010-05-23 21:47 ` Richard Sandiford
@ 2010-05-25  2:08   ` Maciej W. Rozycki
  2010-05-26 19:59     ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2010-05-25  2:08 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Catherine Moore, binutils

On Sun, 23 May 2010, Richard Sandiford wrote:

> > This patch changes the opcode membership for the ssnop instruction to 
> > I1 from I32 | N55.  It also changes the opcode membership of the ehb 
> > instruction to I32 from I33.  For those ASE's which don't support ehb 
> > and/or ssnop, the instruction executes as a nop.  This change make the 
> > disassembly read more cleanly.  Does this look okay to install?
> 
> I'm not convinced this is a good idea.  Do we really want:
> 
> 	.set mips32
>         ehb
> 
> to be acceptable?

 Well, EHB is an assembly idiom for "SLL $0, $0, 3" and likewise SSNOP for 
"SLL $0, $0, 1".  Both instructions were chosen deliberately to execute 
correctly back to the MIPS I ISA.  I see no harm with assembling them and, 
more importantly, they are much more readable in `objdump' or GDB where 
you don't have the luxury of a per-instruction ISA annotation.

 With an example hazard-clearing sequence like this:

 ssnop; ssnop; ehb

(EHB executes as SSNOP on pre-MIPS-r2 processors) I see no gain with 
interspersing this code with .set directives (or even wrapping it all).

 What's the cause of your concern?

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
  2010-05-25  2:08   ` Maciej W. Rozycki
@ 2010-05-26 19:59     ` Richard Sandiford
  2010-05-26 20:51       ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2010-05-26 19:59 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Catherine Moore, binutils

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Sun, 23 May 2010, Richard Sandiford wrote:
>> > This patch changes the opcode membership for the ssnop instruction to 
>> > I1 from I32 | N55.  It also changes the opcode membership of the ehb 
>> > instruction to I32 from I33.  For those ASE's which don't support ehb 
>> > and/or ssnop, the instruction executes as a nop.  This change make the 
>> > disassembly read more cleanly.  Does this look okay to install?
>> 
>> I'm not convinced this is a good idea.  Do we really want:
>> 
>> 	.set mips32
>>         ehb
>> 
>> to be acceptable?
>
>  Well, EHB is an assembly idiom for "SLL $0, $0, 3" and likewise SSNOP for 
> "SLL $0, $0, 1".  Both instructions were chosen deliberately to execute 
> correctly back to the MIPS I ISA.

Right, I get that, but...

> I see no harm with assembling them and, 
> more importantly, they are much more readable in `objdump' or GDB where 
> you don't have the luxury of a per-instruction ISA annotation.
>
>  With an example hazard-clearing sequence like this:
>
>  ssnop; ssnop; ehb
>
> (EHB executes as SSNOP on pre-MIPS-r2 processors) I see no gain with 
> interspersing this code with .set directives (or even wrapping it all).
>
>  What's the cause of your concern?

...it was simply that these directives currently carry certain architectural
guarantees in cases where they're accepted.  After the patch, we'd silently
allow SSNOP for some processors that are used in SMP environments without
the SSNOP acting any differently from a normal nop.  I thought it was at
least worth asking the question whether this was a good idea.

If it is, then why only bump EHB down to MIPS32?  Why not bump it down
all the way to MIPS I?

Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
  2010-05-26 19:59     ` Richard Sandiford
@ 2010-05-26 20:51       ` Maciej W. Rozycki
  2010-05-26 21:24         ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2010-05-26 20:51 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Catherine Moore, binutils

On Wed, 26 May 2010, Richard Sandiford wrote:

> >  What's the cause of your concern?
> 
> ...it was simply that these directives currently carry certain architectural
> guarantees in cases where they're accepted.  After the patch, we'd silently

 Correct.  What value does it have in this case?  Can you think of a 
single case where a user would get a build error because of one of these 
mnemonics and then realised they need to rewrite the piece of code 
differently because their CPU predates architectural definitions of these 
aliases?  I find it highly unlikely these days.  And Linux kernel hackers 
that fiddle with old SGI or DEC gear tend to be smart enough not to need 
such aids. ;)

> allow SSNOP for some processors that are used in SMP environments without
> the SSNOP acting any differently from a normal nop.  I thought it was at
> least worth asking the question whether this was a good idea.

 SSNOP is for superscalar processors rather than SMP (you may have had 
SYNC in mind).  Introduced with the R8000 BTW (which we don't support 
anyway) for things like the usual CP0 hazard avoidance.  I'd expect any 
pre-MIPS32/MIPS64 ISA superscalar hardware either to implement it as 
expected (like the MIPS R8000 or the NEC VR5500) or not to need it at all 
(like the MIPS R10000 and friends that interlock instead).

> If it is, then why only bump EHB down to MIPS32?  Why not bump it down
> all the way to MIPS I?

 That would be my preference as well.  I missed this somehow from the 
original patch, but I don't think it makes any sense to limit EHB like 
this.  The original choice comes from MIPS UK, but unfortunately I can't 
recall why this was done like this, perhaps just because MTI did not 
actively care about legacy MIPS ISAs.  I don't think I was involved with 
the decision.  And the two changes were probably made separately, hence 
the inconsisteny.

 I appreciate your concerns BTW.  Questioning changes makes it easier to 
find any possible flaws in understanding.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] [MIPS]  Change opcode membership for the ssnop and ehb instructions
  2010-05-26 20:51       ` Maciej W. Rozycki
@ 2010-05-26 21:24         ` Richard Sandiford
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Sandiford @ 2010-05-26 21:24 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Catherine Moore, binutils

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Wed, 26 May 2010, Richard Sandiford wrote:
>> >  What's the cause of your concern?
>> 
>> ...it was simply that these directives currently carry certain architectural
>> guarantees in cases where they're accepted.  After the patch, we'd silently
>
>  Correct.  What value does it have in this case?  Can you think of a 
> single case where a user would get a build error because of one of these 
> mnemonics and then realised they need to rewrite the piece of code 
> differently because their CPU predates architectural definitions of these 
> aliases?  I find it highly unlikely these days.  And Linux kernel hackers 
> that fiddle with old SGI or DEC gear tend to be smart enough not to need 
> such aids. ;)
>
>> allow SSNOP for some processors that are used in SMP environments without
>> the SSNOP acting any differently from a normal nop.  I thought it was at
>> least worth asking the question whether this was a good idea.
>
>  SSNOP is for superscalar processors rather than SMP (you may have had 
> SYNC in mind).

No, my bad.  I meant superscalar, but had been thinking about an
SMP problem just before ;-)

> Introduced with the R8000 BTW (which we don't support 
> anyway) for things like the usual CP0 hazard avoidance.  I'd expect any 
> pre-MIPS32/MIPS64 ISA superscalar hardware either to implement it as 
> expected (like the MIPS R8000 or the NEC VR5500) or not to need it at all 
> (like the MIPS R10000 and friends that interlock instead).

I don't recall it being a superscalar nop on the VR4131 (which didn't
have interlocks for all hazards) but perhaps I'm wrong.

>> If it is, then why only bump EHB down to MIPS32?  Why not bump it down
>> all the way to MIPS I?
>
>  That would be my preference as well.  I missed this somehow from the 
> original patch, but I don't think it makes any sense to limit EHB like 
> this.  The original choice comes from MIPS UK, but unfortunately I can't 
> recall why this was done like this, perhaps just because MTI did not 
> actively care about legacy MIPS ISAs.  I don't think I was involved with 
> the decision.  And the two changes were probably made separately, hence 
> the inconsisteny.

Well, OK, I'm not entirely convinced, but the weight of opinion
is obviously the other way.  Patch is OK with EHB changed to I1.

Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-05-26 21:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-20 18:00 [Patch] [MIPS] Change opcode membership for the ssnop and ehb instructions Catherine Moore
2010-05-23 21:47 ` Richard Sandiford
2010-05-25  2:08   ` Maciej W. Rozycki
2010-05-26 19:59     ` Richard Sandiford
2010-05-26 20:51       ` Maciej W. Rozycki
2010-05-26 21:24         ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).