This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: PR gas/1874: mwait/monitor don't work in 64bit


On Fri, Nov 18, 2005 at 09:06:17AM +0100, Jan Beulich wrote:
> >I really don't like to use memory operand for monitor. Here is the new
> >patch without memory operand.
> 
> Any particular reason for that? But I don't really mind...
> 
> >+	  if (((flag_code == CODE_16BIT || flag_code == CODE_64BIT)
> >+	       && (i.op->regs[0].reg_type & Reg32))
> >+	      || (flag_code == CODE_32BIT
> >+		  && i.op->regs[0].reg_type & Reg16))
> 
> While I recognize this is partly a stylistic thing, I would see this
> more readable if simplified:
> 
> 	  if (i.op->regs[0].reg_type & (flag_code != CODE_32BIT ? Reg32
> : Reg16))

Ok.

> 
> >+      /* Remove "addr16/addr32" if we aren't in Intel mode.  */
> >+      if (!intel_syntax
> >+	  && (prefixes & PREFIX_ADDR)
> >+	  && olen >= (4 + 7)
> >+	  && *(p - 1) == ' '
> >+	  && strncmp (p - 7, "addr", 4) == 0
> >+	  && (strncmp (p - 3, "16", 2) == 0
> >+	      || strncmp (p - 3, "32", 2) == 0))
> >+	p -= 7;
> >+
> 
> Oh, I see, I asked for a thing that needs general cleanup. I was mixing
> up the behaviors of PREFIX_ADDR and PREFIX_DATA. It really should be
> opcode-independent code that deals with suppressing this. I withdraw my
> request to suppress addrXX, but I leave it up to you if you want to keep
> this piece.
> 

Here is the updated patch. I also changed "objdump -d -Mi8086". I think
there is a bigger issue for -Mi8086:

[hjl@gnu-13 monitor]$ ./objdump -d -Mintel:i8086 32.o

32.o:     file format elf32-i386

Disassembly of section .text:

00000000 <.text>:
   0:   0f 01 c8                monitor
   3:   67 0f 01 c8             addr16 monitor

It should be addr32, not addr16. I think the mode_16bit may help fix
it.

BTW, I will take a vacation until Dec. 3. If I don't check in my patch
today, I will do it after 2 weeks.


H.J.
----
gas/

2005-11-17  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-i386.c (match_template): Handle monitor.
	(process_suffix): Likewise.

gas/testsuite/

2005-11-17  H.J. Lu  <hongjiu.lu@intel.com>

	PR gas/1874
	* gas/i386/i386.exp: Add x86-64-prescott for 64bit.

	* gas/i386/prescott.s: Test address size override for monitor.
	* gas/i386/prescott.d: Updated.

	* gas/i386/x86-64-prescott.d: New file.
	* gas/i386/x86-64-prescott.s: Likewise.

include/opcode/

2005-11-17  H.J. Lu  <hongjiu.lu@intel.com>

	PR gas/1874
	* i386.h (i386_optab): Add 64bit support for monitor and mwait.

opcodes/

2005-11-17  H.J. Lu  <hongjiu.lu@intel.com>

	PR gas/1874
	* i386-dis.c (mode_16bit): New.
	(print_insn): Set mode_16bit to 1 for "i8086".
	(PNI_Fixup): Add 64bit and address size override
	support for monitor and mwait.

