public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: PATCH: Remove memory check on SVME instructions
@ 2007-09-04 19:27 Jan Beulich
  2007-09-04 22:48 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2007-09-04 19:27 UTC (permalink / raw)
  To: dwarak.rajagopal, hjl; +Cc: christophe.harle, michael.meissner, binutils

>>> "H.J. Lu" <hjl@lucon.org> 09/04/07 4:08 PM >>>
>On Thu, Aug 30, 2007 at 07:19:02PM -0500, rajagopal, dwarak wrote:
>> H.J.,
>> 
>> Yes, SVM instructions have register only operand.  So please fix this.
>> 
>> Thanks,
>> Dwarak
>> 
>
>I am checking this patch to remove memory check on SVME instructions.

Why? Did you check that you get proper warnings/errors for invalid operand
specifications after you removed these checks?

>>>>>>>>>>>>>>>>>>>>>>>>>>>>
H.J.
---
2007-09-04  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-i386.c (process_operands): Remove segment override
	check on SVME instructions.
	(i386_index_check): Remove memory operand check on  SVME
	instructions.

--- gas/config/tc-i386.c.bar	2007-08-28 21:59:04.000000000 -0700
+++ gas/config/tc-i386.c	2007-09-04 07:04:48.000000000 -0700
@@ -3477,9 +3477,9 @@ process_operands (void)
       default_seg = &ds;
     }
 
-  if ((i.tm.base_opcode == 0x8d /* lea */
-       || (i.tm.cpu_flags & CpuSVME))
-      && i.seg[0] && !quiet_warnings)
+  if (i.tm.base_opcode == 0x8d /* lea */
+      && i.seg[0]
+      && !quiet_warnings)
     as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
 
   /* If a segment was explicitly specified, and the specified segment
@@ -4962,30 +4962,7 @@ i386_index_check (const char *operand_st
  tryprefix:
 #endif
   ok = 1;
-  if ((current_templates->start->cpu_flags & CpuSVME)
-      && current_templates->end[-1].operand_types[0] == AnyMem)
-    {
-      /* Memory operands of SVME insns are special in that they only allow
-	 rAX as their memory address and ignore any segment override.  */
-      unsigned RegXX;
-
-      /* SKINIT is even more restrictive: it always requires EAX.  */
-      if (strcmp (current_templates->start->name, "skinit") == 0)
-	RegXX = Reg32;
-      else if (flag_code == CODE_64BIT)
-	RegXX = i.prefix[ADDR_PREFIX] == 0 ? Reg64 : Reg32;
-      else
-	RegXX = ((flag_code == CODE_16BIT) ^ (i.prefix[ADDR_PREFIX] != 0)
-		 ? Reg16
-		 : Reg32);
-      if (!i.base_reg
-	  || !(i.base_reg->reg_type & Acc)
-	  || !(i.base_reg->reg_type & RegXX)
-	  || i.index_reg
-	  || (i.types[0] & Disp))
-	ok = 0;
-    }
-  else if (flag_code == CODE_64BIT)
+  if (flag_code == CODE_64BIT)
     {
       unsigned RegXX = (i.prefix[ADDR_PREFIX] == 0 ? Reg64 : Reg32);
 

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

* Re: PATCH: Remove memory check on SVME instructions
  2007-09-04 19:27 PATCH: Remove memory check on SVME instructions Jan Beulich
@ 2007-09-04 22:48 ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2007-09-04 22:48 UTC (permalink / raw)
  To: Jan Beulich
  Cc: dwarak.rajagopal, christophe.harle, michael.meissner, binutils

On Tue, Sep 04, 2007 at 08:27:09PM +0100, Jan Beulich wrote:
> >>> "H.J. Lu" <hjl@lucon.org> 09/04/07 4:08 PM >>>
> >On Thu, Aug 30, 2007 at 07:19:02PM -0500, rajagopal, dwarak wrote:
> >> H.J.,
> >> 
> >> Yes, SVM instructions have register only operand.  So please fix this.
> >> 
> >> Thanks,
> >> Dwarak
> >> 
> >
> >I am checking this patch to remove memory check on SVME instructions.
> 
> Why? Did you check that you get proper warnings/errors for invalid operand
> specifications after you removed these checks?
> 

SVME instructions don't take any memory operand. You will get an
error if you try to use an memory operand.


H.J.

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

* Re: PATCH: Remove memory check on SVME instructions
@ 2007-09-06 16:37 Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2007-09-06 16:37 UTC (permalink / raw)
  To: hjl; +Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

>>> "H.J. Lu" <hjl@lucon.org> 09/06/07 8:01 AM >>>
>They didn't do it for a good reason. In x86 assembly, memory operand
>is specified as base + index*scale + offset. If an operand can't be

That's not true (and I suppose you're not intending to teach me x86
assembly). Even the base 8086 opcode set already had exceptions to
that - opcodes 0xA0...0xA3. Following your argumentation, these would
have to have an immediate operand, which they don't. If I wanted to be
picky, I could also include all the string instructions here, including xlat.

