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]

[PATCH] i386: Remove IgnoreSize from string versions of cmpsd and movsd

From: "H.J. Lu" <hjl dot tools at gmail dot com>
To: Jan Beulich <jbeulich at suse dot com>
Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
Date: Wed, 6 Nov 2019 09:37:35 -0800
Subject: [PATCH] i386: Remove IgnoreSize from string versions of cmpsd and movsd
References: <78d4c121-b563-c41c-9ba8-314653bb25c0@suse.com> <CAMe9rOpqb5pQoG1saWLj6D3EzoHX5JfVgQRoefEZGHJDKYPD7g@mail.gmail.com> <f3a1ab2e-afd7-5761-3074-7cd06a1e3e28@suse.com> <CAMe9rOqYJeXVxWfSc=QOi4bSNABzQg43LRbxgUj-9-OHsimLvA@mail.gmail.com> <165eb513-01f1-473d-3525-590413f71a96@suse.com>

On Wed, Nov 6, 2019 at 9:02 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.11.2019 16:58, H.J. Lu wrote:
> > On Wed, Nov 6, 2019 at 5:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 06.11.2019 01:12, H.J. Lu wrote:
> >>> On Fri, Oct 4, 2019 at 12:45 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> First and foremost the EsSeg attribute was misplaced for CMPSD. Then
> >>>> both it and MOVSD were lacking Dword on both of their operands.
> >>>> Finally string insns with multiple operands and requiring use of ES:
> >>>> had the wrong operand number reported in the diagnostic.
> >>>>
> >>>> gas/
> >>>> 2019-10-04  Jan Beulich  <jbeulich@suse.com>
> >>>>
> >>>>         * config/tc-i386.c (check_string): Make reported operand number
> >>>>         depend on Intel syntax.
> >>>>         * testsuite/gas/i386/intel-cmps.s,
> >>>>         testsuite/gas/i386/intel-cmps32.d,
> >>>>         testsuite/gas/i386/intel-cmps64.d: New.
> >>>>         * testsuite/gas/i386/i386.exp: Run new tests.
> >>>>         * testsuite/gas/i386/intel-movs.s: Extend.
> >>>>         * testsuite/gas/i386/intel-movs32.d,
> >>>>         testsuite/gas/i386/intel-movs64.d: Adjust expectations.
> >>>>         * testsuite/gas/i386/string-bad.l: Tighten expectations.
> >>>>
> >>>> opcodes/
> >>>> 2019-10-04  Jan Beulich  <jbeulich@suse.com>
> >>>>
> >>>>         * opcodes/i386-opc.tbl (movsd): Add Dword and IgnoreSize.
> >>>>         (cmpsd): Likewise. Move EsSeg to other operand.
> >>>>         * opcodes/i386-tbl.h: Re-generate.
> >>>>
> >>>
> >>> This breaks:
> >>>
> >>> [hjl@gnu-skx-1 build-x86_64-linux]$ cat x.s
> >>> .code16
> >>> rep; movsd
> >>> [hjl@gnu-skx-1 build-x86_64-linux]$ gcc -c -m32 x.s
> >>> [hjl@gnu-skx-1 build-x86_64-linux]$ objdump -dw -Mi8086 x.o
> >>>
> >>> x.o:     file format elf32-i386
> >>>
> >>>
> >>> Disassembly of section .text:
> >>>
> >>> 00000000 <.text>:
> >>>    0: f3 66 a5              rep movsl %ds:(%si),%es:(%di)
> >>> [hjl@gnu-skx-1 build-x86_64-linux]$ gcc -c -m32 x.s -B/bin/
> >>> [hjl@gnu-skx-1 build-x86_64-linux]$ objdump -dw -Mi8086 x.o
> >>>
> >>> x.o:     file format elf32-i386
> >>>
> >>>
> >>> Disassembly of section .text:
> >>>
> >>> 00000000 <.text>:
> >>>    0: f3 a5                rep movsw %ds:(%si),%es:(%di)  <<<<<<< This is wrong.
> >>
> >> I suppose that's the IgnoreSize that I mistakenly added also to
> >> the operand-less forms. You've already approved this as a separate
> >
> > It is wrong for both forms.
>
> But it is helpful to know that the operand-less form can be fixed
> by simply dropping the bad IgnoreSize. I think I also see what needs
> changing for the forms with operands, but I have to yet try it out.
> Also I will want to further extend the intel-cmps and intel-movs
> test cases, which is going to take a little more time.
>

This is the patch I am testing.


-- 
H.J.

From 8f01d3580b9b3f383bdabadd6c5c43cf7a235090 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 6 Nov 2019 09:31:12 -0800
Subject: [PATCH] i386: Remove IgnoreSize from string versions of cmpsd and
 movsd

Since string version of 32-bit cmpsd and movsd are used to distinguish
them from SSE versions of cmpsd and movsd, they need size prefix in
16-bit mode.

gas/

	PR gas/25167
	* config/tc-i386.c (match_template): Special cases for string
	versions of cmpsd and movsd.
	* testsuite/gas/i386/code16.d: New file.
	* testsuite/gas/i386/code16.s: Likewise.
	* testsuite/gas/i386/i386.exp: Run code16.

opcodes/

	PR gas/25167
	* i386-opc.tbl: Remove IgnoreSize from string versions of cmpsd
	and movsd.
	* i386-tbl.h: Regenerated.