--- binutils/gas/config/tc-i386.c.pni	2005-11-11 10:48:52.000000000 -0800
+++ binutils/gas/config/tc-i386.c	2005-11-18 08:51:59.000000000 -0800
@@ -2274,10 +2274,15 @@ match_template ()
 	  overlap1 = i.types[1] & t->operand_types[1];
 	  if (!MATCH (overlap0, i.types[0], t->operand_types[0])
 	      || !MATCH (overlap1, i.types[1], t->operand_types[1])
-	      || !CONSISTENT_REGISTER_MATCH (overlap0, i.types[0],
-					     t->operand_types[0],
-					     overlap1, i.types[1],
-					     t->operand_types[1]))
+	      /* monitor in SSE3 is a very special case.  The first
+		 register and the second register may have differnet
+		 sizes.  */
+	      || !((t->base_opcode == 0x0f01
+		    && t->extension_opcode == 0xc8)
+		   || CONSISTENT_REGISTER_MATCH (overlap0, i.types[0],
+						 t->operand_types[0],
+						 overlap1, i.types[1],
+						 t->operand_types[1])))
 	    {
 	      /* Check if other direction is valid ...  */
 	      if ((t->opcode_modifier & (D | FloatD)) == 0)
@@ -2545,12 +2550,22 @@ 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.suffix != QWORD_MNEM_SUFFIX
-	  && i.suffix != LONG_DOUBLE_MNEM_SUFFIX
-	  && !(i.tm.opcode_modifier & (IgnoreSize | FloatMF))
-	  && ((i.suffix == LONG_MNEM_SUFFIX) == (flag_code == CODE_16BIT)
-	      || (flag_code == CODE_64BIT
-		  && (i.tm.opcode_modifier & JumpByte))))
+      if (i.tm.base_opcode == 0x0f01 && i.tm.extension_opcode == 0xc8)
+	{
+	  /* 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.  */
+	  if (i.op->regs[0].reg_type &
+	      (flag_code == CODE_32BIT ? Reg16 : Reg32))
+	    if (!add_prefix (ADDR_PREFIX_OPCODE))
+	      return 0;
+	}
+      else if (i.suffix != QWORD_MNEM_SUFFIX
+	       && i.suffix != LONG_DOUBLE_MNEM_SUFFIX
+	       && !(i.tm.opcode_modifier & (IgnoreSize | FloatMF))
+	       && ((i.suffix == LONG_MNEM_SUFFIX) == (flag_code == CODE_16BIT)
+		   || (flag_code == CODE_64BIT
+		       && (i.tm.opcode_modifier & JumpByte))))
 	{
 	  unsigned int prefix = DATA_PREFIX_OPCODE;
 
--- binutils/gas/testsuite/gas/i386/i386.exp.pni	2005-11-11 10:48:53.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/i386.exp	2005-11-15 15:37:50.000000000 -0800
@@ -131,6 +131,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     run_dump_test "svme64"
     run_dump_test "x86-64-vmx"
     run_dump_test "immed64"
+    run_dump_test "x86-64-prescott"
 
     # For ELF targets verify that @unwind works.
     if { ([istarget "*-*-elf*"] || [istarget "*-*-linux*"]
--- binutils/gas/testsuite/gas/i386/prescott.d.pni	2004-06-23 08:06:57.000000000 -0700
+++ binutils/gas/testsuite/gas/i386/prescott.d	2005-11-17 15:06:52.000000000 -0800
@@ -32,5 +32,6 @@ Disassembly of section .text:
   62:	f3 0f 12 dc [ 	]*movsldup %xmm4,%xmm3
   66:	0f 01 c9 [ 	]*mwait  %eax,%ecx
   69:	0f 01 c9 [ 	]*mwait  %eax,%ecx
-  6c:	00 00 [ 	]*add    %al,\(%eax\)
+  6c:	67 0f 01 c8 [ 	]*monitor %ax,%ecx,%edx
+  70:	67 0f 01 c8 [ 	]*monitor %ax,%ecx,%edx
 	...
--- binutils/gas/testsuite/gas/i386/prescott.s.pni	2004-06-23 08:06:58.000000000 -0700
+++ binutils/gas/testsuite/gas/i386/prescott.s	2005-11-17 15:02:25.000000000 -0800
@@ -29,4 +29,7 @@ foo:
 	mwait
 	mwait		%eax,%ecx
 
+	monitor		%ax,%ecx,%edx
+	addr16 monitor
+
 	.p2align	4,0
--- binutils/gas/testsuite/gas/i386/x86-64-prescott.d.pni	2005-11-15 15:33:49.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/x86-64-prescott.d	2005-11-17 15:02:46.000000000 -0800
@@ -0,0 +1,37 @@
+#objdump: -dw
+#name: x86-64 prescott
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+000 <foo>:
+   0:	66 0f d0 01 [ 	]*addsubpd \(%rcx\),%xmm0
+   4:	66 0f d0 ca [ 	]*addsubpd %xmm2,%xmm1
+   8:	f2 0f d0 13 [ 	]*addsubps \(%rbx\),%xmm2
+   c:	f2 0f d0 dc [ 	]*addsubps %xmm4,%xmm3
+  10:	df 88 90 90 90 90 [ 	]*fisttp 0xffffffff90909090\(%rax\)
+  16:	db 88 90 90 90 90 [ 	]*fisttpl 0xffffffff90909090\(%rax\)
+  1c:	dd 88 90 90 90 90 [ 	]*fisttpll 0xffffffff90909090\(%rax\)
+  22:	66 0f 7c 65 00 [ 	]*haddpd 0x0\(%rbp\),%xmm4
+  27:	66 0f 7c ee [ 	]*haddpd %xmm6,%xmm5
+  2b:	f2 0f 7c 37 [ 	]*haddps \(%rdi\),%xmm6
+  2f:	f2 0f 7c f8 [ 	]*haddps %xmm0,%xmm7
+  33:	66 0f 7d c1 [ 	]*hsubpd %xmm1,%xmm0
+  37:	66 0f 7d 0a [ 	]*hsubpd \(%rdx\),%xmm1
+  3b:	f2 0f 7d d2 [ 	]*hsubps %xmm2,%xmm2
+  3f:	f2 0f 7d 1c 24 [ 	]*hsubps \(%rsp\),%xmm3
+  44:	f2 0f f0 2e [ 	]*lddqu  \(%rsi\),%xmm5
+  48:	0f 01 c8 [ 	]*monitor %rax,%rcx,%rdx
+  4b:	0f 01 c8 [ 	]*monitor %rax,%rcx,%rdx
+  4e:	f2 0f 12 f7 [ 	]*movddup %xmm7,%xmm6
+  52:	f2 0f 12 38 [ 	]*movddup \(%rax\),%xmm7
+  56:	f3 0f 16 01 [ 	]*movshdup \(%rcx\),%xmm0
+  5a:	f3 0f 16 ca [ 	]*movshdup %xmm2,%xmm1
+  5e:	f3 0f 12 13 [ 	]*movsldup \(%rbx\),%xmm2
+  62:	f3 0f 12 dc [ 	]*movsldup %xmm4,%xmm3
+  66:	0f 01 c9 [ 	]*mwait  %rax,%rcx
+  69:	0f 01 c9 [ 	]*mwait  %rax,%rcx
+  6c:	67 0f 01 c8 [ 	]*monitor %eax,%rcx,%rdx
+  70:	67 0f 01 c8 [ 	]*monitor %eax,%rcx,%rdx
+	...
--- binutils/gas/testsuite/gas/i386/x86-64-prescott.s.pni	2005-11-15 15:33:47.000000000 -0800
+++ binutils/gas/testsuite/gas/i386/x86-64-prescott.s	2005-11-17 15:03:00.000000000 -0800
@@ -0,0 +1,35 @@
+#Prescott New Instructions
+
+	.text
+foo:
+	addsubpd	(%rcx),%xmm0
+	addsubpd	%xmm2,%xmm1
+	addsubps	(%rbx),%xmm2
+	addsubps	%xmm4,%xmm3
+	fisttp		0x90909090(%rax)
+	fisttpl		0x90909090(%rax)
+	fisttpll	0x90909090(%rax)
+	haddpd		0x0(%rbp),%xmm4
+	haddpd		%xmm6,%xmm5
+	haddps		(%rdi),%xmm6
+	haddps		%xmm0,%xmm7
+	hsubpd		%xmm1,%xmm0
+	hsubpd		(%rdx),%xmm1
+	hsubps		%xmm2,%xmm2
+	hsubps		(%rsp,1),%xmm3
+	lddqu		(%rsi),%xmm5
+	monitor
+	monitor		%rax,%rcx,%rdx
+	movddup		%xmm7,%xmm6
+	movddup		(%rax),%xmm7
+	movshdup	(%rcx),%xmm0
+	movshdup	%xmm2,%xmm1
+	movsldup	(%rbx),%xmm2
+	movsldup	%xmm4,%xmm3
+	mwait
+	mwait		%rax,%rcx
+
+	monitor		%eax,%rcx,%rdx
+	addr32 monitor
+
+	.p2align	4,0
--- binutils/include/opcode/i386.h.pni	2005-07-27 08:41:15.000000000 -0700
+++ binutils/include/opcode/i386.h	2005-11-17 15:09:47.000000000 -0800
@@ -1346,14 +1346,24 @@ static const template i386_optab[] =
 {"hsubps",    2, 0xf20f7d,  X, CpuPNI, NoSuf|IgnoreSize|Modrm,	{ RegXMM|LLongMem, RegXMM, 0 } },
 {"lddqu",     2, 0xf20ff0,  X, CpuPNI, NoSuf|IgnoreSize|Modrm,	{ LLongMem, RegXMM, 0 } },
 {"monitor",   0, 0x0f01, 0xc8, CpuPNI, NoSuf|ImmExt,	{ 0, 0, 0} },
-/* Need to ensure only "monitor %eax,%ecx,%edx" is accepted. */
-{"monitor",   3, 0x0f01, 0xc8, CpuPNI, NoSuf|ImmExt,	{ Reg32, Reg32, Reg32} },
+/* monitor is very special. CX and DX are always 64bits with zero upper
+   32bits in 64bit mode, and 32bits in 16bit and 32bit modes. The
+   address size override prefix can be used to overrride the AX size in
+   all modes.  */
+/* Need to ensure only "monitor %eax/%ax,%ecx,%edx" is accepted. */
+{"monitor",   3, 0x0f01, 0xc8, CpuPNI|CpuNo64, NoSuf|ImmExt,	{ Reg16|Reg32, Reg32, Reg32 } },
+/* Need to ensure only "monitor %rax/%eax,%rcx,%rdx" is accepted. */
+{"monitor",   3, 0x0f01, 0xc8, CpuPNI|Cpu64, NoSuf|ImmExt|NoRex64,	{ Reg32|Reg64, Reg64, Reg64 } },
 {"movddup",   2, 0xf20f12,  X, CpuPNI, NoSuf|IgnoreSize|Modrm,	{ RegXMM|LLongMem, RegXMM, 0 } },
 {"movshdup",  2, 0xf30f16,  X, CpuPNI, NoSuf|IgnoreSize|Modrm,	{ RegXMM|LLongMem, RegXMM, 0 } },
 {"movsldup",  2, 0xf30f12,  X, CpuPNI, NoSuf|IgnoreSize|Modrm,	{ RegXMM|LLongMem, RegXMM, 0 } },
 {"mwait",     0, 0x0f01, 0xc9, CpuPNI, NoSuf|ImmExt,	{ 0, 0, 0} },
+/* mwait is very special. AX and CX are always 64bits with zero upper
+   32bits in 64bit mode, and 32bits in 16bit and 32bit modes.  */
 /* Need to ensure only "mwait %eax,%ecx" is accepted.  */
-{"mwait",     2, 0x0f01, 0xc9, CpuPNI, NoSuf|ImmExt,	{ Reg32, Reg32, 0} },
+{"mwait",     2, 0x0f01, 0xc9, CpuPNI|CpuNo64, NoSuf|ImmExt,	{ Reg32, Reg32, 0} },
+/* Need to ensure only "mwait %rax,%rcx" is accepted.  */
+{"mwait",     2, 0x0f01, 0xc9, CpuPNI|Cpu64, NoSuf|ImmExt|NoRex64,	{ Reg64, Reg64, 0} },
 
 /* VMX instructions.  */
 {"vmcall",    0, 0x0f01, 0xc1, CpuVMX, NoSuf|ImmExt,	{ 0, 0, 0} },
--- binutils/opcodes/i386-dis.c.pni	2005-11-11 10:48:50.000000000 -0800
+++ binutils/opcodes/i386-dis.c	2005-11-18 09:12:15.000000000 -0800
@@ -118,6 +118,9 @@ struct dis_private {
 /* Set to 1 for 64bit mode disassembly.  */
 static int mode_64bit;
 
+/* Set to 1 for 16bit mode disassembly.  */
+static int mode_16bit;
+
 /* Flags for the prefixes for the current instruction.  See below.  */
 static int prefixes;
 
@@ -1972,6 +1975,7 @@ print_insn (bfd_vma pc, disassemble_info
 
   mode_64bit = (info->mach == bfd_mach_x86_64_intel_syntax
 		|| info->mach == bfd_mach_x86_64);
+  mode_16bit = 0;
 
   if (intel_syntax == (char) -1)
     intel_syntax = (info->mach == bfd_mach_i386_i386_intel_syntax
@@ -1992,16 +1996,19 @@ print_insn (bfd_vma pc, disassemble_info
       if (strncmp (p, "x86-64", 6) == 0)
 	{
 	  mode_64bit = 1;
+	  mode_16bit = 0;
 	  priv.orig_sizeflag = AFLAG | DFLAG;
 	}
       else if (strncmp (p, "i386", 4) == 0)
 	{
 	  mode_64bit = 0;
+	  mode_16bit = 0;
 	  priv.orig_sizeflag = AFLAG | DFLAG;
 	}
       else if (strncmp (p, "i8086", 5) == 0)
 	{
 	  mode_64bit = 0;
+	  mode_16bit = 1;
 	  priv.orig_sizeflag = 0;
 	}
       else if (strncmp (p, "intel", 5) == 0)
@@ -4412,18 +4419,30 @@ PNI_Fixup (int extrachar ATTRIBUTE_UNUSE
   if (mod == 3 && reg == 1 && rm <= 1)
     {
       /* Override "sidt".  */
-      char *p = obuf + strlen (obuf) - 4;
+      size_t olen = strlen (obuf);
+      char *p = obuf + olen - 4;
+      const char **names = mode_64bit ? names64 : names32;
 
       /* We might have a suffix when disassembling with -Msuffix.  */
       if (*p == 'i')
 	--p;
 
+      /* Remove "addr16/addr32" if we aren't in Intel mode.  */
+      if (!intel_syntax
+	  && (prefixes & PREFIX_ADDR)
+	  && olen >= (4 + 7)
+	  && *(p - 1) == ' '
+	  && strncmp (p - 7, "addr", 4) == 0
+	  && (strncmp (p - 3, "16", 2) == 0
+	      || strncmp (p - 3, "32", 2) == 0))
+	p -= 7;
+
       if (rm)
 	{
 	  /* mwait %eax,%ecx  */
 	  strcpy (p, "mwait");
 	  if (!intel_syntax)
-	    strcpy (op1out, names32[0]);
+	    strcpy (op1out, names[0]);
 	}
       else
 	{
@@ -4431,21 +4450,22 @@ PNI_Fixup (int extrachar ATTRIBUTE_UNUSE
 	  strcpy (p, "monitor");
 	  if (!intel_syntax)
 	    {
-	      if (!mode_64bit)
-		strcpy (op1out, names32[0]);
-	      else if (!(prefixes & PREFIX_ADDR))
-		strcpy (op1out, names64[0]);
+	      const char **op1_names;
+	      if (!(prefixes & PREFIX_ADDR))
+		op1_names = mode_16bit ? names16 : names;
 	      else
 		{
-		  strcpy (op1out, names32[0]);
+		  op1_names = ((mode_64bit || mode_16bit)
+			       ? names32 : names16);
 		  used_prefixes |= PREFIX_ADDR;
 		}
-	      strcpy (op3out, names32[2]);
+	      strcpy (op1out, op1_names[0]);
+	      strcpy (op3out, names[2]);
 	    }
 	}
       if (!intel_syntax)
 	{
-	  strcpy (op2out, names32[1]);
+	  strcpy (op2out, names[1]);
 	  two_source_ops = 1;
 	}
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]