i386: Remove IgnoreSize from string versions of cmpsd and movsd

Message ID CAMe9rOq+as5Y2JOFx3tDS3uTiXkeVoiruxtb=cYbtDXh1Mx0mA@mail.gmail.com
State New
Headers show
Series
  • i386: Remove IgnoreSize from string versions of cmpsd and movsd
Related show

Commit Message

H.J. Lu Nov. 6, 2019, 5:37 p.m.
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.

Comments

Jan Beulich Nov. 7, 2019, 7:12 a.m. | #1
On 06.11.2019 18:37, H.J. Lu wrote:
> This is the patch I am testing.


This is more complicated than my planned change, and I think
my variant would also address a general (previously just
latent) issue, whereas yours simply does it the "brute force"
way by adding special casing of individual insn variants. Let
me see if my variant actually works.

Also I don't think the way your testsuite extension is done
is appropriate: I don't mind the addition, but I think the
primary part should be 16-bit variants of intel-cmps.s and
intel-movs.s. At least that's what I'm about to do.

Jan

Patch

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