public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86-64: Properly encode and decode movsxd
@ 2020-01-23 20:25 H.J. Lu
  2020-01-24  8:21 ` Jan Beulich
  2020-02-10 10:50 ` Jan Beulich
  0 siblings, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2020-01-23 20:25 UTC (permalink / raw)
  To: binutils

movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
destination registers.  Its AT&T mnemonic is movslq which only supports
64-bit destination register.  There is also a discrepancy between AMD64
and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
32-bit source operand and Intel64 supports 16-bit source operand.

This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
destination registers.  It also handles movsxd with 16-bit destination
register for AMD64 and Intel 64.

gas/

	PR binutils/25445
	* config/tc-i386.c (check_long_reg): Also convert to QWORD for
	movsxd.
	* doc/c-i386.texi: Add a node for AMD64 vs. Intel64 ISA
	differences.  Document movslq and movsxd.
	* testsuite/gas/i386/i386.exp: Run PR binutils/25445 tests.
	* testsuite/gas/i386/x86-64-movsxd-intel.d: New file.
	* testsuite/gas/i386/x86-64-movsxd-intel64-intel.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64-inval.l: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64-inval.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-inval.l: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-inval.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd.s: Likewise.

opcodes/

	PR binutils/25445
	* i386-dis.c (MOVSXD_Fixup): New.
	(movsxd_mode): New enum.
	(x86_64_table): Use MOVSXD_Fixup and movsxd_mode on movsxd.
	(intel_operand_size): Handle movsxd_mode.
	(OP_E_register): Likewise.
	(OP_G): Likewise.
	* i386-opc.tbl: Remove Rex64 and allow 32-bit destination
	register on movsxd.  Add movsxd with 16-bit destination register
	for AMD64 and Intel64 ISAs.
	* i386-tbl.h: Regenerated.
---
 gas/config/tc-i386.c                          |  4 +-
 gas/doc/c-i386.texi                           | 18 ++++++
 gas/testsuite/gas/i386/i386.exp               |  6 ++
 gas/testsuite/gas/i386/x86-64-movsxd-intel.d  | 26 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-intel.d    | 26 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-inval.l    | 27 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-inval.s    | 14 +++++
 .../gas/i386/x86-64-movsxd-intel64.d          | 25 ++++++++
 .../gas/i386/x86-64-movsxd-intel64.s          | 20 ++++++
 gas/testsuite/gas/i386/x86-64-movsxd-inval.l  | 27 ++++++++
 gas/testsuite/gas/i386/x86-64-movsxd-inval.s  | 14 +++++
 gas/testsuite/gas/i386/x86-64-movsxd.d        | 25 ++++++++
 gas/testsuite/gas/i386/x86-64-movsxd.s        | 20 ++++++
 opcodes/i386-dis.c                            | 61 ++++++++++++++++++-
 opcodes/i386-opc.tbl                          |  4 +-
 opcodes/i386-tbl.h                            | 34 ++++++++++-
 16 files changed, 344 insertions(+), 7 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-inval.l
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd.s

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 34778ae7605..e3c971ca499 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6690,7 +6690,9 @@ check_long_reg (void)
 	     && i.tm.operand_types[op].bitfield.dword)
       {
 	if (intel_syntax
-	    && i.tm.opcode_modifier.toqword
+	    && (i.tm.opcode_modifier.toqword
+		/* Also convert to QWORD for MOVSXD.  */
+		|| i.tm.base_opcode == 0x63)
 	    && i.types[0].bitfield.class != RegSIMD)
 	  {
 	    /* Convert to QWORD.  We want REX byte. */
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 6d556a01a10..9fb681e8729 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -37,6 +37,7 @@ extending the Intel architecture to 64-bits.
 * i386-TBM::                    AMD's Trailing Bit Manipulation Instructions
 * i386-16bit::                  Writing 16-bit Code
 * i386-Arch::                   Specifying an x86 CPU architecture
+* i386-ISA::                    AMD64 ISA vs. Intel64 ISA
 * i386-Bugs::                   AT&T Syntax bugs
 * i386-Notes::                  Notes
 @end menu
@@ -856,6 +857,12 @@ Several x87 instructions, @samp{fadd}, @samp{fdiv}, @samp{fdivp},
 assembler with different mnemonics from those in Intel IA32 specification.
 @code{@value{GCC}} generates those instructions with AT&T mnemonic.
 
+@itemize @bullet
+@item @samp{movslq} with AT&T mnemonic only accepts 64-bit destination
+register.  @samp{movsxd} should be used to encode 16-bit or 32-bit
+destination register with both AT&T and Intel mnemonics.
+@end itemize
+
 @node i386-Regs
 @section Register Naming
 
@@ -1438,6 +1445,17 @@ For example
  .arch i8086,nojumps
 @end smallexample
 
+@node i386-ISA
+@section AMD64 ISA vs. Intel64 ISA
+
+There are some discrepancies between AMD64 and Intel64 ISAs.
+
+@itemize @bullet
+@item For @samp{movsxd} with 16-bit destination register, AMD64
+supports 32-bit source operand and Intel64 supports 16-bit source
+operand.
+@end itemize
+
 @node i386-Bugs
 @section AT&T Syntax bugs
 
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index aedcf147380..feab8c2be95 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1050,6 +1050,12 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
     run_dump_test "x86-64-movd-intel"
     run_dump_test "x86-64-nop-1"
     run_dump_test "x86-64-nop-2"
+    run_dump_test "x86-64-movsxd"
+    run_dump_test "x86-64-movsxd-intel"
+    run_list_test "x86-64-movsxd-inval" "-al"
+    run_dump_test "x86-64-movsxd-intel64"
+    run_dump_test "x86-64-movsxd-intel64-intel"
+    run_list_test "x86-64-movsxd-intel64-inval" "-mintel64 -al"
     run_dump_test "x86-64-optimize-1"
     run_dump_test "x86-64-optimize-2"
     run_dump_test "x86-64-optimize-2a"
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel.d
new file mode 100644
index 00000000000..b7f55d41681
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel.d
@@ -0,0 +1,26 @@
+#source: x86-64-movsxd.s
+#as:
+#objdump: -dw -Mintel
+#name: x86-64 movsxd (AMD64) (Intel mode)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 08             	movsxd cx,DWORD PTR \[rax\]
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
new file mode 100644
index 00000000000..1145df2971f
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
@@ -0,0 +1,26 @@
+#source: x86-64-movsxd-intel64.s
+#as: -mintel64
+#objdump: -dw -Mintel -Mintel64
+#name: x86-64 movsxd (Intel64) (Intel mode)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,ax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,ax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
new file mode 100644
index 00000000000..b3219e0c671
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
@@ -0,0 +1,27 @@
+.*: Assembler messages:
+.*:4: Error: .*
+.*:5: Error: .*
+.*:6: Error: .*
+.*:7: Error: .*
+.*:10: Error: .*
+.*:11: Error: .*
+.*:12: Error: .*
+.*:13: Error: .*
+.*:14: Error: .*
+GAS LISTING .*
+
+
+[ 	]*1[ 	]+\# 64-bit only invalid MOVSXD with Intel64 ISA
+[ 	]*2[ 	]+\.text
+[ 	]*3[ 	]+_start:
+[ 	]*4[ 	]+movslq	%eax, %cx
+[ 	]*5[ 	]+movslq	%eax, %ecx
+[ 	]*6[ 	]+movslq	\(%rax\), %ecx
+[ 	]*7[ 	]+movsxd	%ax, %ecx
+[ 	]*8[ 	]+
+[ 	]*9[ 	]+\.intel_syntax noprefix
+[ 	]*10[ 	]+movslq	cx, ax
+[ 	]*11[ 	]+movslq	ecx, eax
+[ 	]*12[ 	]+movslq	ecx, \[rax\]
+[ 	]*13[ 	]+movsxd	cx, eax
+[ 	]*14[ 	]+movsxd	cx, DWORD PTR \[rax\]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
new file mode 100644
index 00000000000..2edcaf83302
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
@@ -0,0 +1,14 @@
+# 64-bit only invalid MOVSXD with Intel64 ISA
+	.text
+_start:
+	movslq	%eax, %cx
+	movslq	%eax, %ecx
+	movslq	(%rax), %ecx
+	movsxd	%ax, %ecx
+
+	.intel_syntax noprefix
+	movslq	cx, ax
+	movslq	ecx, eax
+	movslq	ecx, [rax]
+	movsxd	cx, eax
+	movsxd	cx, DWORD PTR [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
new file mode 100644
index 00000000000..afd26d93a51
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
@@ -0,0 +1,25 @@
+#as: -mintel64
+#objdump: -dw -Mintel64
+#name: x86-64 movsxd (Intel64)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %ax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %ax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
new file mode 100644
index 00000000000..842cdef42f5
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
@@ -0,0 +1,20 @@
+# 64-bit only MOVSXD with Intel64 ISA
+	.text
+_start:
+	movslq	%eax, %rcx
+	movslq	(%rax), %rcx
+	movsxd	%eax, %ecx
+	movsxd	(%rax), %ecx
+	movsxd	%ax, %cx
+	movsxd	(%rax), %cx
+
+	.intel_syntax noprefix
+	movsxd	rcx, eax
+	movsxd	rcx, DWORD PTR [rax]
+	movsxd	rcx, [rax]
+	movsxd	ecx, eax
+	movsxd	ecx, DWORD PTR [rax]
+	movsxd	ecx, [rax]
+	movsxd	cx, ax
+	movsxd	cx, WORD PTR [rax]
+	movsxd	cx, [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-inval.l b/gas/testsuite/gas/i386/x86-64-movsxd-inval.l
new file mode 100644
index 00000000000..7db46d6af39
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-inval.l
@@ -0,0 +1,27 @@
+.*: Assembler messages:
+.*:4: Error: .*
+.*:5: Error: .*
+.*:6: Error: .*
+.*:7: Error: .*
+.*:10: Error: .*
+.*:11: Error: .*
+.*:12: Error: .*
+.*:13: Error: .*
+.*:14: Error: .*
+GAS LISTING .*
+
+
+[ 	]*1[ 	]+\# 64-bit only invalid MOVSXD with AMD64 ISA
+[ 	]*2[ 	]+\.text
+[ 	]*3[ 	]+_start:
+[ 	]*4[ 	]+movslq	%ax, %cx
+[ 	]*5[ 	]+movslq	%eax, %ecx
+[ 	]*6[ 	]+movslq	\(%rax\), %ecx
+[ 	]*7[ 	]+movsxd	%ax, %cx
+[ 	]*8[ 	]+
+[ 	]*9[ 	]+\.intel_syntax noprefix
+[ 	]*10[ 	]+movslq	cx, eax
+[ 	]*11[ 	]+movslq	ecx, eax
+[ 	]*12[ 	]+movslq	ecx, \[rax\]
+[ 	]*13[ 	]+movsxd	cx, ax
+[ 	]*14[ 	]+movsxd	cx, WORD PTR \[rax\]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-inval.s b/gas/testsuite/gas/i386/x86-64-movsxd-inval.s
new file mode 100644
index 00000000000..84bf5209057
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-inval.s
@@ -0,0 +1,14 @@
+# 64-bit only invalid MOVSXD with AMD64 ISA
+	.text
+_start:
+	movslq	%ax, %cx
+	movslq	%eax, %ecx
+	movslq	(%rax), %ecx
+	movsxd	%ax, %cx
+
+	.intel_syntax noprefix
+	movslq	cx, eax
+	movslq	ecx, eax
+	movslq	ecx, [rax]
+	movsxd	cx, ax
+	movsxd	cx, WORD PTR [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd.d b/gas/testsuite/gas/i386/x86-64-movsxd.d
new file mode 100644
index 00000000000..1881fe2e313
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd.d
@@ -0,0 +1,25 @@
+#as:
+#objdump: -dw
+#name: x86-64 movsxd (AMD64)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd.s b/gas/testsuite/gas/i386/x86-64-movsxd.s
new file mode 100644
index 00000000000..f0efd59319a
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd.s
@@ -0,0 +1,20 @@
+# 64-bit only MOVSXD with AMD64 ISA
+	.text
+_start:
+	movslq	%eax, %rcx
+	movslq	(%rax), %rcx
+	movsxd	%eax, %ecx
+	movsxd	(%rax), %ecx
+	movsxd	%eax, %cx
+	movsxd	(%rax), %cx
+
+	.intel_syntax noprefix
+	movsxd	rcx, eax
+	movsxd	rcx, DWORD PTR [rax]
+	movsxd	rcx, [rax]
+	movsxd	ecx, eax
+	movsxd	ecx, DWORD PTR [rax]
+	movsxd	ecx, [rax]
+	movsxd	cx, eax
+	movsxd	cx, DWORD PTR [rax]
+	movsxd	cx, [rax]
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index c73e964b546..e6f73bff206 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -124,6 +124,7 @@ static void OP_Vex_2src_1 (int, int);
 static void OP_Vex_2src_2 (int, int);
 
 static void MOVBE_Fixup (int, int);
+static void MOVSXD_Fixup (int, int);
 
 static void OP_Mask (int, int);
 
@@ -556,6 +557,7 @@ enum
   a_mode,
   cond_jump_mode,
   loop_jcxz_mode,
+  movsxd_mode,
   v_bnd_mode,
   /* like v_bnd_mode in 32bit, no RIP-rel in 64bit mode.  */
   v_bndmk_mode,
@@ -6873,7 +6875,7 @@ static const struct dis386 x86_64_table[][2] = {
   /* X86_64_63 */
   {
     { "arpl", { Ew, Gw }, 0 },
-    { "movs{lq|xd}", { Gv, Ed }, 0 },
+    { "movs", { { OP_G, movsxd_mode }, { MOVSXD_Fixup, movsxd_mode } }, 0 },
   },
 
   /* X86_64_6D */
@@ -13536,6 +13538,13 @@ intel_operand_size (int bytemode, int sizeflag)
 	oappend ("DWORD PTR ");
       used_prefixes |= (prefixes & PREFIX_DATA);
       break;
+    case movsxd_mode:
+      if (!(sizeflag & DFLAG) && isa64 == intel64)
+	oappend ("WORD PTR ");
+      else
+	oappend ("DWORD PTR ");
+      used_prefixes |= (prefixes & PREFIX_DATA);
+      break;
     case d_mode:
     case d_scalar_mode:
     case d_scalar_swap_mode:
@@ -13921,6 +13930,13 @@ OP_E_register (int bytemode, int sizeflag)
 	  used_prefixes |= (prefixes & PREFIX_DATA);
 	}
       break;
+    case movsxd_mode:
+      if (!(sizeflag & DFLAG) && isa64 == intel64)
+	names = names16;
+      else
+	names = names32;
+      used_prefixes |= (prefixes & PREFIX_DATA);
+      break;
     case va_mode:
       names = (address_mode == mode_64bit
 	       ? names64 : names32);
@@ -14492,12 +14508,14 @@ OP_G (int bytemode, int sizeflag)
     case dqb_mode:
     case dqd_mode:
     case dqw_mode:
+    case movsxd_mode:
       USED_REX (REX_W);
       if (rex & REX_W)
 	oappend (names64[modrm.reg + add]);
       else
 	{
-	  if ((sizeflag & DFLAG) || bytemode != v_mode)
+	  if ((sizeflag & DFLAG)
+	      || (bytemode != v_mode && bytemode != movsxd_mode))
 	    oappend (names32[modrm.reg + add]);
 	  else
 	    oappend (names16[modrm.reg + add]);
@@ -16563,6 +16581,45 @@ skip:
   OP_M (bytemode, sizeflag);
 }
 
+static void
+MOVSXD_Fixup (int bytemode, int sizeflag)
+{
+  /* Add proper suffix to "movsxd".  */
+  char *p = mnemonicendp;
+
+  switch (bytemode)
+    {
+    case movsxd_mode:
+      if (intel_syntax)
+	{
+	  *p++ = 'x';
+	  *p++ = 'd';
+	  goto skip;
+	}
+
+      USED_REX (REX_W);
+      if (rex & REX_W)
+	{
+	  *p++ = 'l';
+	  *p++ = 'q';
+	}
+      else
+	{
+	  *p++ = 'x';
+	  *p++ = 'd';
+	}
+      break;
+    default:
+      oappend (INTERNAL_DISASSEMBLER_ERROR);
+      break;
+    }
+
+skip:
+  mnemonicendp = p;
+  *p = '\0';
+  OP_E (bytemode, sizeflag);
+}
+
 static void
 OP_LWPCB_E (int bytemode ATTRIBUTE_UNUSED, int sizeflag ATTRIBUTE_UNUSED)
 {
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 2acb76bfa15..19793fdcd45 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -135,7 +135,9 @@ movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|R
 movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg8|Byte|BaseIndex, Reg16|Reg32|Reg64 }
 movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg16|Word|BaseIndex, Reg32|Reg64 }
 movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64|IntelSyntax, { Reg32|Dword|BaseIndex, Reg64 }
-movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex, Reg64 }
+movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
+movsxd, 2, 0x63, None, 1, Cpu64, AMD64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg16 }
+movsxd, 2, 0x63, None, 1, Cpu64, Intel64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Unspecified|BaseIndex, Reg16 }
 
 // Move with zero extend.
 movzb, 2, 0xfb6, None, 2, Cpu386, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index c8fa7e95a00..d1a6c0915a9 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -435,12 +435,40 @@ 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, 1, 0, 0 } },
