x86: drop vex_encoding_vex2 enumerator

Message ID c8ca9ce3-4dcc-f251-88d4-e3fb31d80f71@suse.com
State New
Headers show
Series
  • x86: drop vex_encoding_vex2 enumerator
Related show

Commit Message

Jan Beulich Jan. 15, 2020, 7:42 a.m.
Documentation clearly says "prefer {2,3}-byte VEX prefix for VEX
instruction", as opposed to "encode with EVEX prefix" for {evex}. Hence
there not being a way to VEX-encode an insn should not be an error (not
even a warning), and with this the separate enumerator becomes unneeded.
(Really I'm having trouble seeing what {vex2} would be useful for when
it's just a suggestion hint, not one allowing the programmer to mandate
the used encoding.)

gas/
2020-01-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (struct _i386_insn): Drop vex_encoding_vex2
	enumerator from vec_encoding enumeration.
	(parse_insn): Use vex_encoding_default for {vex2}.
	(VEX_check_operands): Don't error when {vex2} or {vex3} cannot
	be honored.

Comments

H.J. Lu Jan. 15, 2020, 10:22 p.m. | #1
On Tue, Jan 14, 2020 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>

> Documentation clearly says "prefer {2,3}-byte VEX prefix for VEX

> instruction", as opposed to "encode with EVEX prefix" for {evex}. Hence

> there not being a way to VEX-encode an insn should not be an error (not


We can't prefer any VEX prefix on non AVX instructions:

{vex} inc %rax

should be an error.

> even a warning), and with this the separate enumerator becomes unneeded.

> (Really I'm having trouble seeing what {vex2} would be useful for when

> it's just a suggestion hint, not one allowing the programmer to mandate

> the used encoding.)


Please keep it for now.   We are planning to use it in the future.

-- 
H.J.
Jan Beulich Jan. 16, 2020, 8:36 a.m. | #2
On 15.01.2020 23:22, H.J. Lu wrote:
> On Tue, Jan 14, 2020 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> Documentation clearly says "prefer {2,3}-byte VEX prefix for VEX

>> instruction", as opposed to "encode with EVEX prefix" for {evex}. Hence

>> there not being a way to VEX-encode an insn should not be an error (not

> 

> We can't prefer any VEX prefix on non AVX instructions:

> 

> {vex} inc %rax

> 

> should be an error.


How is this in line with what the documentation states? (I
assume you mean {vex2} in your example; I could see a future
{vex} as being mandatory just like {evex} is. If {vex2} is
to be mandatory, what use is it for VEX-encodable insns? The
2-byte form will be preferred anyway.)

>> even a warning), and with this the separate enumerator becomes unneeded.

>> (Really I'm having trouble seeing what {vex2} would be useful for when

>> it's just a suggestion hint, not one allowing the programmer to mandate

>> the used encoding.)

> 

> Please keep it for now.   We are planning to use it in the future.


Then may I please ask that you update the documentation to
clarify which pseudo prefixes are hints (and silently ignored
when they cannot be fulfilled) and which ones are mandatory?

Thanks, Jan
H.J. Lu Jan. 16, 2020, 10:32 p.m. | #3
On Thu, Jan 16, 2020 at 12:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 15.01.2020 23:22, H.J. Lu wrote:

> > On Tue, Jan 14, 2020 at 11:42 PM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> Documentation clearly says "prefer {2,3}-byte VEX prefix for VEX

> >> instruction", as opposed to "encode with EVEX prefix" for {evex}. Hence

> >> there not being a way to VEX-encode an insn should not be an error (not

> >

> > We can't prefer any VEX prefix on non AVX instructions:

> >

> > {vex} inc %rax

> >

> > should be an error.

>

> How is this in line with what the documentation states? (I

> assume you mean {vex2} in your example; I could see a future

> {vex} as being mandatory just like {evex} is. If {vex2} is

> to be mandatory, what use is it for VEX-encodable insns? The

> 2-byte form will be preferred anyway.)

>

> >> even a warning), and with this the separate enumerator becomes unneeded.

> >> (Really I'm having trouble seeing what {vex2} would be useful for when

> >> it's just a suggestion hint, not one allowing the programmer to mandate

> >> the used encoding.)