>expressed that way, it isn't a memory operand.  It is different from
>memory operand in high level language.  SVME instructions do take
>memory address in C and C++.

Jan

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

* Re: PATCH: Remove memory check on SVME instructions
  2007-09-06  5:41 Jan Beulich
@ 2007-09-06  6:02 ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2007-09-06  6:02 UTC (permalink / raw)
  To: Jan Beulich
  Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

On Thu, Sep 06, 2007 at 06:41:44AM +0100, Jan Beulich wrote:
> >>> "H.J. Lu" <hjl@lucon.org> 09/06/07 6:42 AM >>>
> >On Wed, Sep 05, 2007 at 06:07:04PM +0100, Jan Beulich wrote:
> >> >>> "H.J. Lu" <hjl@lucon.org> 09/05/07 3:27 PM >>>
> >> >> >SVME instructions don't take any memory operand. You will get an
> >> >> >error if you try to use an memory operand.
> >> >> 
> >> >> They really do, just the addressing is so that only rAX can be used. If you check
> >> >> the svme test case, you'll see that there are explict uses of memory operands. In
> >> >> the original patch implementation it may not have been that way, but since the
> >> >> meaning of the operands is memory-like, we agreed on a memory-like notation
> >> >> iirc.
> >> >
> >> >It is wrong and has been fixed.
> >> 
> >> On what basis are you concluding this? As I said, we had specifically agreed on a
> >> memory operand like notation, to reflect the actual effect these opcodes have.
> >
> >It doesn't make those instructions take memory operands as specified
> >in IA32 encoding scheme.
> 
> But that is the point I'm trying to make: No matter what the byte level encoding,
> mnemonics and operands should be expressing what an instruction does (e.g. I
> would also say that monitor hasn't been defined properly). And from that
> perspective, invlpga parallels invlpg, and vmload/vmrun/vmsave parallel fsave/
> frstor/etc (and I was actually promised the mnemonics in the manual would be
> changed accordingly, which unfortunately hasn't happened).
> 

They didn't do it for a good reason. In x86 assembly, memory operand
is specified as base + index*scale + offset. If an operand can't be
expressed that way, it isn't a memory operand.  It is different from
memory operand in high level language.  SVME instructions do take
memory address in C and C++.

BTW, I double checked the spec. Some SVME instructions take 32bit
register in 64bit mode. This patch corrects it.


H.J.
----
gas/

2007-09-05  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-i386.c (match_template): Handle invlpga, vmload,
	vmrun and vmsave in SVME.
	(process_suffix): Likewise.

gas/testsuite/

2007-09-05  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/i386/svme.s: Updated to allow eax in 64bit.
	* gas/i386/svme.d: Updated.
	* gas/i386/svme64.d: Likewise.

opcodes/

2007-08-30  H.J. Lu  <hongjiu.lu@intel.com>

	* i386-opc.tbl: Correct SVME instructions to allow 32bit register
	operand in 64bit mode.
	* i386-tbl.h: Regenerated.

--- binutils/gas/config/tc-i386.c.svme	2007-09-05 22:15:04.000000000 -0700
+++ binutils/gas/config/tc-i386.c	2007-09-05 22:59:31.000000000 -0700
@@ -2664,9 +2664,15 @@ match_template (void)
 	      || !MATCH (overlap1, i.types[1], operand_types[1])
 	      /* monitor in SSE3 is a very special case.  The first
 		 register and the second register may have different
-		 sizes.  The same applies to crc32 in SSE4.2.  */
+		 sizes.  The same applies to crc32 in SSE4.2.  It is
+		 also true for invlpga, vmload, vmrun and vmsave in
+		 SVME.  */
 	      || !((t->base_opcode == 0x0f01
-		    && t->extension_opcode == 0xc8)
+		    && (t->extension_opcode == 0xc8
+			|| t->extension_opcode == 0xd8
+			|| t->extension_opcode == 0xda
+			|| t->extension_opcode == 0xdb
+			|| t->extension_opcode == 0xdf))
 		   || t->base_opcode == 0xf20f38f1
 		   || CONSISTENT_REGISTER_MATCH (overlap0, i.types[0],
 						 operand_types[0],
@@ -3000,11 +3006,17 @@ process_suffix (void)
       /* Now select between word & dword operations via the operand
 	 size prefix, except for instructions that will ignore this
 	 prefix anyway.  */
-      if (i.tm.base_opcode == 0x0f01 && i.tm.extension_opcode == 0xc8)
+      if (i.tm.base_opcode == 0x0f01
+	   && (i.tm.extension_opcode == 0xc8
+	       || i.tm.extension_opcode == 0xd8
+	       || i.tm.extension_opcode == 0xda
+	       || i.tm.extension_opcode == 0xdb
+	       || i.tm.extension_opcode == 0xdf))
 	{
 	  /* monitor in SSE3 is a very special case. The default size
 	     of AX is the size of mode. The address size override
-	     prefix will change the size of AX.  */
+	     prefix will change the size of AX.  It is also true for
+	     invlpga, vmload, vmrun and vmsave in SVME.  */
 	  if (i.op->regs[0].reg_type &
 	      (flag_code == CODE_32BIT ? Reg16 : Reg32))
 	    if (!add_prefix (ADDR_PREFIX_OPCODE))
--- binutils/gas/testsuite/gas/i386/svme.d.svme	2007-08-31 16:00:02.000000000 -0700
+++ binutils/gas/testsuite/gas/i386/svme.d	2007-09-05 22:39:41.000000000 -0700
@@ -15,15 +15,15 @@ Disassembly of section .text:
 [	 ]*[0-9a-f]+:[	 ]+0f 01 d8[	 ]+vmrun[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 db[	 ]+vmsave[	 ]*
 [0-9a-f]+ <att32>:
+[	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 df[	 ]+invlpga[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 da[	 ]+vmload[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 d8[	 ]+vmrun[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 db[	 ]+vmsave[	 ]*
-[	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
 [0-9a-f]+ <intel32>:
+[	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 df[	 ]+invlpga[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 da[	 ]+vmload[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 d8[	 ]+vmrun[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 db[	 ]+vmsave[	 ]*
-[	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
 #pass
--- binutils/gas/testsuite/gas/i386/svme.s.svme	2007-08-31 16:00:02.000000000 -0700
+++ binutils/gas/testsuite/gas/i386/svme.s	2007-09-05 22:40:19.000000000 -0700
@@ -19,20 +19,18 @@ common:
 .ifdef __amd64__
 att64:
 	do_args	%rax, %ecx
-.else
-att32:
-	do_args	%eax, %ecx
 .endif
+att32:
 	skinit	%eax
+	do_args	%eax, %ecx
 
 .intel_syntax noprefix
 .ifdef __amd64__
 intel64:
 	do_args	rax, ecx
-.else
-intel32:
-	do_args	eax, ecx
 .endif
+intel32:
 	skinit	eax
+	do_args	eax, ecx
 
 	.p2align 4,0
--- binutils/gas/testsuite/gas/i386/svme64.d.svme	2007-08-31 16:00:02.000000000 -0700
+++ binutils/gas/testsuite/gas/i386/svme64.d	2007-09-05 22:41:31.000000000 -0700
@@ -21,11 +21,21 @@ Disassembly of section .text:
 [	 ]*[0-9a-f]+:[	 ]+0f 01 da[	 ]+vmload[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 d8[	 ]+vmrun[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 db[	 ]+vmsave[	 ]*
+[0-9a-f]+ <att32>:
 [	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 df[	 ]+addr32 invlpga[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 da[	 ]+addr32 vmload[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 d8[	 ]+addr32 vmrun[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 db[	 ]+addr32 vmsave[	 ]
 [0-9a-f]+ <intel64>:
 [	 ]*[0-9a-f]+:[	 ]+0f 01 df[	 ]+invlpga[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 da[	 ]+vmload[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 d8[	 ]+vmrun[	 ]*
 [	 ]*[0-9a-f]+:[	 ]+0f 01 db[	 ]+vmsave[	 ]*
+[0-9a-f]+ <intel32>:
 [	 ]*[0-9a-f]+:[	 ]+0f 01 de[	 ]+skinit[	 ]*
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 df[	 ]+addr32 invlpga[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 da[	 ]+addr32 vmload[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 d8[	 ]+addr32 vmrun[	 ]
+[	 ]*[0-9a-f]+:[	 ]+67 0f 01 db[	 ]+addr32 vmsave[	 ]
 #pass
--- binutils/opcodes/i386-opc.tbl.svme	2007-08-31 16:00:02.000000000 -0700
+++ binutils/opcodes/i386-opc.tbl	2007-09-05 22:33:51.000000000 -0700
@@ -1460,30 +1460,22 @@ rdtscp, 0, 0xf01, 0xf9, CpuSledgehammer,
 // AMD Pacifica additions.
 clgi, 0, 0xf01, 0xdd, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
 invlpga, 0, 0xf01, 0xdf, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
-// FIXME: Need to ensure only "invlpga %eax,%ecx" is accepted.
-invlpga, 2, 0xf01, 0xdf, CpuSVME|CpuNo64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { Reg32, Reg32 }
-// FIXME: Need to ensure only "invlpga %rax,%ecx" is accepted.
-invlpga, 2, 0xf01, 0xdf, CpuSVME|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg64, Reg32 }
+// FIXME: Need to ensure only "invlpga %[re]ax,%ecx" is accepted.
+invlpga, 2, 0xf01, 0xdf, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg32|Reg64, Reg32 }
 skinit, 0, 0xf01, 0xde, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
 // FIXME: Need to ensure only "skinit %eax" is accepted.
 skinit, 1, 0xf01, 0xde, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { Reg32 }
 stgi, 0, 0xf01, 0xdc, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
 vmload, 0, 0xf01, 0xda, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
-// FIXME: Need to ensure only "vmload %eax" is accepted.
-vmload, 1, 0xf01, 0xda, CpuSVME|CpuNo64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { Reg32 }
-// FIXME: Need to ensure only "vmload %rax" is accepted.
-vmload, 1, 0xf01, 0xda, CpuSVME|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg64 }
+// FIXME: Need to ensure only "vmload %[re]ax" is accepted.
+vmload, 1, 0xf01, 0xda, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg32|Reg64 }
 vmmcall, 0, 0xf01, 0xd9, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
 vmrun, 0, 0xf01, 0xd8, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
-// FIXME: Need to ensure only "vmrun %eax" is accepted.
-vmrun, 1, 0xf01, 0xd8, CpuSVME|CpuNo64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { Reg32 }
-// FIXME: Need to ensure only "vmrun %rax" is accepted.
-vmrun, 1, 0xf01, 0xd8, CpuSVME|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg64 }
+// FIXME: Need to ensure only "vmrun %[re]ax" is accepted.
+vmrun, 1, 0xf01, 0xd8, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg32|Reg64 }
 vmsave, 0, 0xf01, 0xdb, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { 0 }
-// FIXME: Need to ensure only "vmsave %eax" is accepted.
-vmsave, 1, 0xf01, 0xdb, CpuSVME|CpuNo64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt, { Reg32 }
-// FIXME: Need to ensure only "vmsave %rax" is accepted.
-vmsave, 1, 0xf01, 0xdb, CpuSVME|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg64 }
+// FIXME: Need to ensure only "vmsave %[re]ax" is accepted.
+vmsave, 1, 0xf01, 0xdb, CpuSVME, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64, { Reg32|Reg64 }
 
 
 // SSE4a instructions
--- binutils/opcodes/i386-tbl.h.svme	2007-08-31 16:00:02.000000000 -0700
+++ binutils/opcodes/i386-tbl.h	2007-09-05 22:34:04.000000000 -0700
@@ -4189,13 +4189,9 @@ const template i386_optab[] =
   { "invlpga", 0, 0xf01, 0xdf, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
     { 0 } },
-  { "invlpga", 2, 0xf01, 0xdf, CpuSVME|CpuNo64,
-    No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
-    { Reg32,
-      Reg32 } },
-  { "invlpga", 2, 0xf01, 0xdf, CpuSVME|Cpu64,
+  { "invlpga", 2, 0xf01, 0xdf, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64,
-    { Reg64,
+    { Reg32|Reg64,
       Reg32 } },
   { "skinit", 0, 0xf01, 0xde, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
@@ -4209,33 +4205,24 @@ const template i386_optab[] =
   { "vmload", 0, 0xf01, 0xda, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
     { 0 } },
-  { "vmload", 1, 0xf01, 0xda, CpuSVME|CpuNo64,
-    No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
-    { Reg32 } },
-  { "vmload", 1, 0xf01, 0xda, CpuSVME|Cpu64,
+  { "vmload", 1, 0xf01, 0xda, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64,
-    { Reg64 } },
+    { Reg32|Reg64 } },
   { "vmmcall", 0, 0xf01, 0xd9, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
     { 0 } },
   { "vmrun", 0, 0xf01, 0xd8, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
     { 0 } },
-  { "vmrun", 1, 0xf01, 0xd8, CpuSVME|CpuNo64,
-    No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
-    { Reg32 } },
-  { "vmrun", 1, 0xf01, 0xd8, CpuSVME|Cpu64,
+  { "vmrun", 1, 0xf01, 0xd8, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64,
-    { Reg64 } },
+    { Reg32|Reg64 } },
   { "vmsave", 0, 0xf01, 0xdb, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
     { 0 } },
-  { "vmsave", 1, 0xf01, 0xdb, CpuSVME|CpuNo64,
-    No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt,
-    { Reg32 } },
-  { "vmsave", 1, 0xf01, 0xdb, CpuSVME|Cpu64,
+  { "vmsave", 1, 0xf01, 0xdb, CpuSVME,
     No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf|ImmExt|NoRex64,
-    { Reg64 } },
+    { Reg32|Reg64 } },
   { "movntsd", 2, 0xf20f2b, None, CpuSSE4a,
     Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_xSuf,
     { RegXMM,

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

* Re: PATCH: Remove memory check on SVME instructions
@ 2007-09-06  5:41 Jan Beulich
  2007-09-06  6:02 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2007-09-06  5:41 UTC (permalink / raw)
  To: hjl; +Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

>>> "H.J. Lu" <hjl@lucon.org> 09/06/07 6:42 AM >>>
>On Wed, Sep 05, 2007 at 06:07:04PM +0100, Jan Beulich wrote:
>> >>> "H.J. Lu" <hjl@lucon.org> 09/05/07 3:27 PM >>>
>> >> >SVME instructions don't take any memory operand. You will get an
>> >> >error if you try to use an memory operand.
>> >> 
>> >> They really do, just the addressing is so that only rAX can be used. If you check
>> >> the svme test case, you'll see that there are explict uses of memory operands. In
>> >> the original patch implementation it may not have been that way, but since the
>> >> meaning of the operands is memory-like, we agreed on a memory-like notation
>> >> iirc.
>> >
>> >It is wrong and has been fixed.
>> 
>> On what basis are you concluding this? As I said, we had specifically agreed on a
>> memory operand like notation, to reflect the actual effect these opcodes have.
>
>It doesn't make those instructions take memory operands as specified
>in IA32 encoding scheme.

But that is the point I'm trying to make: No matter what the byte level encoding,
mnemonics and operands should be expressing what an instruction does (e.g. I
would also say that monitor hasn't been defined properly). And from that
perspective, invlpga parallels invlpg, and vmload/vmrun/vmsave parallel fsave/
frstor/etc (and I was actually promised the mnemonics in the manual would be
changed accordingly, which unfortunately hasn't happened).

But anyway, I'm giving up.

Jan

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

* Re: PATCH: Remove memory check on SVME instructions
  2007-09-05 17:07 Jan Beulich
@ 2007-09-06  4:43 ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2007-09-06  4:43 UTC (permalink / raw)
  To: Jan Beulich
  Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

On Wed, Sep 05, 2007 at 06:07:04PM +0100, Jan Beulich wrote:
> >>> "H.J. Lu" <hjl@lucon.org> 09/05/07 3:27 PM >>>
> >> >SVME instructions don't take any memory operand. You will get an
> >> >error if you try to use an memory operand.
> >> 
> >> They really do, just the addressing is so that only rAX can be used. If you check
> >> the svme test case, you'll see that there are explict uses of memory operands. In
> >> the original patch implementation it may not have been that way, but since the
> >> meaning of the operands is memory-like, we agreed on a memory-like notation
> >> iirc.
> >
> >It is wrong and has been fixed.
> 
> On what basis are you concluding this? As I said, we had specifically agreed on a
> memory operand like notation, to reflect the actual effect these opcodes have.

It doesn't make those instructions take memory operands as specified
in IA32 encoding scheme.


H.J.

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

* Re: PATCH: Remove memory check on SVME instructions
@ 2007-09-05 17:07 Jan Beulich
  2007-09-06  4:43 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2007-09-05 17:07 UTC (permalink / raw)
  To: hjl; +Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

>>> "H.J. Lu" <hjl@lucon.org> 09/05/07 3:27 PM >>>
>> >SVME instructions don't take any memory operand. You will get an
>> >error if you try to use an memory operand.
>> 
>> They really do, just the addressing is so that only rAX can be used. If you check
>> the svme test case, you'll see that there are explict uses of memory operands. In
>> the original patch implementation it may not have been that way, but since the
>> meaning of the operands is memory-like, we agreed on a memory-like notation
>> iirc.
>
>It is wrong and has been fixed.

On what basis are you concluding this? As I said, we had specifically agreed on a
memory operand like notation, to reflect the actual effect these opcodes have.

Jan

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

* Re: PATCH: Remove memory check on SVME instructions
  2007-09-05  5:33 Jan Beulich
@ 2007-09-05 13:28 ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2007-09-05 13:28 UTC (permalink / raw)
  To: Jan Beulich
  Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

On Wed, Sep 05, 2007 at 06:33:35AM +0100, Jan Beulich wrote:
> >>> "H.J. Lu" <hjl@lucon.org> 09/05/07 12:48 AM >>>
> >On Tue, Sep 04, 2007 at 08:27:09PM +0100, Jan Beulich wrote:
> >> >>> "H.J. Lu" <hjl@lucon.org> 09/04/07 4:08 PM >>>
> >> >On Thu, Aug 30, 2007 at 07:19:02PM -0500, rajagopal, dwarak wrote:
> >> >> H.J.,
> >> >> 
> >> >> Yes, SVM instructions have register only operand.  So please fix this.
> >> >> 
> >> >> Thanks,
> >> >> Dwarak
> >> >> 
> >> >
> >> >I am checking this patch to remove memory check on SVME instructions.
> >> 
> >> Why? Did you check that you get proper warnings/errors for invalid operand
> >> specifications after you removed these checks?
> >> 
> >
> >SVME instructions don't take any memory operand. You will get an
> >error if you try to use an memory operand.
> 
> They really do, just the addressing is so that only rAX can be used. If you check
> the svme test case, you'll see that there are explict uses of memory operands. In
> the original patch implementation it may not have been that way, but since the
> meaning of the operands is memory-like, we agreed on a memory-like notation
> iirc.

It is wrong and has been fixed.


H.J.

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

* Re: PATCH: Remove memory check on SVME instructions
@ 2007-09-05  5:33 Jan Beulich
  2007-09-05 13:28 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2007-09-05  5:33 UTC (permalink / raw)
  To: hjl; +Cc: christophe.harle, dwarak.rajagopal, michael.meissner, binutils

>>> "H.J. Lu" <hjl@lucon.org> 09/05/07 12:48 AM >>>
>On Tue, Sep 04, 2007 at 08:27:09PM +0100, Jan Beulich wrote:
>> >>> "H.J. Lu" <hjl@lucon.org> 09/04/07 4:08 PM >>>
>> >On Thu, Aug 30, 2007 at 07:19:02PM -0500, rajagopal, dwarak wrote:
>> >> H.J.,
>> >> 
>> >> Yes, SVM instructions have register only operand.  So please fix this.
>> >> 
>> >> Thanks,
>> >> Dwarak
>> >> 
>> >
>> >I am checking this patch to remove memory check on SVME instructions.
>> 
>> Why? Did you check that you get proper warnings/errors for invalid operand
>> specifications after you removed these checks?
>> 
>
>SVME instructions don't take any memory operand. You will get an
>error if you try to use an memory operand.

They really do, just the addressing is so that only rAX can be used. If you check
the svme test case, you'll see that there are explict uses of memory operands. In
the original patch implementation it may not have been that way, but since the
meaning of the operands is memory-like, we agreed on a memory-like notation
iirc.

Jan

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

* PATCH: Remove memory check on SVME instructions
       [not found]   ` <9E1304B144EBEB4C97F4162BFAC478862124F9@SAUSEXMB2.amd.com>
@ 2007-09-04 14:09     ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2007-09-04 14:09 UTC (permalink / raw)
  To: rajagopal, dwarak; +Cc: Meissner, Michael, Harle, Christophe, binutils

On Thu, Aug 30, 2007 at 07:19:02PM -0500, rajagopal, dwarak wrote:
> H.J.,
> 
> Yes, SVM instructions have register only operand.  So please fix this.
> 
> Thanks,
> Dwarak
> 

I am checking this patch to remove memory check on SVME instructions.


H.J.
---
2007-09-04  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-i386.c (process_operands): Remove segment override
	check on SVME instructions.
	(i386_index_check): Remove memory operand check on  SVME
	instructions.

--- gas/config/tc-i386.c.bar	2007-08-28 21:59:04.000000000 -0700
+++ gas/config/tc-i386.c	2007-09-04 07:04:48.000000000 -0700
@@ -3477,9 +3477,9 @@ process_operands (void)
       default_seg = &ds;
     }
 
-  if ((i.tm.base_opcode == 0x8d /* lea */
-       || (i.tm.cpu_flags & CpuSVME))
-      && i.seg[0] && !quiet_warnings)
+  if (i.tm.base_opcode == 0x8d /* lea */
+      && i.seg[0]
+      && !quiet_warnings)
     as_warn (_("segment override on `%s' is ineffectual"), i.tm.name);
 
   /* If a segment was explicitly specified, and the specified segment
@@ -4962,30 +4962,7 @@ i386_index_check (const char *operand_st
  tryprefix:
 #endif
   ok = 1;
-  if ((current_templates->start->cpu_flags & CpuSVME)
-      && current_templates->end[-1].operand_types[0] == AnyMem)
-    {
-      /* Memory operands of SVME insns are special in that they only allow
-	 rAX as their memory address and ignore any segment override.  */
-      unsigned RegXX;
-
-      /* SKINIT is even more restrictive: it always requires EAX.  */
-      if (strcmp (current_templates->start->name, "skinit") == 0)
-	RegXX = Reg32;
-      else if (flag_code == CODE_64BIT)
-	RegXX = i.prefix[ADDR_PREFIX] == 0 ? Reg64 : Reg32;
-      else
-	RegXX = ((flag_code == CODE_16BIT) ^ (i.prefix[ADDR_PREFIX] != 0)
-		 ? Reg16
-		 : Reg32);
-      if (!i.base_reg
-	  || !(i.base_reg->reg_type & Acc)
-	  || !(i.base_reg->reg_type & RegXX)
-	  || i.index_reg
-	  || (i.types[0] & Disp))
-	ok = 0;
-    }
-  else if (flag_code == CODE_64BIT)
+  if (flag_code == CODE_64BIT)
     {
       unsigned RegXX = (i.prefix[ADDR_PREFIX] == 0 ? Reg64 : Reg32);
 

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

end of thread, other threads:[~2007-09-06 16:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-04 19:27 PATCH: Remove memory check on SVME instructions Jan Beulich
2007-09-04 22:48 ` H.J. Lu
  -- strict thread matches above, loose matches on Subject: below --
2007-09-06 16:37 Jan Beulich
2007-09-06  5:41 Jan Beulich
2007-09-06  6:02 ` H.J. Lu
2007-09-05 17:07 Jan Beulich
2007-09-06  4:43 ` H.J. Lu
2007-09-05  5:33 Jan Beulich
2007-09-05 13:28 ` H.J. Lu
     [not found] <20070830163046.GA21146@lucon.org>
     [not found] ` <6096959DEF5C9447A6BF80BDC7EB9EDC061560C5@SBOSEXMB1.amd.com>
     [not found]   ` <9E1304B144EBEB4C97F4162BFAC478862124F9@SAUSEXMB2.amd.com>
2007-09-04 14:09     ` H.J. Lu

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).