[PATCH] x86: Properly decode EVEX.W in vcvt[u]si2s[sd] in 32-bit

[H.J. Lu]

Update x86 disassembler to ignore the EVEX.W bit in EVEX vcvt[u]si2s[sd]
instructions in 32-bit mode.

gas/

PR binutils/23655
* testsuite/gas/i386/evex.d: New file.
* testsuite/gas/i386/evex.s: Likewise.
* testsuite/gas/i386/i386.exp: Run evex.

opcodes/

PR binutils/23655
* i386-dis-evex.h (evex_table): Replace Eq with Edqa for
vcvtsi2ss%LQ, vcvtsi2sd%LQ, vcvtusi2ss%LQ and vcvtusi2sd%LQ.
* i386-dis.c (Edqa): New.
(dqa_mode): Likewise.
(intel_operand_size): Handle dqa_mode as m_mode.
(OP_E_register): Handle dqa_mode as dq_mode.
(OP_E_memory): Set shift for dqa_mode based on address_mode.
---
gas/testsuite/gas/i386/evex.d | 16 ++++++++++++++++
gas/testsuite/gas/i386/evex.s | 11 +++++++++++
gas/testsuite/gas/i386/i386.exp | 1 +
opcodes/i386-dis-evex.h | 8 ++++----
opcodes/i386-dis.c | 8 ++++++++
5 files changed, 40 insertions(+), 4 deletions(-)
create mode 100644 gas/testsuite/gas/i386/evex.d
create mode 100644 gas/testsuite/gas/i386/evex.s