> >

> > Please keep it for now.   We are planning to use it in the future.

>

> Then may I please ask that you update the documentation to

> clarify which pseudo prefixes are hints (and silently ignored

> when they cannot be fulfilled) and which ones are mandatory?

>


How about this?

-- 
H.J.
From 8f8b6e750f013a51ec15128781c17bba36ca6512 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 16 Jan 2020 14:27:03 -0800
Subject: [PATCH] x86: Update {vex2} and {vex3} documentation

There are 2-byte VEX prefix and 3-byte VEX prefix.  2-byte VEX prefix
can't encode all operands.  By default, assembler uses 2-byte VEX prefix
if possible.  {vex3} can be used to force 3-byte VEX prefix.  {vex2} is
nop at the moment.

	* doc/c-i386.texi: Update {vex2} and {vex3} documentation.
---
 gas/ChangeLog       | 4 ++++
 gas/doc/c-i386.texi | 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index f2b49b7a89..94fb08c2f0 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-16  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* doc/c-i386.texi: Update {vex2} and {vex3} documentation.
+
 2020-01-16  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 
 	PR 25376
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 4b25803013..37965607ec 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -758,10 +758,11 @@ Different encoding options can be specified via pseudo prefixes:
 @samp{@{store@}} -- prefer store-form instruction.
 
 @item
-@samp{@{vex2@}} -- prefer 2-byte VEX prefix for VEX instruction.
+@samp{@{vex2@}} -- encode with 2-byte VEX prefix for VEX instruction if
+possible.
 
 @item
-@samp{@{vex3@}} -- prefer 3-byte VEX prefix for VEX instruction.
+@samp{@{vex3@}} -- encode with 3-byte VEX prefix for VEX instruction
 
 @item
 @samp{@{evex@}} --  encode with EVEX prefix.
Jan Beulich Jan. 17, 2020, 9:03 a.m. | #4
On 16.01.2020 23:32,  H.J. Lu  wrote:
> How about this?


For {vex2} "if possible" still leaves open what happens if this
is not possible. Aiui there are two distinct cases:
- 3-byte VEX could be used, in which case the prefix is ignored
- only legacy or EVEX encoding is possible, in which case there'd
  be an error (not really sure about EVEX here)
I think it would be better to have {vex} and {vex3}.

Furthermore both say "for VEX instruction", leaving open what is
to happen for non-VEX-encodable ones. It also leaves open what
their interaction with XOP-encoded insns is.

Jan
H.J. Lu Jan. 17, 2020, 2:48 p.m. | #5
On Fri, Jan 17, 2020 at 1:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 16.01.2020 23:32,  H.J. Lu  wrote:

> > How about this?

>

> For {vex2} "if possible" still leaves open what happens if this

> is not possible. Aiui there are two distinct cases:

> - 3-byte VEX could be used, in which case the prefix is ignored


Did you mean "must be used"?

> - only legacy or EVEX encoding is possible, in which case there'd

>   be an error (not really sure about EVEX here)

> I think it would be better to have {vex} and {vex3}.


Agreed.  I will make the change and keep {vex2} for backward compatibility.
How about this patch?

> Furthermore both say "for VEX instruction", leaving open what is

> to happen for non-VEX-encodable ones. It also leaves open what

> their interaction with XOP-encoded insns is.


Here is the updated document.

--
H.J.
From 862d4c8d4aa5b69b687d95a47e94786b6dec02a6 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 16 Jan 2020 14:27:03 -0800
Subject: [PATCH] x86: Add {vex} pseudo prefix

There are 2-byte VEX prefix and 3-byte VEX prefix.  2-byte VEX prefix
can't encode all operands.  By default, assembler tries 2-byte VEX prefix
first.  {vex3} can be used to force 3-byte VEX prefix.  This patch adds
{vex} pseudo prefix and keeps {vex2} for backward compatibility.

gas/

	* config/tc-i386.c (_i386_insn): Replace vex_encoding_vex2
	with vex_encoding_vex.
	(parse_insn): Likewise.
	* doc/c-i386.texi: Replace {vex2} with {vex}.  Update {vex}
	and {vex3} documentation.
	* testsuite/gas/i386/pseudos.s: Replace 3 {vex2} tests with
	{vex}.
	* testsuite/gas/i386/x86-64-pseudos.s: Likewise.