-    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 1, 1, 1, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0,
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
     { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 0,
 	  0, 0, 0, 0, 1, 0 } },
-      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1,
+	  0, 0, 0, 0, 0, 0 } } } },
+  { "movsxd", 0x63, None, 1, 2,
+    { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0 } },
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0 },
+    { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 0,
+	  0, 0, 0, 0, 1, 0 } },
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
+	  0, 0, 0, 0, 0, 0 } } } },
+  { "movsxd", 0x63, None, 1, 2,
+    { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0 } },
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 },
+    { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 1, 0, 0, 0,
+	  0, 0, 0, 0, 1, 0 } },
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
 	  0, 0, 0, 0, 0, 0 } } } },
   { "movzb", 0xfb6, None, 2, 2,
     { { 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-- 
2.24.1

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-23 20:25 [PATCH] x86-64: Properly encode and decode movsxd H.J. Lu
@ 2020-01-24  8:21 ` Jan Beulich
  2020-01-24 11:52   ` H.J. Lu
  2020-02-10 10:50 ` Jan Beulich
  1 sibling, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-01-24  8:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On 23.01.2020 21:24, H.J. Lu wrote:
> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> destination registers.  Its AT&T mnemonic is movslq which only supports
> 64-bit destination register.  There is also a discrepancy between AMD64
> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> 32-bit source operand and Intel64 supports 16-bit source operand.
> 
> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> destination registers.  It also handles movsxd with 16-bit destination
> register for AMD64 and Intel 64.

I appreciate you taking care of the disassembler and documentation
side, but would you mind pointing out what's wrong with the previously
posted patches making the assembler deal with this correctly? For
example, I get away ...

> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6690,7 +6690,9 @@ check_long_reg (void)
>  	     && i.tm.operand_types[op].bitfield.dword)
>        {
>  	if (intel_syntax
> -	    && i.tm.opcode_modifier.toqword
> +	    && (i.tm.opcode_modifier.toqword
> +		/* Also convert to QWORD for MOVSXD.  */
> +		|| i.tm.base_opcode == 0x63)
>  	    && i.types[0].bitfield.class != RegSIMD)

... without such a (pretty arbitrary) adjustment (as an aside: the
comment is slightly misleading in that the change also affects some
MOVSX templates), while ...

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -135,7 +135,9 @@ movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|R
>  movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg8|Byte|BaseIndex, Reg16|Reg32|Reg64 }
>  movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg16|Word|BaseIndex, Reg32|Reg64 }
>  movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64|IntelSyntax, { Reg32|Dword|BaseIndex, Reg64 }
> -movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex, Reg64 }
> +movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
> +movsxd, 2, 0x63, None, 1, Cpu64, AMD64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg16 }
> +movsxd, 2, 0x63, None, 1, Cpu64, Intel64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Unspecified|BaseIndex, Reg16 }

... e.g. all of this matches what, in sum, the two patches I've
submitted (which also deal with other insns) do as well. In the
end this means that I'll want to undo the check_long_reg()
adjustment above, in which case my patches could as well go in
alongside your disassembler adjustment.

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-24  8:21 ` Jan Beulich
@ 2020-01-24 11:52   ` H.J. Lu
  2020-01-26 15:01     ` H.J. Lu
  2020-01-27 16:17     ` Jan Beulich
  0 siblings, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2020-01-24 11:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 23.01.2020 21:24, H.J. Lu wrote:
> > movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> > destination registers.  Its AT&T mnemonic is movslq which only supports
> > 64-bit destination register.  There is also a discrepancy between AMD64
> > and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> > 32-bit source operand and Intel64 supports 16-bit source operand.
> >
> > This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> > destination registers.  It also handles movsxd with 16-bit destination
> > register for AMD64 and Intel 64.
>
> I appreciate you taking care of the disassembler and documentation
> side, but would you mind pointing out what's wrong with the previously
> posted patches making the assembler deal with this correctly? For
> example, I get away ...
>
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -6690,7 +6690,9 @@ check_long_reg (void)
> >            && i.tm.operand_types[op].bitfield.dword)
> >        {
> >       if (intel_syntax
> > -         && i.tm.opcode_modifier.toqword
> > +         && (i.tm.opcode_modifier.toqword
> > +             /* Also convert to QWORD for MOVSXD.  */
> > +             || i.tm.base_opcode == 0x63)
> >           && i.types[0].bitfield.class != RegSIMD)
>
> ... without such a (pretty arbitrary) adjustment (as an aside: the
> comment is slightly misleading in that the change also affects some
> MOVSX templates), while ...

My patch in assembler

1.  Keep movslq unchanged, which only allows 64-bit destination register.
2.  Allow DWORD source with movsxd.

My testcases cover the above.  Without the assembler change, I got

FAIL: x86-64 movsxd (AMD64)
FAIL: x86-64 movsxd (AMD64) (Intel mode)
FAIL: x86-64 movsxd (Intel64)
FAIL: x86-64 movsxd (Intel64) (Intel mode)

-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-24 11:52   ` H.J. Lu
@ 2020-01-26 15:01     ` H.J. Lu
  2020-02-03  8:38       ` Jan Beulich
  2020-01-27 16:17     ` Jan Beulich
  1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-01-26 15:01 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

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

On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 23.01.2020 21:24, H.J. Lu wrote:
> > > movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> > > destination registers.  Its AT&T mnemonic is movslq which only supports
> > > 64-bit destination register.  There is also a discrepancy between AMD64
> > > and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> > > 32-bit source operand and Intel64 supports 16-bit source operand.
> > >
> > > This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> > > destination registers.  It also handles movsxd with 16-bit destination
> > > register for AMD64 and Intel 64.
> >
> > I appreciate you taking care of the disassembler and documentation
> > side, but would you mind pointing out what's wrong with the previously
> > posted patches making the assembler deal with this correctly? For
> > example, I get away ...
> >
> > > --- a/gas/config/tc-i386.c
> > > +++ b/gas/config/tc-i386.c
> > > @@ -6690,7 +6690,9 @@ check_long_reg (void)
> > >            && i.tm.operand_types[op].bitfield.dword)
> > >        {
> > >       if (intel_syntax
> > > -         && i.tm.opcode_modifier.toqword
> > > +         && (i.tm.opcode_modifier.toqword
> > > +             /* Also convert to QWORD for MOVSXD.  */
> > > +             || i.tm.base_opcode == 0x63)
> > >           && i.types[0].bitfield.class != RegSIMD)
> >
> > ... without such a (pretty arbitrary) adjustment (as an aside: the
> > comment is slightly misleading in that the change also affects some
> > MOVSX templates), while ...
>
> My patch in assembler
>
> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
> 2.  Allow DWORD source with movsxd.
>
> My testcases cover the above.  Without the assembler change, I got
>
> FAIL: x86-64 movsxd (AMD64)
> FAIL: x86-64 movsxd (AMD64) (Intel mode)
> FAIL: x86-64 movsxd (Intel64)
> FAIL: x86-64 movsxd (Intel64) (Intel mode)
>

I am checking in this patch tomorrow.

-- 
H.J.

[-- Attachment #2: 0001-x86-64-Properly-encode-and-decode-movsxd.patch --]
[-- Type: text/x-patch, Size: 22719 bytes --]

From 6a4f27d706713a69ff5ead8151f28d38a1661881 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 22 Jan 2020 14:32:02 -0800
Subject: [PATCH] x86-64: Properly encode and decode movsxd

movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
destination registers.  Its AT&T mnemonic is movslq which only supports
64-bit destination register.  There is also a discrepancy between AMD64
and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
32-bit source operand and Intel64 supports 16-bit source operand.

This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
destination registers.  It also handles movsxd with 16-bit destination
register for AMD64 and Intel 64.

gas/

2020-01-27  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/25445
	* config/tc-i386.c (check_long_reg): Also convert to QWORD for
	movsxd.
	* doc/c-i386.texi: Add a node for AMD64 vs. Intel64 ISA
	differences.  Document movslq and movsxd.
	* testsuite/gas/i386/i386.exp: Run PR binutils/25445 tests.
	* testsuite/gas/i386/x86-64-movsxd-intel.d: New file.
	* testsuite/gas/i386/x86-64-movsxd-intel64-intel.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64-inval.l: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64-inval.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-intel64.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-inval.l: Likewise.
	* testsuite/gas/i386/x86-64-movsxd-inval.s: Likewise.
	* testsuite/gas/i386/x86-64-movsxd.d: Likewise.
	* testsuite/gas/i386/x86-64-movsxd.s: Likewise.

opcodes/

2020-01-27  H.J. Lu  <hongjiu.lu@intel.com>
	    Jan Beulich  <jbeulich@suse.com>

	PR binutils/25445
	* i386-dis.c (MOVSXD_Fixup): New function.
	(movsxd_mode): New enum.
	(x86_64_table): Use MOVSXD_Fixup and movsxd_mode on movsxd.
	(intel_operand_size): Handle movsxd_mode.
	(OP_E_register): Likewise.
	(OP_G): Likewise.
	* i386-opc.tbl: Remove Rex64 and allow 32-bit destination
	register on movsxd.  Add movsxd with 16-bit destination register
	for AMD64 and Intel64 ISAs.
	* i386-tbl.h: Regenerated.
---
 gas/config/tc-i386.c                          |  4 +-
 gas/doc/c-i386.texi                           | 18 ++++++
 gas/testsuite/gas/i386/i386.exp               |  6 ++
 gas/testsuite/gas/i386/x86-64-movsxd-intel.d  | 26 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-intel.d    | 26 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-inval.l    | 27 ++++++++
 .../gas/i386/x86-64-movsxd-intel64-inval.s    | 14 +++++
 .../gas/i386/x86-64-movsxd-intel64.d          | 25 ++++++++
 .../gas/i386/x86-64-movsxd-intel64.s          | 20 ++++++
 gas/testsuite/gas/i386/x86-64-movsxd-inval.l  | 27 ++++++++
 gas/testsuite/gas/i386/x86-64-movsxd-inval.s  | 14 +++++
 gas/testsuite/gas/i386/x86-64-movsxd.d        | 25 ++++++++
 gas/testsuite/gas/i386/x86-64-movsxd.s        | 20 ++++++
 opcodes/i386-dis.c                            | 61 ++++++++++++++++++-
 opcodes/i386-opc.tbl                          |  4 +-
 opcodes/i386-tbl.h                            | 34 ++++++++++-
 16 files changed, 344 insertions(+), 7 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-inval.l
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-movsxd.s

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 34778ae7605..e3c971ca499 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6690,7 +6690,9 @@ check_long_reg (void)
 	     && i.tm.operand_types[op].bitfield.dword)
       {
 	if (intel_syntax
-	    && i.tm.opcode_modifier.toqword
+	    && (i.tm.opcode_modifier.toqword
+		/* Also convert to QWORD for MOVSXD.  */
+		|| i.tm.base_opcode == 0x63)
 	    && i.types[0].bitfield.class != RegSIMD)
 	  {
 	    /* Convert to QWORD.  We want REX byte. */
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 6d556a01a10..9fb681e8729 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -37,6 +37,7 @@ extending the Intel architecture to 64-bits.
 * i386-TBM::                    AMD's Trailing Bit Manipulation Instructions
 * i386-16bit::                  Writing 16-bit Code
 * i386-Arch::                   Specifying an x86 CPU architecture
+* i386-ISA::                    AMD64 ISA vs. Intel64 ISA
 * i386-Bugs::                   AT&T Syntax bugs
 * i386-Notes::                  Notes
 @end menu
@@ -856,6 +857,12 @@ Several x87 instructions, @samp{fadd}, @samp{fdiv}, @samp{fdivp},
 assembler with different mnemonics from those in Intel IA32 specification.
 @code{@value{GCC}} generates those instructions with AT&T mnemonic.
 
+@itemize @bullet
+@item @samp{movslq} with AT&T mnemonic only accepts 64-bit destination
+register.  @samp{movsxd} should be used to encode 16-bit or 32-bit
+destination register with both AT&T and Intel mnemonics.
+@end itemize
+
 @node i386-Regs
 @section Register Naming
 
@@ -1438,6 +1445,17 @@ For example
  .arch i8086,nojumps
 @end smallexample
 
+@node i386-ISA
+@section AMD64 ISA vs. Intel64 ISA
+
+There are some discrepancies between AMD64 and Intel64 ISAs.
+
+@itemize @bullet
+@item For @samp{movsxd} with 16-bit destination register, AMD64
+supports 32-bit source operand and Intel64 supports 16-bit source
+operand.
+@end itemize
+
 @node i386-Bugs
 @section AT&T Syntax bugs
 
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index aedcf147380..feab8c2be95 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1050,6 +1050,12 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
     run_dump_test "x86-64-movd-intel"
     run_dump_test "x86-64-nop-1"
     run_dump_test "x86-64-nop-2"
+    run_dump_test "x86-64-movsxd"
+    run_dump_test "x86-64-movsxd-intel"
+    run_list_test "x86-64-movsxd-inval" "-al"
+    run_dump_test "x86-64-movsxd-intel64"
+    run_dump_test "x86-64-movsxd-intel64-intel"
+    run_list_test "x86-64-movsxd-intel64-inval" "-mintel64 -al"
     run_dump_test "x86-64-optimize-1"
     run_dump_test "x86-64-optimize-2"
     run_dump_test "x86-64-optimize-2a"
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel.d
new file mode 100644
index 00000000000..b7f55d41681
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel.d
@@ -0,0 +1,26 @@
+#source: x86-64-movsxd.s
+#as:
+#objdump: -dw -Mintel
+#name: x86-64 movsxd (AMD64) (Intel mode)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 08             	movsxd cx,DWORD PTR \[rax\]
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
new file mode 100644
index 00000000000..1145df2971f
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-intel.d
@@ -0,0 +1,26 @@
+#source: x86-64-movsxd-intel64.s
+#as: -mintel64
+#objdump: -dw -Mintel -Mintel64
+#name: x86-64 movsxd (Intel64) (Intel mode)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,ax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 c8                	movsxd ecx,eax
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 c8             	movsxd cx,ax
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+ +[a-f0-9]+:	66 63 08             	movsxd cx,WORD PTR \[rax\]
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
new file mode 100644
index 00000000000..b3219e0c671
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.l
@@ -0,0 +1,27 @@
+.*: Assembler messages:
+.*:4: Error: .*
+.*:5: Error: .*
+.*:6: Error: .*
+.*:7: Error: .*
+.*:10: Error: .*
+.*:11: Error: .*
+.*:12: Error: .*
+.*:13: Error: .*
+.*:14: Error: .*
+GAS LISTING .*
+
+
+[ 	]*1[ 	]+\# 64-bit only invalid MOVSXD with Intel64 ISA
+[ 	]*2[ 	]+\.text
+[ 	]*3[ 	]+_start:
+[ 	]*4[ 	]+movslq	%eax, %cx
+[ 	]*5[ 	]+movslq	%eax, %ecx
+[ 	]*6[ 	]+movslq	\(%rax\), %ecx
+[ 	]*7[ 	]+movsxd	%ax, %ecx
+[ 	]*8[ 	]+
+[ 	]*9[ 	]+\.intel_syntax noprefix
+[ 	]*10[ 	]+movslq	cx, ax
+[ 	]*11[ 	]+movslq	ecx, eax
+[ 	]*12[ 	]+movslq	ecx, \[rax\]
+[ 	]*13[ 	]+movsxd	cx, eax
+[ 	]*14[ 	]+movsxd	cx, DWORD PTR \[rax\]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
new file mode 100644
index 00000000000..2edcaf83302
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64-inval.s
@@ -0,0 +1,14 @@
+# 64-bit only invalid MOVSXD with Intel64 ISA
+	.text
+_start:
+	movslq	%eax, %cx
+	movslq	%eax, %ecx
+	movslq	(%rax), %ecx
+	movsxd	%ax, %ecx
+
+	.intel_syntax noprefix
+	movslq	cx, ax
+	movslq	ecx, eax
+	movslq	ecx, [rax]
+	movsxd	cx, eax
+	movsxd	cx, DWORD PTR [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
new file mode 100644
index 00000000000..afd26d93a51
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.d
@@ -0,0 +1,25 @@
+#as: -mintel64
+#objdump: -dw -Mintel64
+#name: x86-64 movsxd (Intel64)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %ax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %ax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
new file mode 100644
index 00000000000..842cdef42f5
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel64.s
@@ -0,0 +1,20 @@
+# 64-bit only MOVSXD with Intel64 ISA
+	.text
+_start:
+	movslq	%eax, %rcx
+	movslq	(%rax), %rcx
+	movsxd	%eax, %ecx
+	movsxd	(%rax), %ecx
+	movsxd	%ax, %cx
+	movsxd	(%rax), %cx
+
+	.intel_syntax noprefix
+	movsxd	rcx, eax
+	movsxd	rcx, DWORD PTR [rax]
+	movsxd	rcx, [rax]
+	movsxd	ecx, eax
+	movsxd	ecx, DWORD PTR [rax]
+	movsxd	ecx, [rax]
+	movsxd	cx, ax
+	movsxd	cx, WORD PTR [rax]
+	movsxd	cx, [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-inval.l b/gas/testsuite/gas/i386/x86-64-movsxd-inval.l
new file mode 100644
index 00000000000..7db46d6af39
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-inval.l
@@ -0,0 +1,27 @@
+.*: Assembler messages:
+.*:4: Error: .*
+.*:5: Error: .*
+.*:6: Error: .*
+.*:7: Error: .*
+.*:10: Error: .*
+.*:11: Error: .*
+.*:12: Error: .*
+.*:13: Error: .*
+.*:14: Error: .*
+GAS LISTING .*
+
+
+[ 	]*1[ 	]+\# 64-bit only invalid MOVSXD with AMD64 ISA
+[ 	]*2[ 	]+\.text
+[ 	]*3[ 	]+_start:
+[ 	]*4[ 	]+movslq	%ax, %cx
+[ 	]*5[ 	]+movslq	%eax, %ecx
+[ 	]*6[ 	]+movslq	\(%rax\), %ecx
+[ 	]*7[ 	]+movsxd	%ax, %cx
+[ 	]*8[ 	]+
+[ 	]*9[ 	]+\.intel_syntax noprefix
+[ 	]*10[ 	]+movslq	cx, eax
+[ 	]*11[ 	]+movslq	ecx, eax
+[ 	]*12[ 	]+movslq	ecx, \[rax\]
+[ 	]*13[ 	]+movsxd	cx, ax
+[ 	]*14[ 	]+movsxd	cx, WORD PTR \[rax\]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd-inval.s b/gas/testsuite/gas/i386/x86-64-movsxd-inval.s
new file mode 100644
index 00000000000..84bf5209057
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd-inval.s
@@ -0,0 +1,14 @@
+# 64-bit only invalid MOVSXD with AMD64 ISA
+	.text
+_start:
+	movslq	%ax, %cx
+	movslq	%eax, %ecx
+	movslq	(%rax), %ecx
+	movsxd	%ax, %cx
+
+	.intel_syntax noprefix
+	movslq	cx, eax
+	movslq	ecx, eax
+	movslq	ecx, [rax]
+	movsxd	cx, ax
+	movsxd	cx, WORD PTR [rax]
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd.d b/gas/testsuite/gas/i386/x86-64-movsxd.d
new file mode 100644
index 00000000000..1881fe2e313
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd.d
@@ -0,0 +1,25 @@
+#as:
+#objdump: -dw
+#name: x86-64 movsxd (AMD64)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+ +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
+ +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
+ +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
+ +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-movsxd.s b/gas/testsuite/gas/i386/x86-64-movsxd.s
new file mode 100644
index 00000000000..f0efd59319a
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-movsxd.s
@@ -0,0 +1,20 @@
+# 64-bit only MOVSXD with AMD64 ISA
+	.text
+_start:
+	movslq	%eax, %rcx
+	movslq	(%rax), %rcx
+	movsxd	%eax, %ecx
+	movsxd	(%rax), %ecx
+	movsxd	%eax, %cx
+	movsxd	(%rax), %cx
+
+	.intel_syntax noprefix
+	movsxd	rcx, eax
+	movsxd	rcx, DWORD PTR [rax]
+	movsxd	rcx, [rax]
+	movsxd	ecx, eax
+	movsxd	ecx, DWORD PTR [rax]
+	movsxd	ecx, [rax]
+	movsxd	cx, eax
+	movsxd	cx, DWORD PTR [rax]
+	movsxd	cx, [rax]
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index c73e964b546..e6f73bff206 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -124,6 +124,7 @@ static void OP_Vex_2src_1 (int, int);
 static void OP_Vex_2src_2 (int, int);
 
 static void MOVBE_Fixup (int, int);
+static void MOVSXD_Fixup (int, int);
 
 static void OP_Mask (int, int);
 
@@ -556,6 +557,7 @@ enum
   a_mode,
   cond_jump_mode,
   loop_jcxz_mode,
+  movsxd_mode,
   v_bnd_mode,
   /* like v_bnd_mode in 32bit, no RIP-rel in 64bit mode.  */
   v_bndmk_mode,
@@ -6873,7 +6875,7 @@ static const struct dis386 x86_64_table[][2] = {
   /* X86_64_63 */
   {
     { "arpl", { Ew, Gw }, 0 },
-    { "movs{lq|xd}", { Gv, Ed }, 0 },
+    { "movs", { { OP_G, movsxd_mode }, { MOVSXD_Fixup, movsxd_mode } }, 0 },
   },
 
   /* X86_64_6D */
@@ -13536,6 +13538,13 @@ intel_operand_size (int bytemode, int sizeflag)
 	oappend ("DWORD PTR ");
       used_prefixes |= (prefixes & PREFIX_DATA);
       break;
+    case movsxd_mode:
+      if (!(sizeflag & DFLAG) && isa64 == intel64)
+	oappend ("WORD PTR ");
+      else
+	oappend ("DWORD PTR ");
+      used_prefixes |= (prefixes & PREFIX_DATA);
+      break;
     case d_mode:
     case d_scalar_mode:
     case d_scalar_swap_mode:
@@ -13921,6 +13930,13 @@ OP_E_register (int bytemode, int sizeflag)
 	  used_prefixes |= (prefixes & PREFIX_DATA);
 	}
       break;
+    case movsxd_mode:
+      if (!(sizeflag & DFLAG) && isa64 == intel64)
+	names = names16;
+      else
+	names = names32;
+      used_prefixes |= (prefixes & PREFIX_DATA);
+      break;
     case va_mode:
       names = (address_mode == mode_64bit
 	       ? names64 : names32);
@@ -14492,12 +14508,14 @@ OP_G (int bytemode, int sizeflag)
     case dqb_mode:
     case dqd_mode:
     case dqw_mode:
+    case movsxd_mode:
       USED_REX (REX_W);
       if (rex & REX_W)
 	oappend (names64[modrm.reg + add]);
       else
 	{
-	  if ((sizeflag & DFLAG) || bytemode != v_mode)
+	  if ((sizeflag & DFLAG)
+	      || (bytemode != v_mode && bytemode != movsxd_mode))
 	    oappend (names32[modrm.reg + add]);
 	  else
 	    oappend (names16[modrm.reg + add]);
@@ -16563,6 +16581,45 @@ skip:
   OP_M (bytemode, sizeflag);
 }
 
+static void
+MOVSXD_Fixup (int bytemode, int sizeflag)
+{
+  /* Add proper suffix to "movsxd".  */
+  char *p = mnemonicendp;
+
+  switch (bytemode)
+    {
+    case movsxd_mode:
+      if (intel_syntax)
+	{
+	  *p++ = 'x';
+	  *p++ = 'd';
+	  goto skip;
+	}
+
+      USED_REX (REX_W);
+      if (rex & REX_W)
+	{
+	  *p++ = 'l';
+	  *p++ = 'q';
+	}
+      else
+	{
+	  *p++ = 'x';
+	  *p++ = 'd';
+	}
+      break;
+    default:
+      oappend (INTERNAL_DISASSEMBLER_ERROR);
+      break;
+    }
+
+skip:
+  mnemonicendp = p;
+  *p = '\0';
+  OP_E (bytemode, sizeflag);
+}
+
 static void
 OP_LWPCB_E (int bytemode ATTRIBUTE_UNUSED, int sizeflag ATTRIBUTE_UNUSED)
 {
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 2acb76bfa15..19793fdcd45 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -135,7 +135,9 @@ movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|R
 movsx, 2, 0xfbe, None, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg8|Byte|BaseIndex, Reg16|Reg32|Reg64 }
 movsx, 2, 0xfbf, None, 2, Cpu386, Modrm|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IntelSyntax, { Reg16|Word|BaseIndex, Reg32|Reg64 }
 movsx, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64|IntelSyntax, { Reg32|Dword|BaseIndex, Reg64 }
-movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg32|Dword|Unspecified|BaseIndex, Reg64 }
+movsxd, 2, 0x63, None, 1, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
+movsxd, 2, 0x63, None, 1, Cpu64, AMD64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg16 }
+movsxd, 2, 0x63, None, 1, Cpu64, Intel64|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Unspecified|BaseIndex, Reg16 }
 
 // Move with zero extend.
 movzb, 2, 0xfb6, None, 2, Cpu386, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index c8fa7e95a00..d1a6c0915a9 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -435,12 +435,40 @@ 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, 1, 0, 0 } },
-    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 1, 1, 1, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0,
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
     { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 0,
 	  0, 0, 0, 0, 1, 0 } },
-      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1,
+	  0, 0, 0, 0, 0, 0 } } } },
+  { "movsxd", 0x63, None, 1, 2,
+    { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0 } },
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0 },
+    { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 0,
+	  0, 0, 0, 0, 1, 0 } },
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
+	  0, 0, 0, 0, 0, 0 } } } },
+  { "movsxd", 0x63, None, 1, 2,
+    { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0 } },
+    { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 },
+    { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 1, 0, 1, 0, 0, 0,
+	  0, 0, 0, 0, 1, 0 } },
+      { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
 	  0, 0, 0, 0, 0, 0 } } } },
   { "movzb", 0xfb6, None, 2, 2,
     { { 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-- 
2.24.1


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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-24 11:52   ` H.J. Lu
  2020-01-26 15:01     ` H.J. Lu
@ 2020-01-27 16:17     ` Jan Beulich
  2020-01-27 16:29       ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-01-27 16:17 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 24.01.2020 12:51, H.J. Lu wrote:
> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 23.01.2020 21:24, H.J. Lu wrote:
>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
>>> destination registers.  Its AT&T mnemonic is movslq which only supports
>>> 64-bit destination register.  There is also a discrepancy between AMD64
>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
>>> 32-bit source operand and Intel64 supports 16-bit source operand.
>>>
>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
>>> destination registers.  It also handles movsxd with 16-bit destination
>>> register for AMD64 and Intel 64.
>>
>> I appreciate you taking care of the disassembler and documentation
>> side, but would you mind pointing out what's wrong with the previously
>> posted patches making the assembler deal with this correctly? For
>> example, I get away ...
>>
>>> --- a/gas/config/tc-i386.c
>>> +++ b/gas/config/tc-i386.c
>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
>>>            && i.tm.operand_types[op].bitfield.dword)
>>>        {
>>>       if (intel_syntax
>>> -         && i.tm.opcode_modifier.toqword
>>> +         && (i.tm.opcode_modifier.toqword
>>> +             /* Also convert to QWORD for MOVSXD.  */
>>> +             || i.tm.base_opcode == 0x63)
>>>           && i.types[0].bitfield.class != RegSIMD)
>>
>> ... without such a (pretty arbitrary) adjustment (as an aside: the
>> comment is slightly misleading in that the change also affects some
>> MOVSX templates), while ...
> 
> My patch in assembler
> 
> 1.  Keep movslq unchanged, which only allows 64-bit destination register.

None of the patches I've posted so far has touched movslq.

> 2.  Allow DWORD source with movsxd.

I guess you mean WORD, for Intel64 mode. Which again matches what
my patch does.

> My testcases cover the above.  Without the assembler change, I got
> 
> FAIL: x86-64 movsxd (AMD64)
> FAIL: x86-64 movsxd (AMD64) (Intel mode)
> FAIL: x86-64 movsxd (Intel64)
> FAIL: x86-64 movsxd (Intel64) (Intel mode)

My patch also adds tests which would have failed before. There are two
problems I see here:
1) Why, instead of reviewing my patch (using less arbitrary special
casing), did you feel the need of creating your own version? (As said
before, I'm glad you did the disassembler side change, so thanks for
that part.)
2) Wouldn't it have been fair to wait with committing your change
until we've settled this discussion? I'll rebase my series on top,
sure, but the way you've done it will merely make me want undo your
assembler change and put in my variant of it. This could have been
had in one single step. And do you realize that it's a change to
Intel syntax behavior only, of which strictly speaking I'm the
maintainer?

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-27 16:17     ` Jan Beulich
@ 2020-01-27 16:29       ` H.J. Lu
  0 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2020-01-27 16:29 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Jan 27, 2020 at 8:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.01.2020 12:51, H.J. Lu wrote:
> > On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 23.01.2020 21:24, H.J. Lu wrote:
> >>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> >>> destination registers.  Its AT&T mnemonic is movslq which only supports
> >>> 64-bit destination register.  There is also a discrepancy between AMD64
> >>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> >>> 32-bit source operand and Intel64 supports 16-bit source operand.
> >>>
> >>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> >>> destination registers.  It also handles movsxd with 16-bit destination
> >>> register for AMD64 and Intel 64.
> >>
> >> I appreciate you taking care of the disassembler and documentation
> >> side, but would you mind pointing out what's wrong with the previously
> >> posted patches making the assembler deal with this correctly? For
> >> example, I get away ...
> >>
> >>> --- a/gas/config/tc-i386.c
> >>> +++ b/gas/config/tc-i386.c
> >>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
> >>>            && i.tm.operand_types[op].bitfield.dword)
> >>>        {
> >>>       if (intel_syntax
> >>> -         && i.tm.opcode_modifier.toqword
> >>> +         && (i.tm.opcode_modifier.toqword
> >>> +             /* Also convert to QWORD for MOVSXD.  */
> >>> +             || i.tm.base_opcode == 0x63)
> >>>           && i.types[0].bitfield.class != RegSIMD)
> >>
> >> ... without such a (pretty arbitrary) adjustment (as an aside: the
> >> comment is slightly misleading in that the change also affects some
> >> MOVSX templates), while ...
> >
> > My patch in assembler
> >
> > 1.  Keep movslq unchanged, which only allows 64-bit destination register.
>
> None of the patches I've posted so far has touched movslq.
>
> > 2.  Allow DWORD source with movsxd.
>
> I guess you mean WORD, for Intel64 mode. Which again matches what
> my patch does.

I meant DWORD for AMD64.

> > My testcases cover the above.  Without the assembler change, I got
> >
> > FAIL: x86-64 movsxd (AMD64)
> > FAIL: x86-64 movsxd (AMD64) (Intel mode)
> > FAIL: x86-64 movsxd (Intel64)
> > FAIL: x86-64 movsxd (Intel64) (Intel mode)
>
> My patch also adds tests which would have failed before. There are two
> problems I see here:
> 1) Why, instead of reviewing my patch (using less arbitrary special
> casing), did you feel the need of creating your own version? (As said

Your patches check opcode.

> before, I'm glad you did the disassembler side change, so thanks for
> that part.)
> 2) Wouldn't it have been fair to wait with committing your change
> until we've settled this discussion? I'll rebase my series on top,
> sure, but the way you've done it will merely make me want undo your
> assembler change and put in my variant of it. This could have been
> had in one single step. And do you realize that it's a change to
> Intel syntax behavior only, of which strictly speaking I'm the
> maintainer?