---
 gas/config/tc-i386.c            |  8 +++++++-
 gas/testsuite/gas/i386/code16.d | 15 +++++++++++++++
 gas/testsuite/gas/i386/code16.s |  9 +++++++++
 gas/testsuite/gas/i386/i386.exp |  1 +
 opcodes/i386-opc.tbl            |  8 ++++----
 opcodes/i386-tbl.h              |  8 ++++----
 6 files changed, 40 insertions(+), 9 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/code16.d
 create mode 100644 gas/testsuite/gas/i386/code16.s

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 5866bd618e..9c3985124e 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5751,7 +5751,13 @@ match_template (char mnem_suffix)
       if ((!intel_syntax || !t->opcode_modifier.ignoresize)
 	  && ((t->opcode_modifier.no_bsuf && suffix_check.no_bsuf)
 	      || (t->opcode_modifier.no_wsuf && suffix_check.no_wsuf)
-	      || (t->opcode_modifier.no_lsuf && suffix_check.no_lsuf)
+	      || (t->opcode_modifier.no_lsuf
+		  && suffix_check.no_lsuf
+		  /* NB: Special cases for string versions of cmpsd and
+		     movsd.  */
+		  && (t->opcode_modifier.size != SIZE32
+		      || (t->base_opcode != 0xa5
+			  && t->base_opcode != 0xa7)))
 	      || (t->opcode_modifier.no_ssuf && suffix_check.no_ssuf)
 	      || (t->opcode_modifier.no_qsuf && suffix_check.no_qsuf)
 	      || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf)))
diff --git a/gas/testsuite/gas/i386/code16.d b/gas/testsuite/gas/i386/code16.d
new file mode 100644
index 0000000000..b860448d6c
--- /dev/null
+++ b/gas/testsuite/gas/i386/code16.d
@@ -0,0 +1,15 @@
+#objdump: -drw -mi8086
+#name: i386 with .code16
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <.text>:
+ +[a-f0-9]+:	f3 66 a5             	rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+:	f3 66 a7             	repz cmpsl %es:\(%di\),%ds:\(%si\)
+ +[a-f0-9]+:	66 f3 a5             	rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+:	66 f3 a7             	repz cmpsl %es:\(%di\),%ds:\(%si\)
+ +[a-f0-9]+:	66 f3 a5             	rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+:	66 f3 a7             	repz cmpsl %es:\(%di\),%ds:\(%si\)
+#pass
diff --git a/gas/testsuite/gas/i386/code16.s b/gas/testsuite/gas/i386/code16.s
new file mode 100644
index 0000000000..d18fa5179a
--- /dev/null
+++ b/gas/testsuite/gas/i386/code16.s
@@ -0,0 +1,9 @@
+	.text
+	.code16
+	rep; movsd
+	rep; cmpsd
+	rep movsd %ds:(%si),%es:(%di)
+	rep cmpsd %es:(%di),%ds:(%si)
+	.intel_syntax noprefix
+	rep movsd dword ptr es:[di], dword ptr ds:[si]
+	rep cmpsd dword ptr ds:[si], dword ptr es:[di]
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index ff2d235713..4e7f8b982d 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -132,6 +132,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
     run_dump_test "noreg32"
     run_dump_test "addr16"
     run_dump_test "addr32"
+    run_dump_test "code16"
     run_list_test "oversized16" "-al"
     run_dump_test "sse4_1"
     run_dump_test "sse4_1-intel"
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 02c83a30f8..b64afc26c8 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -1393,8 +1393,8 @@ cmpunordsd, 2, 0xf20fc2, 0x3, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lS
 cmppd, 3, 0x66c2, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexVVVV=1|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM }
 cmppd, 3, 0x660fc2, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM }
 // Intel mode string compare.
-cmpsd, 0, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-cmpsd, 2, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex|EsSeg, Dword|Unspecified|BaseIndex }
+cmpsd, 0, 0xa7, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
+cmpsd, 2, 0xa7, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex|EsSeg, Dword|Unspecified|BaseIndex }
 cmpsd, 3, 0xf2c2, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
 cmpsd, 3, 0xf20fc2, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
 comisd, 2, 0x662f, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
@@ -1429,8 +1429,8 @@ movmskpd, 2, 0x660f50, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSu
 movntpd, 2, 0x662b, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { RegXMM, Xmmword|Unspecified|BaseIndex }
 movntpd, 2, 0x660f2b, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM, Xmmword|Unspecified|BaseIndex }
 // Intel mode string move.
-movsd, 0, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-movsd, 2, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex|EsSeg }
+movsd, 0, 0xa5, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
+movsd, 2, 0xa5, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex|EsSeg }
 movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex, RegXMM }
 movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { RegXMM, RegXMM }
 movsd, 2, 0xf20f10, None, 2, CpuSSE2, D|Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index 3156bc8a15..b5a3d881bb 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -15594,7 +15594,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0 } },
-    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
       1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0 },
@@ -15607,7 +15607,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0 } },
-    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
       1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0 },
@@ -16121,7 +16121,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0 } },
-    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
       1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0 },
@@ -16134,7 +16134,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0 } },
-    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+    { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
       1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0 },
-- 
2.21.0

Follow-Ups:
- Re: [PATCH] i386: Remove IgnoreSize from string versions of cmpsd and movsd
  - From: Jan Beulich

References:
- Re: [PATCH] x86/Intel: correct MOVSD and CMPSD handling
  - From: H.J. Lu
- Re: [PATCH] x86/Intel: correct MOVSD and CMPSD handling
  - From: Jan Beulich
- Re: [PATCH] x86/Intel: correct MOVSD and CMPSD handling
  - From: H.J. Lu
- Re: [PATCH] x86/Intel: correct MOVSD and CMPSD handling
  - From: Jan Beulich

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