opcodes/

	* i386-opc.tbl: Add {vex} pseudo prefix.
	* i386-tbl.h: Regenerated.
---
 gas/ChangeLog                           | 11 +++++++++++
 gas/config/tc-i386.c                    |  6 +++---
 gas/doc/c-i386.texi                     |  4 ++--
 gas/testsuite/gas/i386/pseudos.s        |  6 +++---
 gas/testsuite/gas/i386/x86-64-pseudos.s |  6 +++---
 opcodes/ChangeLog                       |  5 +++++
 opcodes/i386-opc.tbl                    |  1 +
 opcodes/i386-tbl.h                      | 12 ++++++++++++
 8 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index f2b49b7a89d..e3adaa722ac 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,14 @@
+2020-01-17  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/tc-i386.c (_i386_insn): Replace vex_encoding_vex2
+	with vex_encoding_vex.
+	(parse_insn): Likewise.
+	* doc/c-i386.texi: Replace {vex2} with {vex}.  Update {vex}
+	and {vex3} documentation.
+	* testsuite/gas/i386/pseudos.s: Replace 3 {vex2} tests with
+	{vex}.
+	* testsuite/gas/i386/x86-64-pseudos.s: Likewise.
+
 2020-01-16  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 
 	PR 25376
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 3f7f4222bf5..8728725b82d 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -419,7 +419,7 @@ struct _i386_insn
     enum
       {
 	vex_encoding_default = 0,
-	vex_encoding_vex2,
+	vex_encoding_vex,
 	vex_encoding_vex3,
 	vex_encoding_evex
       } vec_encoding;
@@ -4722,8 +4722,8 @@ parse_insn (char *line, char *mnemonic)
 		  i.dir_encoding = dir_encoding_store;
 		  break;
 		case 0x4:
-		  /* {vex2} */
-		  i.vec_encoding = vex_encoding_vex2;
+		  /* {vex} */
+		  i.vec_encoding = vex_encoding_vex;
 		  break;
 		case 0x5:
 		  /* {vex3} */
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 4b258030135..e8685dd046f 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -758,10 +758,10 @@ Different encoding options can be specified via pseudo prefixes:
 @samp{@{store@}} -- prefer store-form instruction.
 
 @item
-@samp{@{vex2@}} -- prefer 2-byte VEX prefix for VEX instruction.
+@samp{@{vex@}} --  encode with VEX prefix.
 
 @item
-@samp{@{vex3@}} -- prefer 3-byte VEX prefix for VEX instruction.
+@samp{@{vex3@}} -- encode with 3-byte VEX prefix.
 
 @item
 @samp{@{evex@}} --  encode with EVEX prefix.
diff --git a/gas/testsuite/gas/i386/pseudos.s b/gas/testsuite/gas/i386/pseudos.s
index 419a4c51e92..19900dd7e59 100644
--- a/gas/testsuite/gas/i386/pseudos.s
+++ b/gas/testsuite/gas/i386/pseudos.s
@@ -6,9 +6,9 @@ _start:
 	{vex3} {load} vmovaps %xmm7,%xmm2
 	{vex3} {store} vmovaps %xmm7,%xmm2
 	vmovaps %xmm7,%xmm2
-	{vex2} vmovaps %xmm7,%xmm2
-	{vex2} {load} vmovaps %xmm7,%xmm2
-	{vex2} {store} vmovaps %xmm7,%xmm2
+	{vex} vmovaps %xmm7,%xmm2
+	{vex} {load} vmovaps %xmm7,%xmm2
+	{vex} {store} vmovaps %xmm7,%xmm2
 	{vex3} vmovaps (%eax),%xmm2
 	vmovaps (%eax),%xmm2
 	{vex2} vmovaps (%eax),%xmm2