Please submit a single combined patch to address:

https://sourceware.org/bugzilla/show_bug.cgi?id=25438

-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-26 15:01     ` H.J. Lu
@ 2020-02-03  8:38       ` Jan Beulich
  2020-02-03 14:50         ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-03  8:38 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 26.01.2020 16:00, H.J. Lu wrote:
> On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 23.01.2020 21:24, H.J. Lu wrote:
>>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
>>>> destination registers.  Its AT&T mnemonic is movslq which only supports
>>>> 64-bit destination register.  There is also a discrepancy between AMD64
>>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
>>>> 32-bit source operand and Intel64 supports 16-bit source operand.
>>>>
>>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
>>>> destination registers.  It also handles movsxd with 16-bit destination
>>>> register for AMD64 and Intel 64.
>>>
>>> I appreciate you taking care of the disassembler and documentation
>>> side, but would you mind pointing out what's wrong with the previously
>>> posted patches making the assembler deal with this correctly? For
>>> example, I get away ...
>>>
>>>> --- a/gas/config/tc-i386.c
>>>> +++ b/gas/config/tc-i386.c
>>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
>>>>            && i.tm.operand_types[op].bitfield.dword)
>>>>        {
>>>>       if (intel_syntax
>>>> -         && i.tm.opcode_modifier.toqword
>>>> +         && (i.tm.opcode_modifier.toqword
>>>> +             /* Also convert to QWORD for MOVSXD.  */
>>>> +             || i.tm.base_opcode == 0x63)
>>>>           && i.types[0].bitfield.class != RegSIMD)
>>>
>>> ... without such a (pretty arbitrary) adjustment (as an aside: the
>>> comment is slightly misleading in that the change also affects some
>>> MOVSX templates), while ...
>>
>> My patch in assembler
>>
>> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
>> 2.  Allow DWORD source with movsxd.
>>
>> My testcases cover the above.  Without the assembler change, I got
>>
>> FAIL: x86-64 movsxd (AMD64)
>> FAIL: x86-64 movsxd (AMD64) (Intel mode)
>> FAIL: x86-64 movsxd (Intel64)
>> FAIL: x86-64 movsxd (Intel64) (Intel mode)
>>
> 
> I am checking in this patch tomorrow.

And you broke previously working code, which has no testcase so
far, but which
https://sourceware.org/ml/binutils/2019-12/msg00354.html
adds a test for:

movsxdl (%rax),%rcx

This is because you added No_lSuf to the existing template.
Obviously the No_wSuf / No_lSuf present on the two new templates
then are similarly wrong. Just like for MOVSX/MOVZX in AT&T mode
a suffix should be permitted imo (even if there's no ambiguity
here), specifying the size of the source (memory) operand. An
alternative would be to make MOVSX/MOVZX strictly Intel syntax
mode only, but I guess there would then be a fair risk of
breaking existing code.

I need to know which direction to go in order to sensibly rebase
that patch as well as the other remaining ones from that series.

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-03  8:38       ` Jan Beulich
@ 2020-02-03 14:50         ` H.J. Lu
  2020-02-03 16:34           ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-02-03 14:50 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.01.2020 16:00, H.J. Lu wrote:
> > On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 23.01.2020 21:24, H.J. Lu wrote:
> >>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> >>>> destination registers.  Its AT&T mnemonic is movslq which only supports
> >>>> 64-bit destination register.  There is also a discrepancy between AMD64
> >>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> >>>> 32-bit source operand and Intel64 supports 16-bit source operand.
> >>>>
> >>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> >>>> destination registers.  It also handles movsxd with 16-bit destination
> >>>> register for AMD64 and Intel 64.
> >>>
> >>> I appreciate you taking care of the disassembler and documentation
> >>> side, but would you mind pointing out what's wrong with the previously
> >>> posted patches making the assembler deal with this correctly? For
> >>> example, I get away ...
> >>>
> >>>> --- a/gas/config/tc-i386.c
> >>>> +++ b/gas/config/tc-i386.c
> >>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
> >>>>            && i.tm.operand_types[op].bitfield.dword)
> >>>>        {
> >>>>       if (intel_syntax
> >>>> -         && i.tm.opcode_modifier.toqword
> >>>> +         && (i.tm.opcode_modifier.toqword
> >>>> +             /* Also convert to QWORD for MOVSXD.  */
> >>>> +             || i.tm.base_opcode == 0x63)
> >>>>           && i.types[0].bitfield.class != RegSIMD)
> >>>
> >>> ... without such a (pretty arbitrary) adjustment (as an aside: the
> >>> comment is slightly misleading in that the change also affects some
> >>> MOVSX templates), while ...
> >>
> >> My patch in assembler
> >>
> >> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
> >> 2.  Allow DWORD source with movsxd.
> >>
> >> My testcases cover the above.  Without the assembler change, I got
> >>
> >> FAIL: x86-64 movsxd (AMD64)
> >> FAIL: x86-64 movsxd (AMD64) (Intel mode)
> >> FAIL: x86-64 movsxd (Intel64)
> >> FAIL: x86-64 movsxd (Intel64) (Intel mode)
> >>
> >
> > I am checking in this patch tomorrow.
>
> And you broke previously working code, which has no testcase so
> far, but which
> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> adds a test for:
>
> movsxdl (%rax),%rcx

This isn't valid AT&T mnemonic.  No one should use it.

> This is because you added No_lSuf to the existing template.
> Obviously the No_wSuf / No_lSuf present on the two new templates
> then are similarly wrong. Just like for MOVSX/MOVZX in AT&T mode
> a suffix should be permitted imo (even if there's no ambiguity
> here), specifying the size of the source (memory) operand. An
> alternative would be to make MOVSX/MOVZX strictly Intel syntax
> mode only, but I guess there would then be a fair risk of
> breaking existing code.
>
> I need to know which direction to go in order to sensibly rebase
> that patch as well as the other remaining ones from that series.
>
> Jan
>


-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-03 14:50         ` H.J. Lu
@ 2020-02-03 16:34           ` Jan Beulich
  2020-02-03 17:19             ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-03 16:34 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 03.02.2020 15:49, H.J. Lu wrote:
> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.01.2020 16:00, H.J. Lu wrote:
>>> On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>
>>>> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>
>>>>> On 23.01.2020 21:24, H.J. Lu wrote:
>>>>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
>>>>>> destination registers.  Its AT&T mnemonic is movslq which only supports
>>>>>> 64-bit destination register.  There is also a discrepancy between AMD64
>>>>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
>>>>>> 32-bit source operand and Intel64 supports 16-bit source operand.
>>>>>>
>>>>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
>>>>>> destination registers.  It also handles movsxd with 16-bit destination
>>>>>> register for AMD64 and Intel 64.
>>>>>
>>>>> I appreciate you taking care of the disassembler and documentation
>>>>> side, but would you mind pointing out what's wrong with the previously
>>>>> posted patches making the assembler deal with this correctly? For
>>>>> example, I get away ...
>>>>>
>>>>>> --- a/gas/config/tc-i386.c
>>>>>> +++ b/gas/config/tc-i386.c
>>>>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
>>>>>>            && i.tm.operand_types[op].bitfield.dword)
>>>>>>        {
>>>>>>       if (intel_syntax
>>>>>> -         && i.tm.opcode_modifier.toqword
>>>>>> +         && (i.tm.opcode_modifier.toqword
>>>>>> +             /* Also convert to QWORD for MOVSXD.  */
>>>>>> +             || i.tm.base_opcode == 0x63)
>>>>>>           && i.types[0].bitfield.class != RegSIMD)
>>>>>
>>>>> ... without such a (pretty arbitrary) adjustment (as an aside: the
>>>>> comment is slightly misleading in that the change also affects some
>>>>> MOVSX templates), while ...
>>>>
>>>> My patch in assembler
>>>>
>>>> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
>>>> 2.  Allow DWORD source with movsxd.
>>>>
>>>> My testcases cover the above.  Without the assembler change, I got
>>>>
>>>> FAIL: x86-64 movsxd (AMD64)
>>>> FAIL: x86-64 movsxd (AMD64) (Intel mode)
>>>> FAIL: x86-64 movsxd (Intel64)
>>>> FAIL: x86-64 movsxd (Intel64) (Intel mode)
>>>>
>>>
>>> I am checking in this patch tomorrow.
>>
>> And you broke previously working code, which has no testcase so
>> far, but which
>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>> adds a test for:
>>
>> movsxdl (%rax),%rcx
> 
> This isn't valid AT&T mnemonic.  No one should use it.

Mind telling me what you derive this from? It is my understanding
that the general rule of how to derive AT&T mnemonics is to take
the Intel manual's mnemonic and add a set of suffixes if varying
operand sizes are permitted. I can accept _new_ insns (in
particular SIMD ones) to not get suffixes supported when they're
not really needed. But this is an original x86-64 GPR insn. It
should be consistent with other original x86-64 GPR insns.

Furthermore, if it really was intentional for your commit to
remove support for the suffix, then why does the description not
say so (nor, what's perhaps even worse, the ChangeLog entry)?

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-03 16:34           ` Jan Beulich
@ 2020-02-03 17:19             ` H.J. Lu
  2020-02-05  8:18               ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-02-03 17:19 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 03.02.2020 15:49, H.J. Lu wrote:
> > On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 26.01.2020 16:00, H.J. Lu wrote:
> >>> On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>
> >>>> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>
> >>>>> On 23.01.2020 21:24, H.J. Lu wrote:
> >>>>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
> >>>>>> destination registers.  Its AT&T mnemonic is movslq which only supports
> >>>>>> 64-bit destination register.  There is also a discrepancy between AMD64
> >>>>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
> >>>>>> 32-bit source operand and Intel64 supports 16-bit source operand.
> >>>>>>
> >>>>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
> >>>>>> destination registers.  It also handles movsxd with 16-bit destination
> >>>>>> register for AMD64 and Intel 64.
> >>>>>
> >>>>> I appreciate you taking care of the disassembler and documentation
> >>>>> side, but would you mind pointing out what's wrong with the previously
> >>>>> posted patches making the assembler deal with this correctly? For
> >>>>> example, I get away ...
> >>>>>
> >>>>>> --- a/gas/config/tc-i386.c
> >>>>>> +++ b/gas/config/tc-i386.c
> >>>>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
> >>>>>>            && i.tm.operand_types[op].bitfield.dword)
> >>>>>>        {
> >>>>>>       if (intel_syntax
> >>>>>> -         && i.tm.opcode_modifier.toqword
> >>>>>> +         && (i.tm.opcode_modifier.toqword
> >>>>>> +             /* Also convert to QWORD for MOVSXD.  */
> >>>>>> +             || i.tm.base_opcode == 0x63)
> >>>>>>           && i.types[0].bitfield.class != RegSIMD)
> >>>>>
> >>>>> ... without such a (pretty arbitrary) adjustment (as an aside: the
> >>>>> comment is slightly misleading in that the change also affects some
> >>>>> MOVSX templates), while ...
> >>>>
> >>>> My patch in assembler
> >>>>
> >>>> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
> >>>> 2.  Allow DWORD source with movsxd.
> >>>>
> >>>> My testcases cover the above.  Without the assembler change, I got
> >>>>
> >>>> FAIL: x86-64 movsxd (AMD64)
> >>>> FAIL: x86-64 movsxd (AMD64) (Intel mode)
> >>>> FAIL: x86-64 movsxd (Intel64)
> >>>> FAIL: x86-64 movsxd (Intel64) (Intel mode)
> >>>>
> >>>
> >>> I am checking in this patch tomorrow.
> >>
> >> And you broke previously working code, which has no testcase so
> >> far, but which
> >> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> >> adds a test for:
> >>
> >> movsxdl (%rax),%rcx
> >
> > This isn't valid AT&T mnemonic.  No one should use it.
>
> Mind telling me what you derive this from? It is my understanding
> that the general rule of how to derive AT&T mnemonics is to take
> the Intel manual's mnemonic and add a set of suffixes if varying

MOVSXD is a special case.  The typical usages of AT&T syntax are

movslq %eax, %rcx
movslq (%rax), %rcx

Anything else should be MOVSXD without any suffixes.  It is one
less mnemonic we need to apply complex suffix rules.

> operand sizes are permitted. I can accept _new_ insns (in
> particular SIMD ones) to not get suffixes supported when they're
> not really needed. But this is an original x86-64 GPR insn. It
> should be consistent with other original x86-64 GPR insns.
>
> Furthermore, if it really was intentional for your commit to

Yes, it was intentional.

> remove support for the suffix, then why does the description not
> say so (nor, what's perhaps even worse, the ChangeLog entry)?
>

Sorry I missed it.

-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-03 17:19             ` H.J. Lu
@ 2020-02-05  8:18               ` Jan Beulich
  2020-02-05 12:44                 ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-05  8:18 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 03.02.2020 18:19, H.J. Lu wrote:
> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 03.02.2020 15:49, H.J. Lu wrote:
>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> And you broke previously working code, which has no testcase so
>>>> far, but which
>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>>>> adds a test for:
>>>>
>>>> movsxdl (%rax),%rcx
>>>
>>> This isn't valid AT&T mnemonic.  No one should use it.
>>
>> Mind telling me what you derive this from? It is my understanding
>> that the general rule of how to derive AT&T mnemonics is to take
>> the Intel manual's mnemonic and add a set of suffixes if varying
> 
> MOVSXD is a special case.  The typical usages of AT&T syntax are
> 
> movslq %eax, %rcx
> movslq (%rax), %rcx
> 
> Anything else should be MOVSXD without any suffixes.  It is one
> less mnemonic we need to apply complex suffix rules.
> 
>> operand sizes are permitted. I can accept _new_ insns (in
>> particular SIMD ones) to not get suffixes supported when they're
>> not really needed. But this is an original x86-64 GPR insn. It
>> should be consistent with other original x86-64 GPR insns.
>>
>> Furthermore, if it really was intentional for your commit to
> 
> Yes, it was intentional.

Well, okay then, I'll remove the addition from the patch. But
I guess you realize that consistency here would mean to not
allow MOVSXD at all in AT&T mode (or at least to not "prefer"
it, just like for MOVSX), but instead have suitable
alternative mnemonics: MOVSLL and (AMD64 only) MOVSLW (no
matter how odd they may look).

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05  8:18               ` Jan Beulich
@ 2020-02-05 12:44                 ` H.J. Lu
  2020-02-05 12:48                   ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-02-05 12:44 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 03.02.2020 18:19, H.J. Lu wrote:
> > On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 03.02.2020 15:49, H.J. Lu wrote:
> >>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> And you broke previously working code, which has no testcase so
> >>>> far, but which
> >>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> >>>> adds a test for:
> >>>>
> >>>> movsxdl (%rax),%rcx
> >>>
> >>> This isn't valid AT&T mnemonic.  No one should use it.
> >>
> >> Mind telling me what you derive this from? It is my understanding
> >> that the general rule of how to derive AT&T mnemonics is to take
> >> the Intel manual's mnemonic and add a set of suffixes if varying
> >
> > MOVSXD is a special case.  The typical usages of AT&T syntax are
> >
> > movslq %eax, %rcx
> > movslq (%rax), %rcx
> >
> > Anything else should be MOVSXD without any suffixes.  It is one
> > less mnemonic we need to apply complex suffix rules.
> >
> >> operand sizes are permitted. I can accept _new_ insns (in
> >> particular SIMD ones) to not get suffixes supported when they're
> >> not really needed. But this is an original x86-64 GPR insn. It
> >> should be consistent with other original x86-64 GPR insns.
> >>
> >> Furthermore, if it really was intentional for your commit to
> >
> > Yes, it was intentional.
>
> Well, okay then, I'll remove the addition from the patch. But
> I guess you realize that consistency here would mean to not
> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
> it, just like for MOVSX), but instead have suitable

[hjl@gnu-cfl-2 tmp]$ cat x.s
movslq %eax, %rcx
movslq (%rax), %rcx
movsxd %eax, %rcx
movsxd (%rax), %rcx
movsxd %eax, %ecx
movsxd (%rax), %ecx
movsxd %eax, %cx
movsxd (%rax), %cx
[hjl@gnu-cfl-2 tmp]$ gcc -c x.s
[hjl@gnu-cfl-2 tmp]$ objdump -dw x.o

x.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <.text>:
   0: 48 63 c8              movslq %eax,%rcx
   3: 48 63 08              movslq (%rax),%rcx
   6: 48 63 c8              movslq %eax,%rcx
   9: 48 63 08              movslq (%rax),%rcx
   c: 63 c8                movsxd %eax,%ecx
   e: 63 08                movsxd (%rax),%ecx
  10: 66 63 c8              movsxd %eax,%cx
  13: 66 63 08              movsxd (%rax),%cx
[hjl@gnu-cfl-2 tmp]$

> alternative mnemonics: MOVSLL and (AMD64 only) MOVSLW (no
> matter how odd they may look).
>
> Jan



-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 12:44                 ` H.J. Lu
@ 2020-02-05 12:48                   ` Jan Beulich
  2020-02-05 13:03                     ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-05 12:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 05.02.2020 13:44, H.J. Lu wrote:
> On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 03.02.2020 18:19, H.J. Lu wrote:
>>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 03.02.2020 15:49, H.J. Lu wrote:
>>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> And you broke previously working code, which has no testcase so
>>>>>> far, but which
>>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>>>>>> adds a test for:
>>>>>>
>>>>>> movsxdl (%rax),%rcx
>>>>>
>>>>> This isn't valid AT&T mnemonic.  No one should use it.
>>>>
>>>> Mind telling me what you derive this from? It is my understanding
>>>> that the general rule of how to derive AT&T mnemonics is to take
>>>> the Intel manual's mnemonic and add a set of suffixes if varying
>>>
>>> MOVSXD is a special case.  The typical usages of AT&T syntax are
>>>
>>> movslq %eax, %rcx
>>> movslq (%rax), %rcx
>>>
>>> Anything else should be MOVSXD without any suffixes.  It is one
>>> less mnemonic we need to apply complex suffix rules.
>>>
>>>> operand sizes are permitted. I can accept _new_ insns (in
>>>> particular SIMD ones) to not get suffixes supported when they're
>>>> not really needed. But this is an original x86-64 GPR insn. It
>>>> should be consistent with other original x86-64 GPR insns.
>>>>
>>>> Furthermore, if it really was intentional for your commit to
>>>
>>> Yes, it was intentional.
>>
>> Well, okay then, I'll remove the addition from the patch. But
>> I guess you realize that consistency here would mean to not
>> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
>> it, just like for MOVSX), but instead have suitable
> 
> [hjl@gnu-cfl-2 tmp]$ cat x.s
> movslq %eax, %rcx
> movslq (%rax), %rcx
> movsxd %eax, %rcx
> movsxd (%rax), %rcx
> movsxd %eax, %ecx
> movsxd (%rax), %ecx
> movsxd %eax, %cx
> movsxd (%rax), %cx
> [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
> [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
> 
> x.o:     file format elf64-x86-64
> 
> 
> Disassembly of section .text:
> 
> 0000000000000000 <.text>:
>    0: 48 63 c8              movslq %eax,%rcx
>    3: 48 63 08              movslq (%rax),%rcx
>    6: 48 63 c8              movslq %eax,%rcx
>    9: 48 63 08              movslq (%rax),%rcx
>    c: 63 c8                movsxd %eax,%ecx
>    e: 63 08                movsxd (%rax),%ecx
>   10: 66 63 c8              movsxd %eax,%cx
>   13: 66 63 08              movsxd (%rax),%cx
> [hjl@gnu-cfl-2 tmp]$

I'm sorry, but I have no idea what you're trying to tell or
demonstrate me with this. Things being the way you show them
right now does in no way mean this is how they should be.

Jan

>> alternative mnemonics: MOVSLL and (AMD64 only) MOVSLW (no
>> matter how odd they may look).
>>
>> Jan
> 
> 
> 

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 12:48                   ` Jan Beulich
@ 2020-02-05 13:03                     ` H.J. Lu
  2020-02-05 13:14                       ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-02-05 13:03 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Feb 5, 2020 at 4:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.02.2020 13:44, H.J. Lu wrote:
> > On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 03.02.2020 18:19, H.J. Lu wrote:
> >>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 03.02.2020 15:49, H.J. Lu wrote:
> >>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> And you broke previously working code, which has no testcase so
> >>>>>> far, but which
> >>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> >>>>>> adds a test for:
> >>>>>>
> >>>>>> movsxdl (%rax),%rcx
> >>>>>
> >>>>> This isn't valid AT&T mnemonic.  No one should use it.
> >>>>
> >>>> Mind telling me what you derive this from? It is my understanding
> >>>> that the general rule of how to derive AT&T mnemonics is to take
> >>>> the Intel manual's mnemonic and add a set of suffixes if varying
> >>>
> >>> MOVSXD is a special case.  The typical usages of AT&T syntax are
> >>>
> >>> movslq %eax, %rcx
> >>> movslq (%rax), %rcx
> >>>
> >>> Anything else should be MOVSXD without any suffixes.  It is one
> >>> less mnemonic we need to apply complex suffix rules.
> >>>
> >>>> operand sizes are permitted. I can accept _new_ insns (in
> >>>> particular SIMD ones) to not get suffixes supported when they're
> >>>> not really needed. But this is an original x86-64 GPR insn. It
> >>>> should be consistent with other original x86-64 GPR insns.
> >>>>
> >>>> Furthermore, if it really was intentional for your commit to
> >>>
> >>> Yes, it was intentional.
> >>
> >> Well, okay then, I'll remove the addition from the patch. But
> >> I guess you realize that consistency here would mean to not
> >> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
> >> it, just like for MOVSX), but instead have suitable
> >
> > [hjl@gnu-cfl-2 tmp]$ cat x.s
> > movslq %eax, %rcx
> > movslq (%rax), %rcx
> > movsxd %eax, %rcx
> > movsxd (%rax), %rcx
> > movsxd %eax, %ecx
> > movsxd (%rax), %ecx
> > movsxd %eax, %cx
> > movsxd (%rax), %cx
> > [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
> > [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
> >
> > x.o:     file format elf64-x86-64
> >
> >
> > Disassembly of section .text:
> >
> > 0000000000000000 <.text>:
> >    0: 48 63 c8              movslq %eax,%rcx
> >    3: 48 63 08              movslq (%rax),%rcx
> >    6: 48 63 c8              movslq %eax,%rcx
> >    9: 48 63 08              movslq (%rax),%rcx
> >    c: 63 c8                movsxd %eax,%ecx
> >    e: 63 08                movsxd (%rax),%ecx
> >   10: 66 63 c8              movsxd %eax,%cx
> >   13: 66 63 08              movsxd (%rax),%cx
> > [hjl@gnu-cfl-2 tmp]$
>
> I'm sorry, but I have no idea what you're trying to tell or
> demonstrate me with this. Things being the way you show them
> right now does in no way mean this is how they should be.

  * 'movslq' with AT&T mnemonic only accepts 64-bit destination
     register.  'movsxd' should be used to encode 16-bit or 32-bit
     destination register with both AT&T and Intel mnemonics.

> Jan
>
> >> alternative mnemonics: MOVSLL and (AMD64 only) MOVSLW (no
> >> matter how odd they may look).
> >>
> >> Jan
> >
> >
> >
>


-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 13:03                     ` H.J. Lu
@ 2020-02-05 13:14                       ` Jan Beulich
  2020-02-05 13:18                         ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-05 13:14 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 05.02.2020 14:03, H.J. Lu wrote:
> On Wed, Feb 5, 2020 at 4:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.02.2020 13:44, H.J. Lu wrote:
>>> On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 03.02.2020 18:19, H.J. Lu wrote:
>>>>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 03.02.2020 15:49, H.J. Lu wrote:
>>>>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> And you broke previously working code, which has no testcase so
>>>>>>>> far, but which
>>>>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>>>>>>>> adds a test for:
>>>>>>>>
>>>>>>>> movsxdl (%rax),%rcx
>>>>>>>
>>>>>>> This isn't valid AT&T mnemonic.  No one should use it.
>>>>>>
>>>>>> Mind telling me what you derive this from? It is my understanding
>>>>>> that the general rule of how to derive AT&T mnemonics is to take
>>>>>> the Intel manual's mnemonic and add a set of suffixes if varying
>>>>>
>>>>> MOVSXD is a special case.  The typical usages of AT&T syntax are
>>>>>
>>>>> movslq %eax, %rcx
>>>>> movslq (%rax), %rcx
>>>>>
>>>>> Anything else should be MOVSXD without any suffixes.  It is one
>>>>> less mnemonic we need to apply complex suffix rules.
>>>>>
>>>>>> operand sizes are permitted. I can accept _new_ insns (in
>>>>>> particular SIMD ones) to not get suffixes supported when they're
>>>>>> not really needed. But this is an original x86-64 GPR insn. It
>>>>>> should be consistent with other original x86-64 GPR insns.
>>>>>>
>>>>>> Furthermore, if it really was intentional for your commit to
>>>>>
>>>>> Yes, it was intentional.
>>>>
>>>> Well, okay then, I'll remove the addition from the patch. But
>>>> I guess you realize that consistency here would mean to not
>>>> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
>>>> it, just like for MOVSX), but instead have suitable
>>>
>>> [hjl@gnu-cfl-2 tmp]$ cat x.s
>>> movslq %eax, %rcx
>>> movslq (%rax), %rcx
>>> movsxd %eax, %rcx
>>> movsxd (%rax), %rcx
>>> movsxd %eax, %ecx
>>> movsxd (%rax), %ecx
>>> movsxd %eax, %cx
>>> movsxd (%rax), %cx
>>> [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
>>> [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
>>>
>>> x.o:     file format elf64-x86-64
>>>
>>>
>>> Disassembly of section .text:
>>>
>>> 0000000000000000 <.text>:
>>>    0: 48 63 c8              movslq %eax,%rcx
>>>    3: 48 63 08              movslq (%rax),%rcx
>>>    6: 48 63 c8              movslq %eax,%rcx
>>>    9: 48 63 08              movslq (%rax),%rcx
>>>    c: 63 c8                movsxd %eax,%ecx
>>>    e: 63 08                movsxd (%rax),%ecx
>>>   10: 66 63 c8              movsxd %eax,%cx
>>>   13: 66 63 08              movsxd (%rax),%cx
>>> [hjl@gnu-cfl-2 tmp]$
>>
>> I'm sorry, but I have no idea what you're trying to tell or
>> demonstrate me with this. Things being the way you show them
>> right now does in no way mean this is how they should be.
> 
>   * 'movslq' with AT&T mnemonic only accepts 64-bit destination
>      register.  'movsxd' should be used to encode 16-bit or 32-bit
>      destination register with both AT&T and Intel mnemonics.

I think you've said exactly this before. I don't think, however,
that this addresses in any way my most recent remark regarding
the inconsistency of this.

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 13:14                       ` Jan Beulich
@ 2020-02-05 13:18                         ` H.J. Lu
  2020-02-05 13:23                           ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2020-02-05 13:18 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Feb 5, 2020 at 5:14 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.02.2020 14:03, H.J. Lu wrote:
> > On Wed, Feb 5, 2020 at 4:48 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 05.02.2020 13:44, H.J. Lu wrote:
> >>> On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 03.02.2020 18:19, H.J. Lu wrote:
> >>>>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> On 03.02.2020 15:49, H.J. Lu wrote:
> >>>>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>> And you broke previously working code, which has no testcase so
> >>>>>>>> far, but which
> >>>>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> >>>>>>>> adds a test for:
> >>>>>>>>
> >>>>>>>> movsxdl (%rax),%rcx
> >>>>>>>
> >>>>>>> This isn't valid AT&T mnemonic.  No one should use it.
> >>>>>>
> >>>>>> Mind telling me what you derive this from? It is my understanding
> >>>>>> that the general rule of how to derive AT&T mnemonics is to take
> >>>>>> the Intel manual's mnemonic and add a set of suffixes if varying
> >>>>>
> >>>>> MOVSXD is a special case.  The typical usages of AT&T syntax are
> >>>>>
> >>>>> movslq %eax, %rcx
> >>>>> movslq (%rax), %rcx
> >>>>>
> >>>>> Anything else should be MOVSXD without any suffixes.  It is one
> >>>>> less mnemonic we need to apply complex suffix rules.
> >>>>>
> >>>>>> operand sizes are permitted. I can accept _new_ insns (in
> >>>>>> particular SIMD ones) to not get suffixes supported when they're
> >>>>>> not really needed. But this is an original x86-64 GPR insn. It
> >>>>>> should be consistent with other original x86-64 GPR insns.
> >>>>>>
> >>>>>> Furthermore, if it really was intentional for your commit to
> >>>>>
> >>>>> Yes, it was intentional.
> >>>>
> >>>> Well, okay then, I'll remove the addition from the patch. But
> >>>> I guess you realize that consistency here would mean to not
> >>>> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
> >>>> it, just like for MOVSX), but instead have suitable
> >>>
> >>> [hjl@gnu-cfl-2 tmp]$ cat x.s
> >>> movslq %eax, %rcx
> >>> movslq (%rax), %rcx
> >>> movsxd %eax, %rcx
> >>> movsxd (%rax), %rcx
> >>> movsxd %eax, %ecx
> >>> movsxd (%rax), %ecx
> >>> movsxd %eax, %cx
> >>> movsxd (%rax), %cx
> >>> [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
> >>> [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
> >>>
> >>> x.o:     file format elf64-x86-64
> >>>
> >>>
> >>> Disassembly of section .text:
> >>>
> >>> 0000000000000000 <.text>:
> >>>    0: 48 63 c8              movslq %eax,%rcx
> >>>    3: 48 63 08              movslq (%rax),%rcx
> >>>    6: 48 63 c8              movslq %eax,%rcx
> >>>    9: 48 63 08              movslq (%rax),%rcx
> >>>    c: 63 c8                movsxd %eax,%ecx
> >>>    e: 63 08                movsxd (%rax),%ecx
> >>>   10: 66 63 c8              movsxd %eax,%cx
> >>>   13: 66 63 08              movsxd (%rax),%cx
> >>> [hjl@gnu-cfl-2 tmp]$
> >>
> >> I'm sorry, but I have no idea what you're trying to tell or
> >> demonstrate me with this. Things being the way you show them
> >> right now does in no way mean this is how they should be.
> >
> >   * 'movslq' with AT&T mnemonic only accepts 64-bit destination
> >      register.  'movsxd' should be used to encode 16-bit or 32-bit
> >      destination register with both AT&T and Intel mnemonics.
>
> I think you've said exactly this before. I don't think, however,
> that this addresses in any way my most recent remark regarding
> the inconsistency of this.
>

Some inconsistencies are unavoidable.  MOVSXD is one of them.

-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 13:18                         ` H.J. Lu
@ 2020-02-05 13:23                           ` Jan Beulich
  2020-02-05 13:36                             ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2020-02-05 13:23 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 05.02.2020 14:17, H.J. Lu wrote:
> On Wed, Feb 5, 2020 at 5:14 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.02.2020 14:03, H.J. Lu wrote:
>>> On Wed, Feb 5, 2020 at 4:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 05.02.2020 13:44, H.J. Lu wrote:
>>>>> On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 03.02.2020 18:19, H.J. Lu wrote:
>>>>>>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> On 03.02.2020 15:49, H.J. Lu wrote:
>>>>>>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>> And you broke previously working code, which has no testcase so
>>>>>>>>>> far, but which
>>>>>>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>>>>>>>>>> adds a test for:
>>>>>>>>>>
>>>>>>>>>> movsxdl (%rax),%rcx
>>>>>>>>>
>>>>>>>>> This isn't valid AT&T mnemonic.  No one should use it.
>>>>>>>>
>>>>>>>> Mind telling me what you derive this from? It is my understanding
>>>>>>>> that the general rule of how to derive AT&T mnemonics is to take
>>>>>>>> the Intel manual's mnemonic and add a set of suffixes if varying
>>>>>>>
>>>>>>> MOVSXD is a special case.  The typical usages of AT&T syntax are
>>>>>>>
>>>>>>> movslq %eax, %rcx
>>>>>>> movslq (%rax), %rcx
>>>>>>>
>>>>>>> Anything else should be MOVSXD without any suffixes.  It is one
>>>>>>> less mnemonic we need to apply complex suffix rules.
>>>>>>>
>>>>>>>> operand sizes are permitted. I can accept _new_ insns (in
>>>>>>>> particular SIMD ones) to not get suffixes supported when they're
>>>>>>>> not really needed. But this is an original x86-64 GPR insn. It
>>>>>>>> should be consistent with other original x86-64 GPR insns.
>>>>>>>>
>>>>>>>> Furthermore, if it really was intentional for your commit to
>>>>>>>
>>>>>>> Yes, it was intentional.
>>>>>>
>>>>>> Well, okay then, I'll remove the addition from the patch. But
>>>>>> I guess you realize that consistency here would mean to not
>>>>>> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
>>>>>> it, just like for MOVSX), but instead have suitable
>>>>>
>>>>> [hjl@gnu-cfl-2 tmp]$ cat x.s
>>>>> movslq %eax, %rcx
>>>>> movslq (%rax), %rcx
>>>>> movsxd %eax, %rcx
>>>>> movsxd (%rax), %rcx
>>>>> movsxd %eax, %ecx
>>>>> movsxd (%rax), %ecx
>>>>> movsxd %eax, %cx
>>>>> movsxd (%rax), %cx
>>>>> [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
>>>>> [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
>>>>>
>>>>> x.o:     file format elf64-x86-64
>>>>>
>>>>>
>>>>> Disassembly of section .text:
>>>>>
>>>>> 0000000000000000 <.text>:
>>>>>    0: 48 63 c8              movslq %eax,%rcx
>>>>>    3: 48 63 08              movslq (%rax),%rcx
>>>>>    6: 48 63 c8              movslq %eax,%rcx
>>>>>    9: 48 63 08              movslq (%rax),%rcx
>>>>>    c: 63 c8                movsxd %eax,%ecx
>>>>>    e: 63 08                movsxd (%rax),%ecx
>>>>>   10: 66 63 c8              movsxd %eax,%cx
>>>>>   13: 66 63 08              movsxd (%rax),%cx
>>>>> [hjl@gnu-cfl-2 tmp]$
>>>>
>>>> I'm sorry, but I have no idea what you're trying to tell or
>>>> demonstrate me with this. Things being the way you show them
>>>> right now does in no way mean this is how they should be.
>>>
>>>   * 'movslq' with AT&T mnemonic only accepts 64-bit destination
>>>      register.  'movsxd' should be used to encode 16-bit or 32-bit
>>>      destination register with both AT&T and Intel mnemonics.
>>
>> I think you've said exactly this before. I don't think, however,
>> that this addresses in any way my most recent remark regarding
>> the inconsistency of this.
>>
> 
> Some inconsistencies are unavoidable.  MOVSXD is one of them.

Unavoidable? I've outlined how they could be avoided here. It's
not like this can't be done properly. It's more like you don't
want it to be so (sort of an excuse being that there's endless
other inconsistencies, I guess, which I'm hoping to be able to
slowly get rid of, but for some backwards compatibility
considerations stand in the way).

Jan

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-02-05 13:23                           ` Jan Beulich
@ 2020-02-05 13:36                             ` H.J. Lu
  0 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2020-02-05 13:36 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Feb 5, 2020 at 5:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.02.2020 14:17, H.J. Lu wrote:
> > On Wed, Feb 5, 2020 at 5:14 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 05.02.2020 14:03, H.J. Lu wrote:
> >>> On Wed, Feb 5, 2020 at 4:48 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 05.02.2020 13:44, H.J. Lu wrote:
> >>>>> On Wed, Feb 5, 2020 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 03.02.2020 18:19, H.J. Lu wrote:
> >>>>>>> On Mon, Feb 3, 2020 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>> On 03.02.2020 15:49, H.J. Lu wrote:
> >>>>>>>>> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>>> And you broke previously working code, which has no testcase so
> >>>>>>>>>> far, but which
> >>>>>>>>>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
> >>>>>>>>>> adds a test for:
> >>>>>>>>>>
> >>>>>>>>>> movsxdl (%rax),%rcx
> >>>>>>>>>
> >>>>>>>>> This isn't valid AT&T mnemonic.  No one should use it.
> >>>>>>>>
> >>>>>>>> Mind telling me what you derive this from? It is my understanding
> >>>>>>>> that the general rule of how to derive AT&T mnemonics is to take
> >>>>>>>> the Intel manual's mnemonic and add a set of suffixes if varying
> >>>>>>>
> >>>>>>> MOVSXD is a special case.  The typical usages of AT&T syntax are
> >>>>>>>
> >>>>>>> movslq %eax, %rcx
> >>>>>>> movslq (%rax), %rcx
> >>>>>>>
> >>>>>>> Anything else should be MOVSXD without any suffixes.  It is one
> >>>>>>> less mnemonic we need to apply complex suffix rules.
> >>>>>>>
> >>>>>>>> operand sizes are permitted. I can accept _new_ insns (in
> >>>>>>>> particular SIMD ones) to not get suffixes supported when they're
> >>>>>>>> not really needed. But this is an original x86-64 GPR insn. It
> >>>>>>>> should be consistent with other original x86-64 GPR insns.
> >>>>>>>>
> >>>>>>>> Furthermore, if it really was intentional for your commit to
> >>>>>>>
> >>>>>>> Yes, it was intentional.
> >>>>>>
> >>>>>> Well, okay then, I'll remove the addition from the patch. But
> >>>>>> I guess you realize that consistency here would mean to not
> >>>>>> allow MOVSXD at all in AT&T mode (or at least to not "prefer"
> >>>>>> it, just like for MOVSX), but instead have suitable
> >>>>>
> >>>>> [hjl@gnu-cfl-2 tmp]$ cat x.s
> >>>>> movslq %eax, %rcx
> >>>>> movslq (%rax), %rcx
> >>>>> movsxd %eax, %rcx
> >>>>> movsxd (%rax), %rcx
> >>>>> movsxd %eax, %ecx
> >>>>> movsxd (%rax), %ecx
> >>>>> movsxd %eax, %cx
> >>>>> movsxd (%rax), %cx
> >>>>> [hjl@gnu-cfl-2 tmp]$ gcc -c x.s
> >>>>> [hjl@gnu-cfl-2 tmp]$ objdump -dw x.o
> >>>>>
> >>>>> x.o:     file format elf64-x86-64
> >>>>>
> >>>>>
> >>>>> Disassembly of section .text:
> >>>>>
> >>>>> 0000000000000000 <.text>:
> >>>>>    0: 48 63 c8              movslq %eax,%rcx
> >>>>>    3: 48 63 08              movslq (%rax),%rcx
> >>>>>    6: 48 63 c8              movslq %eax,%rcx
> >>>>>    9: 48 63 08              movslq (%rax),%rcx
> >>>>>    c: 63 c8                movsxd %eax,%ecx
> >>>>>    e: 63 08                movsxd (%rax),%ecx
> >>>>>   10: 66 63 c8              movsxd %eax,%cx
> >>>>>   13: 66 63 08              movsxd (%rax),%cx
> >>>>> [hjl@gnu-cfl-2 tmp]$
> >>>>
> >>>> I'm sorry, but I have no idea what you're trying to tell or
> >>>> demonstrate me with this. Things being the way you show them
> >>>> right now does in no way mean this is how they should be.
> >>>
> >>>   * 'movslq' with AT&T mnemonic only accepts 64-bit destination
> >>>      register.  'movsxd' should be used to encode 16-bit or 32-bit
> >>>      destination register with both AT&T and Intel mnemonics.
> >>
> >> I think you've said exactly this before. I don't think, however,
> >> that this addresses in any way my most recent remark regarding
> >> the inconsistency of this.
> >>
> >
> > Some inconsistencies are unavoidable.  MOVSXD is one of them.
>
> Unavoidable? I've outlined how they could be avoided here. It's
> not like this can't be done properly. It's more like you don't
> want it to be so (sort of an excuse being that there's endless
> other inconsistencies, I guess, which I'm hoping to be able to
> slowly get rid of, but for some backwards compatibility
> considerations stand in the way).

We don't do at the expanse more complex suffix handling
and strange syntax like "movsxdl (%rax),%rcx".

-- 
H.J.

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

* Re: [PATCH] x86-64: Properly encode and decode movsxd
  2020-01-23 20:25 [PATCH] x86-64: Properly encode and decode movsxd H.J. Lu
  2020-01-24  8:21 ` Jan Beulich
@ 2020-02-10 10:50 ` Jan Beulich
  1 sibling, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2020-02-10 10:50 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On 23.01.2020 21:24, H.J. Lu wrote:
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-movsxd-intel.d
> @@ -0,0 +1,26 @@
> +#source: x86-64-movsxd.s
> +#as:
> +#objdump: -dw -Mintel
> +#name: x86-64 movsxd (AMD64) (Intel mode)
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <_start>:
> + +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
> + +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	63 c8                	movsxd ecx,eax
> + +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
> + +[a-f0-9]+:	66 63 08             	movsxd cx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	48 63 c8             	movsxd rcx,eax
> + +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	48 63 08             	movsxd rcx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	63 c8                	movsxd ecx,eax
> + +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]
> + +[a-f0-9]+:	66 63 c8             	movsxd cx,eax
> + +[a-f0-9]+:	63 08                	movsxd ecx,DWORD PTR \[rax\]

FTR this line as well as ...

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-movsxd.d
> @@ -0,0 +1,25 @@
> +#as:
> +#objdump: -dw
> +#name: x86-64 movsxd (AMD64)
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <_start>:
> + +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
> + +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
> + +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
> + +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
> + +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
> + +[a-f0-9]+:	66 63 08             	movsxd \(%rax\),%cx
> + +[a-f0-9]+:	48 63 c8             	movslq %eax,%rcx
> + +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
> + +[a-f0-9]+:	48 63 08             	movslq \(%rax\),%rcx
> + +[a-f0-9]+:	63 c8                	movsxd %eax,%ecx
> + +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
> + +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx
> + +[a-f0-9]+:	66 63 c8             	movsxd %eax,%cx
> + +[a-f0-9]+:	63 08                	movsxd \(%rax\),%ecx

... as this one contradict ...

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-movsxd.s
> @@ -0,0 +1,20 @@
> +# 64-bit only MOVSXD with AMD64 ISA
> +	.text
> +_start:
> +	movslq	%eax, %rcx
> +	movslq	(%rax), %rcx
> +	movsxd	%eax, %ecx
> +	movsxd	(%rax), %ecx
> +	movsxd	%eax, %cx
> +	movsxd	(%rax), %cx
> +
> +	.intel_syntax noprefix
> +	movsxd	rcx, eax
> +	movsxd	rcx, DWORD PTR [rax]
> +	movsxd	rcx, [rax]
> +	movsxd	ecx, eax
> +	movsxd	ecx, DWORD PTR [rax]
> +	movsxd	ecx, [rax]
> +	movsxd	cx, eax
> +	movsxd	cx, DWORD PTR [rax]

... this respective source line. I found this while re-basing my
previously submitted patch, which - as said before - deals with
the whole set of issues in a more generic way, and which caused
this wrong test to fail. No need to create/submit a fix - the
re-worked patch is doing fine, but before submitting I first
want to address the other inconsistencies that you've
introduced by be4c5e58bd ("x86: Always disallow double word
suffix with word general register") changing check_long_reg()
without also changing check_{byte,word}_reg(), as previously
pointed out.

This looks to be yet another instance of a testcase having got
created by taking what the assembler happens to produce,
instead of putting in expectations of what the assembler
_should_ produce if it worked correctly.

Jan

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

end of thread, other threads:[~2020-02-10 10:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 20:25 [PATCH] x86-64: Properly encode and decode movsxd H.J. Lu
2020-01-24  8:21 ` Jan Beulich
2020-01-24 11:52   ` H.J. Lu
2020-01-26 15:01     ` H.J. Lu
2020-02-03  8:38       ` Jan Beulich
2020-02-03 14:50         ` H.J. Lu
2020-02-03 16:34           ` Jan Beulich
2020-02-03 17:19             ` H.J. Lu
2020-02-05  8:18               ` Jan Beulich
2020-02-05 12:44                 ` H.J. Lu
2020-02-05 12:48                   ` Jan Beulich
2020-02-05 13:03                     ` H.J. Lu
2020-02-05 13:14                       ` Jan Beulich
2020-02-05 13:18                         ` H.J. Lu
2020-02-05 13:23                           ` Jan Beulich
2020-02-05 13:36                             ` H.J. Lu
2020-01-27 16:17     ` Jan Beulich
2020-01-27 16:29       ` H.J. Lu
2020-02-10 10:50 ` Jan Beulich

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