diff --git a/gas/testsuite/gas/i386/evex.d b/gas/testsuite/gas/i386/evex.d
new file mode 100644
index 0000000000..ffbcdc1c09
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+#pass
diff --git a/gas/testsuite/gas/i386/evex.s b/gas/testsuite/gas/i386/evex.s
new file mode 100644
index 0000000000..a64cc573dc
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.s
@@ -0,0 +1,11 @@
+# Check EVEX instructions
+
+ .allow_index_reg
+ .text
+_start:
+ .byte 0x62, 0xf1, 0xd6, 0x38, 0x2a, 0xf0
+ .byte 0x62, 0xf1, 0xd7, 0x38, 0x2a, 0xf0
+ .byte 0x62, 0xf1, 0xd6, 0x08, 0x7b, 0xf0
+ .byte 0x62, 0xf1, 0xd7, 0x08, 0x7b, 0xf0
+ .byte 0x62, 0xf1, 0xd6, 0x38, 0x7b, 0xf0
+ .byte 0x62, 0xf1, 0xd7, 0x38, 0x7b, 0xf0
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index 088fece0e7..203def94e9 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -226,6 +226,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_32_check]]
run_dump_test "avx512er-intel"
run_dump_test "avx512pf"
run_dump_test "avx512pf-intel"
+ run_dump_test "evex"
run_dump_test "evex-lig256"
run_dump_test "evex-lig512"
run_dump_test "evex-lig256-intel"
diff --git a/opcodes/i386-dis-evex.h b/opcodes/i386-dis-evex.h
index 8b82578934..f59c7cc872 100644
--- a/opcodes/i386-dis-evex.h
+++ b/opcodes/i386-dis-evex.h
@@ -3046,12 +3046,12 @@ static const struct dis386 evex_table[][256] = {
/* EVEX_W_0F2A_P_1 */
{
{ "vcvtsi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Ed }, 0 },
- { "vcvtsi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Eq }, 0 },
+ { "vcvtsi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Edqa }, 0 },
},
/* EVEX_W_0F2A_P_3 */
{
{ "vcvtsi2sd%LQ", { XMScalar, VexScalar, Ed }, 0 },
- { "vcvtsi2sd%LQ", { XMScalar, VexScalar, EXxEVexR, Eq }, 0 },
+ { "vcvtsi2sd%LQ", { XMScalar, VexScalar, EXxEVexR, Edqa }, 0 },
},
/* EVEX_W_0F2B_P_0 */
{
@@ -3383,7 +3383,7 @@ static const struct dis386 evex_table[][256] = {
/* EVEX_W_0F7B_P_1 */
{
{ "vcvtusi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Ed }, 0 },
- { "vcvtusi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Eq }, 0 },
+ { "vcvtusi2ss%LQ", { XMScalar, VexScalar, EXxEVexR, Edqa }, 0 },
},
/* EVEX_W_0F7B_P_2 */
{
@@ -3393,7 +3393,7 @@ static const struct dis386 evex_table[][256] = {
/* EVEX_W_0F7B_P_3 */
{
{ "vcvtusi2sd%LQ", { XMScalar, VexScalar, Ed }, 0 },
- { "vcvtusi2sd%LQ", { XMScalar, VexScalar, EXxEVexR, Eq }, 0 },
+ { "vcvtusi2sd%LQ", { XMScalar, VexScalar, EXxEVexR, Edqa }, 0 },
},
/* EVEX_W_0F7E_P_1 */
{
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index f453989b70..83c610703d 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -260,6 +260,7 @@ fetch_data (struct disassemble_info *info, bfd_byte *addr)
#define Edb { OP_E, db_mode }
#define Edw { OP_E, dw_mode }
#define Edqd { OP_E, dqd_mode }
+#define Edqa { OP_E, dqa_mode }
#define Eq { OP_E, q_mode }
#define indirEv { OP_indirE, indir_v_mode }
#define indirEp { OP_indirE, f_mode }
@@ -591,6 +592,8 @@ enum
dw_mode,
/* registers like dq_mode, memory like d_mode. */
dqd_mode,
+ /* operand size depends on the W bit as well as address mode. */
+ dqa_mode,
/* normal vex mode */
vex_mode,
/* 128bit vex mode */
@@ -14805,6 +14808,7 @@ intel_operand_size (int bytemode, int sizeflag)
case q_swap_mode:
oappend ("QWORD PTR ");
break;
+ case dqa_mode:
case m_mode:
if (address_mode == mode_64bit)
oappend ("QWORD PTR ");
@@ -15163,6 +15167,7 @@ OP_E_register (int bytemode, int sizeflag)
case dqb_mode:
case dqd_mode:
case dqw_mode:
+ case dqa_mode:
USED_REX (REX_W);
if (rex & REX_W)
names = names64;
@@ -15305,6 +15310,9 @@ OP_E_memory (int bytemode, int sizeflag)
case xmm_mb_mode:
shift = 0;
break;
+ case dqa_mode:
+ shift = address_mode == mode_64bit ? 3 : 2;
+ break;
default:
abort ();
}

[Jan Beulich]

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl %eax,\{rd-sae\},%xmm5,%xmm6

Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?


Jan

[H.J. Lu]

Post by Jan Beulich

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl %eax,\{rd-sae\},%xmm5,%xmm6

Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?

I didn't add any new instructions. These testcases are written in .byte.
I just fixed the existing entries in disassembler.

[Jan Beulich]

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl %eax,\{rd-sae\},%xmm5,%xmm6

Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?

I didn't add any new instructions. These testcases are written in .byte.
I just fixed the existing entries in disassembler.

I didn't say "new instructions", but "new test": In a new test I don't think
it is appropriate to record expectations (here: all of the l suffixes above)
that are actually expected to not be there, but appear just because of
brokenness. Since you touch the respective disassembler patterns
anyway I don't really understand why you didn't make the bogus suffixes
go away in one go. These instructions usefully have suffixes only in
64-bit mode, and earlier you've told me you don't want suffixes on newer
insns when they're not needed.

Jan

[H.J. Lu]

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl %eax,\{rd-sae\},%xmm5,%xmm6

Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?

I didn't add any new instructions. These testcases are written in .byte.
I just fixed the existing entries in disassembler.

I didn't say "new instructions", but "new test": In a new test I don't think
it is appropriate to record expectations (here: all of the l suffixes above)
that are actually expected to not be there, but appear just because of
brokenness. Since you touch the respective disassembler patterns
anyway I don't really understand why you didn't make the bogus suffixes
go away in one go. These instructions usefully have suffixes only in

I am fixing a different issue. Please feel free to submit a separate
patch to address this particular issue.

Post by Jan Beulich
64-bit mode, and earlier you've told me you don't want suffixes on newer
insns when they're not needed.
Jan

[Jan Beulich]

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl

%eax,\{rd-sae\},%xmm5,%xmm6

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl

%eax,\{rd-sae\},%xmm5,%xmm6

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich
Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?

I didn't add any new instructions. These testcases are written in .byte.
I just fixed the existing entries in disassembler.

I didn't say "new instructions", but "new test": In a new test I don't think
it is appropriate to record expectations (here: all of the l suffixes above)
that are actually expected to not be there, but appear just because of
brokenness. Since you touch the respective disassembler patterns
anyway I don't really understand why you didn't make the bogus suffixes
go away in one go. These instructions usefully have suffixes only in

I am fixing a different issue. Please feel free to submit a separate
patch to address this particular issue.

I understand you're fixing a different issue, but while doing so you
introduce a testcase with bogus expected output. I don't think new
testcases should ever be added when their expectations don't
match "good" output.

Jan

[H.J. Lu]

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex.d
@@ -0,0 +1,16 @@
+#objdump: -dw -Msuffix
+#name: i386 EVX insns
+
+.*: +file format .*
+
+
+
+ +[a-f0-9]+: 62 f1 d6 38 2a f0 vcvtsi2ssl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 38 2a f0 vcvtsi2sdl %eax,\{rd-sae\},%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 08 7b f0 vcvtusi2ssl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d7 08 7b f0 vcvtusi2sdl %eax,%xmm5,%xmm6
+ +[a-f0-9]+: 62 f1 d6 38 7b f0 vcvtusi2ssl

%eax,\{rd-sae\},%xmm5,%xmm6

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich

Post by H.J. Lu
+ +[a-f0-9]+: 62 f1 d7 38 7b f0 vcvtusi2sdl

%eax,\{rd-sae\},%xmm5,%xmm6

Post by Jan Beulich

Post by H.J. Lu

Post by Jan Beulich
Hmm, a new test demanding (according to what you've told me in earlier
discussions) bad behavior: You've said that you don't want suffixes on newer
insns when they're not needed. While these insns may indeed better have
suffixes in 64-bit mode (they strictly need them only with memory operands),
there's clearly nothing to disambiguate in 16- and 32-bit modes. May I ask
for consistency please between what you demand for patches I submit and
ones you commit, once again without even giving a little time for reviews?

I didn't add any new instructions. These testcases are written in .byte.
I just fixed the existing entries in disassembler.

I didn't say "new instructions", but "new test": In a new test I don't think
it is appropriate to record expectations (here: all of the l suffixes above)
that are actually expected to not be there, but appear just because of
brokenness. Since you touch the respective disassembler patterns
anyway I don't really understand why you didn't make the bogus suffixes
go away in one go. These instructions usefully have suffixes only in

I am fixing a different issue. Please feel free to submit a separate
patch to address this particular issue.

I understand you're fixing a different issue, but while doing so you
introduce a testcase with bogus expected output. I don't think new
testcases should ever be added when their expectations don't
match "good" output.

The expected disassembler output comes from the existing
disassembler. I prefer to fix a single issue in a single commit.
Please feel free to submit a patch for disassembler, which may
touch other existing testcases.