diff --git a/gas/testsuite/gas/i386/x86-64-pseudos.s b/gas/testsuite/gas/i386/x86-64-pseudos.s
index 33712d3e636..14b6c92b864 100644
--- a/gas/testsuite/gas/i386/x86-64-pseudos.s
+++ b/gas/testsuite/gas/i386/x86-64-pseudos.s
@@ -6,9 +6,9 @@ _start:
 	{vex3} {load} vmovaps %xmm7,%xmm2
 	{vex3} {store} vmovaps %xmm7,%xmm2
 	vmovaps %xmm7,%xmm2
-	{vex2} vmovaps %xmm7,%xmm2
-	{vex2} {load} vmovaps %xmm7,%xmm2
-	{vex2} {store} vmovaps %xmm7,%xmm2
+	{vex} vmovaps %xmm7,%xmm2
+	{vex} {load} vmovaps %xmm7,%xmm2
+	{vex} {store} vmovaps %xmm7,%xmm2
 	{vex3} vmovaps (%rax),%xmm2
 	vmovaps (%rax),%xmm2
 	{vex2} vmovaps (%rax),%xmm2
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index 2d3b6d14401..e48e566cbc3 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-17  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* i386-opc.tbl: Add {vex} pseudo prefix.
+	* i386-tbl.h: Regenerated.
+
 2020-01-16  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 
 	PR 25376
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 73cd6c6a11e..d8af2599032 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -879,6 +879,7 @@ rex.wrxb, 0, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ld
 {disp32}, 0, 0x1, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
 {load}, 0, 0x2, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
 {store}, 0, 0x3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
+{vex}, 0, 0x4, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
 {vex2}, 0, 0x4, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
 {vex3}, 0, 0x5, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
 {evex}, 0, 0x6, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, { 0 }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index e9381bf11b2..758d92bcfd2 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -7600,6 +7600,18 @@ 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 } } } },
+  { "{vex}", 0x4, None, 0, 0,
+    { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        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, 1, 1, 1, 1, 0,
+      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
+      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
+    { { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	  0, 0, 0, 0, 0, 0 } } } },
   { "{vex2}", 0x4, None, 0, 0,
     { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
Jan Beulich Jan. 17, 2020, 2:57 p.m. | #6
On 17.01.2020 15:48, H.J. Lu wrote:
> On Fri, Jan 17, 2020 at 1:03 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 16.01.2020 23:32,  H.J. Lu  wrote:

>>> How about this?

>>

>> For {vex2} "if possible" still leaves open what happens if this

>> is not possible. Aiui there are two distinct cases:

>> - 3-byte VEX could be used, in which case the prefix is ignored

> 

> Did you mean "must be used"?


Well, yes, in a way.

>> - only legacy or EVEX encoding is possible, in which case there'd

>>   be an error (not really sure about EVEX here)

>> I think it would be better to have {vex} and {vex3}.

> 

> Agreed.  I will make the change and keep {vex2} for backward compatibility.

> How about this patch?

> 

>> Furthermore both say "for VEX instruction", leaving open what is

>> to happen for non-VEX-encodable ones. It also leaves open what

>> their interaction with XOP-encoded insns is.

> 

> Here is the updated document.


Looks good, thanks.

Jan

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -419,7 +419,6 @@  struct _i386_insn
     enum
       {
 	vex_encoding_default = 0,
-	vex_encoding_vex2,
 	vex_encoding_vex3,
 	vex_encoding_evex
       } vec_encoding;
@@ -4723,7 +4722,7 @@  parse_insn (char *line, char *mnemonic)
 		  break;
 		case 0x4:
 		  /* {vex2} */
-		  i.vec_encoding = vex_encoding_vex2;
+		  i.vec_encoding = vex_encoding_default;
 		  break;
 		case 0x5:
 		  /* {vex3} */
@@ -5707,15 +5706,7 @@  VEX_check_operands (const insn_template
     }
 
   if (!t->opcode_modifier.vex)
-    {
-      /* This instruction template doesn't have VEX prefix.  */
-      if (i.vec_encoding != vex_encoding_default)
-	{
-	  i.error = unsupported;
-	  return 1;
-	}
-      return 0;
-    }
+    return 0;
 
   /* Check the special Imm4 cases; must be the first operand.  */
   if (t->cpu_flags.bitfield.cpuxop && t->operands == 5)