x86: Extract extended states from instruction template

Message ID CAMe9rOqKcbEzwFUfRnzedP80daF+J+u7NBeyF1RGDtCJ7b9F0Q@mail.gmail.com
State New
Headers show
Series
  • x86: Extract extended states from instruction template
Related show

Commit Message

Alan Modra via Binutils July 10, 2020, 1:41 p.m.
On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 09.07.2020 18:22, H.J. Lu wrote:

> > On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:

> >> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>> On 09.07.2020 17:44, H.J. Lu wrote:

> >>>> Should we add a field to i386 opcode table for this?

> >>>

> >>> I was wanting to try without, to see how much logic would be needed.

> >>> If it gets too much, doing so may be the better choice.

> >>

> >> I am checking in this patch.

> >

> > Here is the right patch.

>

> Well, this is certainly an improvement. But it doesn't come close to

> what's needed. Again - imo the concept of i.has_reg* isn't helpful

> at all. It's not the registers an insn _actually_ accesses, but the

> ones the template says it might access. Think of vfpclass* with a

> memory operand, for example. Or see some of the extra checks of

> certain opcodes when setting the MMX bit, which could be avoided by

> using such an alternative model.

>

> I'm leaving aside here special cases where the templates don't carry

> enough information. I'm also leaving aside the need to e.g. record

> mask register use in some form (maybe translating to ZMM use if you

> want to avoid introducing a separate tracking bit), as well as

> various other issues I had spotted without looking very closely yet.


Here is a patch to extract extended states from operand types in
instruction template and set xstate_zmm for master register move.

> I believe a fundamental issue here is the very scarce testing: While

> it may be avoidable to add an attribute to the insn templates, there

> should be full testing coverage of _all_ insns with _all_ possible

> operand type combinations, if the resulting notes are to be

> dependable. If such (presumably table based) testing was there (and

> of course if the table reflected the real needs rather than what

> gas currently happens to produce), you'd easily notice the issues

> I've spotted (and likely more).


This is just a starting point.  I will fix any issues discovered.

Thanks.

-- 
H.J.

Comments

Jan Beulich July 10, 2020, 3:17 p.m. | #1
On 10.07.2020 15:41, H.J. Lu wrote:
> On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 09.07.2020 18:22, H.J. Lu wrote:

>>> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:

>>>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>> On 09.07.2020 17:44, H.J. Lu wrote:

>>>>>> Should we add a field to i386 opcode table for this?

>>>>>

>>>>> I was wanting to try without, to see how much logic would be needed.

>>>>> If it gets too much, doing so may be the better choice.

>>>>

>>>> I am checking in this patch.

>>>

>>> Here is the right patch.

>>

>> Well, this is certainly an improvement. But it doesn't come close to

>> what's needed. Again - imo the concept of i.has_reg* isn't helpful

>> at all. It's not the registers an insn _actually_ accesses, but the

>> ones the template says it might access. Think of vfpclass* with a

>> memory operand, for example. Or see some of the extra checks of

>> certain opcodes when setting the MMX bit, which could be avoided by

>> using such an alternative model.

>>

>> I'm leaving aside here special cases where the templates don't carry

>> enough information. I'm also leaving aside the need to e.g. record

>> mask register use in some form (maybe translating to ZMM use if you

>> want to avoid introducing a separate tracking bit), as well as

>> various other issues I had spotted without looking very closely yet.

> 

> Here is a patch to extract extended states from operand types in

> instruction template and set xstate_zmm for master register move.


Ah yes, I appreciate this move.

>> I believe a fundamental issue here is the very scarce testing: While

>> it may be avoidable to add an attribute to the insn templates, there

>> should be full testing coverage of _all_ insns with _all_ possible

>> operand type combinations, if the resulting notes are to be

>> dependable. If such (presumably table based) testing was there (and

>> of course if the table reflected the real needs rather than what

>> gas currently happens to produce), you'd easily notice the issues

>> I've spotted (and likely more).

> 

> This is just a starting point.  I will fix any issues discovered.


I'll see to get to running this over the various little examples I
have, once it got committed (and I got around to re-basing).

I don't think altering existing test cases is a good idea though -
accumulating the flags from multiple instructions will leave it
unclear which insn caused which flag(s) to get set. As said before,
I am of the pretty strong opinion that testsuite-wise the only
helpful thing here is to individually test insns (and in various
cases even the same insn with varying operands).

Jan
Alan Modra via Binutils July 10, 2020, 3:42 p.m. | #2
On Fri, Jul 10, 2020 at 8:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 10.07.2020 15:41, H.J. Lu wrote:

> > On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 09.07.2020 18:22, H.J. Lu wrote:

> >>> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:

> >>>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>> On 09.07.2020 17:44, H.J. Lu wrote:

> >>>>>> Should we add a field to i386 opcode table for this?

> >>>>>

> >>>>> I was wanting to try without, to see how much logic would be needed.

> >>>>> If it gets too much, doing so may be the better choice.

> >>>>

> >>>> I am checking in this patch.

> >>>

> >>> Here is the right patch.

> >>

> >> Well, this is certainly an improvement. But it doesn't come close to

> >> what's needed. Again - imo the concept of i.has_reg* isn't helpful

> >> at all. It's not the registers an insn _actually_ accesses, but the

> >> ones the template says it might access. Think of vfpclass* with a

> >> memory operand, for example. Or see some of the extra checks of

> >> certain opcodes when setting the MMX bit, which could be avoided by

> >> using such an alternative model.

> >>

> >> I'm leaving aside here special cases where the templates don't carry

> >> enough information. I'm also leaving aside the need to e.g. record

> >> mask register use in some form (maybe translating to ZMM use if you

> >> want to avoid introducing a separate tracking bit), as well as

> >> various other issues I had spotted without looking very closely yet.

> >

> > Here is a patch to extract extended states from operand types in

> > instruction template and set xstate_zmm for master register move.

>

> Ah yes, I appreciate this move.

>

> >> I believe a fundamental issue here is the very scarce testing: While

> >> it may be avoidable to add an attribute to the insn templates, there

> >> should be full testing coverage of _all_ insns with _all_ possible

> >> operand type combinations, if the resulting notes are to be

> >> dependable. If such (presumably table based) testing was there (and

> >> of course if the table reflected the real needs rather than what

> >> gas currently happens to produce), you'd easily notice the issues

> >> I've spotted (and likely more).

> >

> > This is just a starting point.  I will fix any issues discovered.

>

> I'll see to get to running this over the various little examples I

> have, once it got committed (and I got around to re-basing).

>

> I don't think altering existing test cases is a good idea though -

> accumulating the flags from multiple instructions will leave it

> unclear which insn caused which flag(s) to get set. As said before,

> I am of the pretty strong opinion that testsuite-wise the only

> helpful thing here is to individually test insns (and in various

> cases even the same insn with varying operands).


Fixed.

This is the updated patch I am checking in.

Thanks.

-- 
H.J.
From cc8ae5dcbfe323d14c84012c606e18235176b7e8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 10 Jul 2020 06:15:10 -0700
Subject: [PATCH] x86: Extract extended states from instruction template

Extract extended states from operand types in instruction template.  Set
xstate_zmm for master register move.

	* config/tc-i386.c (_i386_insn): Remove has_regmmx, has_regxmm,
	has_regymm, has_regzmm and has_regtmm.  Add xstate.
	(md_assemble): Set i.xstate from operand types in instruction
	template.
	(build_modrm_byte): Updated.
	(output_insn): Check i.xstate.
	* testsuite/gas/i386/i386.exp: Run property-6 and
	x86-64-property-6.
	* testsuite/gas/i386/property-6.d: New file.
	* testsuite/gas/i386/property-6.s: Updated.
	* testsuite/gas/i386/x86-64-property-6.d: Likewise.
---
 gas/config/tc-i386.c                       | 127 +++++++++------------
 gas/testsuite/gas/i386/i386.exp            |   2 +
 gas/testsuite/gas/i386/property-6.d        |   9 ++
 gas/testsuite/gas/i386/property-6.s        |   2 +
 gas/testsuite/gas/i386/x86-64-property-6.d |  10 ++
 5 files changed, 77 insertions(+), 73 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/property-6.d
 create mode 100644 gas/testsuite/gas/i386/property-6.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-property-6.d

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 0e4291499b..0eb2b94e04 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -362,20 +362,20 @@ struct _i386_insn
     /* The operand to a branch insn indicates an absolute branch.  */
     bfd_boolean jumpabsolute;
 
-    /* Has MMX register operands.  */
-    bfd_boolean has_regmmx;
-
-    /* Has XMM register operands.  */
-    bfd_boolean has_regxmm;
-
-    /* Has YMM register operands.  */
-    bfd_boolean has_regymm;
-
-    /* Has ZMM register operands.  */
-    bfd_boolean has_regzmm;
-
-    /* Has TMM register operands.  */
-    bfd_boolean has_regtmm;
+    /* Extended states.  */
+    enum
+      {
+	/* Use MMX state.  */
+	xstate_mmx = 1 << 0,
+	/* Use XMM state.  */
+	xstate_xmm = 1 << 1,
+	/* Use YMM state.  */
+	xstate_ymm = 1 << 2 | xstate_xmm,
+	/* Use ZMM state.  */
+	xstate_zmm = 1 << 3 | xstate_ymm,
+	/* Use TMM state.  */
+	xstate_tmm = 1 << 4
+      } xstate;
 
     /* Has GOTPC or TLS relocation.  */
     bfd_boolean has_gotpc_tls_reloc;
@@ -4864,9 +4864,32 @@ md_assemble (char *line)
   if (!process_suffix ())
     return;
 
-  /* Update operand types.  */
+  /* Update operand types and check extended states.  */
   for (j = 0; j < i.operands; j++)
-    i.types[j] = operand_type_and (i.types[j], i.tm.operand_types[j]);
+    {
+      i.types[j] = operand_type_and (i.types[j], i.tm.operand_types[j]);
+      switch (i.tm.operand_types[j].bitfield.class)
+	{
+	default:
+	  break;
+	case RegMMX:
+	  i.xstate |= xstate_mmx;
+	  break;
+	case RegMask:
+	  i.xstate |= xstate_zmm;
+	  break;
+	case RegSIMD:
+	  if (i.tm.operand_types[j].bitfield.tmmword)
+	    i.xstate |= xstate_tmm;
+	  else if (i.tm.operand_types[j].bitfield.zmmword)
+	    i.xstate |= xstate_zmm;
+	  else if (i.tm.operand_types[j].bitfield.ymmword)
+	    i.xstate |= xstate_ymm;
+	  else if (i.tm.operand_types[j].bitfield.xmmword)
+	    i.xstate |= xstate_xmm;
+	  break;
+	}
+    }
 
   /* Make still unresolved immediate matches conform to size of immediate
      given in i.suffix.  */
@@ -7958,24 +7981,6 @@ build_modrm_byte (void)
 	{
 	  i.rm.reg = i.op[dest].regs->reg_num;
 	  i.rm.regmem = i.op[source].regs->reg_num;
-	  if (i.op[dest].regs->reg_type.bitfield.class == RegMMX
-	       || i.op[source].regs->reg_type.bitfield.class == RegMMX)
-	    i.has_regmmx = TRUE;
-	  else if (i.op[dest].regs->reg_type.bitfield.class == RegSIMD
-		   || i.op[source].regs->reg_type.bitfield.class == RegSIMD)
-	    {
-	      if (i.types[dest].bitfield.tmmword
-		  || i.types[source].bitfield.tmmword)
-		i.has_regtmm = TRUE;
-	      else if (i.types[dest].bitfield.zmmword
-		       || i.types[source].bitfield.zmmword)
-		i.has_regzmm = TRUE;
-	      else if (i.types[dest].bitfield.ymmword
-		       || i.types[source].bitfield.ymmword)
-		i.has_regymm = TRUE;
-	      else
-		i.has_regxmm = TRUE;
-	    }
 	  set_rex_vrex (i.op[dest].regs, REX_R, i.tm.opcode_modifier.sse2avx);
 	  set_rex_vrex (i.op[source].regs, REX_B, FALSE);
 	}
@@ -8319,33 +8324,16 @@ build_modrm_byte (void)
 	  unsigned int vex_reg = ~0;
 
 	  for (op = 0; op < i.operands; op++)
-	    {
-	      if (i.types[op].bitfield.class == Reg
-		  || i.types[op].bitfield.class == RegBND
-		  || i.types[op].bitfield.class == RegMask
-		  || i.types[op].bitfield.class == SReg
-		  || i.types[op].bitfield.class == RegCR
-		  || i.types[op].bitfield.class == RegDR
-		  || i.types[op].bitfield.class == RegTR)
-		break;
-	      if (i.types[op].bitfield.class == RegSIMD)
-		{
-		  if (i.types[op].bitfield.tmmword)
-		    i.has_regtmm = TRUE;
-		  else if (i.types[op].bitfield.zmmword)
-		    i.has_regzmm = TRUE;
-		  else if (i.types[op].bitfield.ymmword)
-		    i.has_regymm = TRUE;
-		  else
-		    i.has_regxmm = TRUE;
-		  break;
-		}
-	      if (i.types[op].bitfield.class == RegMMX)
-		{
-		  i.has_regmmx = TRUE;
-		  break;
-		}
-	    }
+	    if (i.types[op].bitfield.class == Reg
+		|| i.types[op].bitfield.class == RegBND
+		|| i.types[op].bitfield.class == RegMask
+		|| i.types[op].bitfield.class == SReg
+		|| i.types[op].bitfield.class == RegCR
+		|| i.types[op].bitfield.class == RegDR
+		|| i.types[op].bitfield.class == RegTR
+		|| i.types[op].bitfield.class == RegSIMD
+		|| i.types[op].bitfield.class == RegMMX)
+	      break;
 
 	  if (vex_3_sources)
 	    op = dest;
@@ -9177,22 +9165,15 @@ output_insn (void)
 	  || i.tm.cpu_flags.bitfield.cpu687
 	  || i.tm.cpu_flags.bitfield.cpufisttp)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;
-      if (i.has_regmmx
+      if ((i.xstate & xstate_mmx)
 	  || i.tm.base_opcode == 0xf77 /* emms */
-	  || i.tm.base_opcode == 0xf0e /* femms */
-	  || i.tm.base_opcode == 0xf2a /* cvtpi2ps */
-	  || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)
+	  || i.tm.base_opcode == 0xf0e /* femms */)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_MMX;
-      if (i.has_regxmm)
+      if ((i.xstate & xstate_xmm))
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_XMM;
-      if (i.has_regymm
-	  || (i.has_regxmm
-	      && (i.tm.opcode_modifier.vex
-		  || i.tm.opcode_modifier.evex)))
+      if ((i.xstate & xstate_ymm) == xstate_ymm)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_YMM;
-      if (i.has_regzmm
-	  || ((i.has_regxmm || i.has_regymm)
-	      && i.tm.opcode_modifier.evex))
+      if ((i.xstate & xstate_zmm) == xstate_zmm)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_ZMM;
       if (i.tm.cpu_flags.bitfield.cpufxsr)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_FXSR;
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index eabc09893f..ab5620997f 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -624,6 +624,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
 	run_dump_test "property-3"
 	run_dump_test "property-4"
 	run_dump_test "property-5"
+	run_dump_test "property-6"
 
 	if {[istarget "*-*-linux*"]} then {
 	    run_dump_test "align-branch-3"
@@ -1215,6 +1216,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 	run_dump_test "x86-64-property-3"
 	run_dump_test "x86-64-property-4"
 	run_dump_test "x86-64-property-5"
+	run_dump_test "x86-64-property-6"
 
 	if {[istarget "*-*-linux*"]} then {
 	    run_dump_test "x86-64-align-branch-3"
diff --git a/gas/testsuite/gas/i386/property-6.d b/gas/testsuite/gas/i386/property-6.d
new file mode 100644
index 0000000000..cf175c5357
--- /dev/null
+++ b/gas/testsuite/gas/i386/property-6.d
@@ -0,0 +1,9 @@
+#name: i386 property 6
+#as: -mx86-used-note=yes --generate-missing-build-notes=no
+#readelf: -n
+
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: x86 ISA used: AVX512F
+	x86 feature used: x86, XMM, YMM, ZMM
diff --git a/gas/testsuite/gas/i386/property-6.s b/gas/testsuite/gas/i386/property-6.s
new file mode 100644
index 0000000000..572b58a24f
--- /dev/null
+++ b/gas/testsuite/gas/i386/property-6.s
@@ -0,0 +1,2 @@
+	.text
+	kmovw	%k1, %k2
diff --git a/gas/testsuite/gas/i386/x86-64-property-6.d b/gas/testsuite/gas/i386/x86-64-property-6.d
new file mode 100644
index 0000000000..862d4c3536
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-property-6.d
@@ -0,0 +1,10 @@
+#name: x86-64 property 6
+#source: property-6.s
+#as: -mx86-used-note=yes --generate-missing-build-notes=no
+#readelf: -n
+
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: x86 ISA used: AVX512F
+	x86 feature used: x86, XMM, YMM, ZMM

Patch

From be17fba29c2997d8df792621b1193621fd2ea7f0 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 10 Jul 2020 06:15:10 -0700
Subject: [PATCH] x86: Extract extended states from instruction template

Extract extended states from operand types in instruction template.  Set
xstate_zmm for master register move.

	* config/tc-i386.c (_i386_insn): Remove has_regmmx, has_regxmm,
	has_regymm, has_regzmm and has_regtmm.  Add xstate.
	(md_assemble): Set i.xstate from operand types in instruction
	template.
	(build_modrm_byte): Updated.
	(output_insn): Check i.xstate.
	* testsuite/gas/i386/property-3.s: Add mask register move.
	* testsuite/gas/i386/property-3.d: Updated.
	* testsuite/gas/i386/x86-64-property-3.d: Likewise.
---
 gas/config/tc-i386.c                       | 127 +++++++++------------
 gas/testsuite/gas/i386/property-3.d        |   4 +-
 gas/testsuite/gas/i386/property-3.s        |   1 +
 gas/testsuite/gas/i386/x86-64-property-3.d |   4 +-
 4 files changed, 59 insertions(+), 77 deletions(-)

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 0e4291499b..0eb2b94e04 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -362,20 +362,20 @@  struct _i386_insn
     /* The operand to a branch insn indicates an absolute branch.  */
     bfd_boolean jumpabsolute;
 
-    /* Has MMX register operands.  */
-    bfd_boolean has_regmmx;
-
-    /* Has XMM register operands.  */
-    bfd_boolean has_regxmm;
-
-    /* Has YMM register operands.  */
-    bfd_boolean has_regymm;
-
-    /* Has ZMM register operands.  */
-    bfd_boolean has_regzmm;
-
-    /* Has TMM register operands.  */
-    bfd_boolean has_regtmm;
+    /* Extended states.  */
+    enum
+      {
+	/* Use MMX state.  */
+	xstate_mmx = 1 << 0,
+	/* Use XMM state.  */
+	xstate_xmm = 1 << 1,
+	/* Use YMM state.  */
+	xstate_ymm = 1 << 2 | xstate_xmm,
+	/* Use ZMM state.  */
+	xstate_zmm = 1 << 3 | xstate_ymm,
+	/* Use TMM state.  */
+	xstate_tmm = 1 << 4
+      } xstate;
 
     /* Has GOTPC or TLS relocation.  */
     bfd_boolean has_gotpc_tls_reloc;
@@ -4864,9 +4864,32 @@  md_assemble (char *line)
   if (!process_suffix ())
     return;
 
-  /* Update operand types.  */
+  /* Update operand types and check extended states.  */
   for (j = 0; j < i.operands; j++)
-    i.types[j] = operand_type_and (i.types[j], i.tm.operand_types[j]);
+    {
+      i.types[j] = operand_type_and (i.types[j], i.tm.operand_types[j]);
+      switch (i.tm.operand_types[j].bitfield.class)
+	{
+	default:
+	  break;
+	case RegMMX:
+	  i.xstate |= xstate_mmx;
+	  break;
+	case RegMask:
+	  i.xstate |= xstate_zmm;
+	  break;
+	case RegSIMD:
+	  if (i.tm.operand_types[j].bitfield.tmmword)
+	    i.xstate |= xstate_tmm;
+	  else if (i.tm.operand_types[j].bitfield.zmmword)
+	    i.xstate |= xstate_zmm;
+	  else if (i.tm.operand_types[j].bitfield.ymmword)
+	    i.xstate |= xstate_ymm;
+	  else if (i.tm.operand_types[j].bitfield.xmmword)
+	    i.xstate |= xstate_xmm;
+	  break;
+	}
+    }
 
   /* Make still unresolved immediate matches conform to size of immediate
      given in i.suffix.  */
@@ -7958,24 +7981,6 @@  build_modrm_byte (void)
 	{
 	  i.rm.reg = i.op[dest].regs->reg_num;
 	  i.rm.regmem = i.op[source].regs->reg_num;
-	  if (i.op[dest].regs->reg_type.bitfield.class == RegMMX
-	       || i.op[source].regs->reg_type.bitfield.class == RegMMX)
-	    i.has_regmmx = TRUE;
-	  else if (i.op[dest].regs->reg_type.bitfield.class == RegSIMD
-		   || i.op[source].regs->reg_type.bitfield.class == RegSIMD)
-	    {
-	      if (i.types[dest].bitfield.tmmword
-		  || i.types[source].bitfield.tmmword)
-		i.has_regtmm = TRUE;
-	      else if (i.types[dest].bitfield.zmmword
-		       || i.types[source].bitfield.zmmword)
-		i.has_regzmm = TRUE;
-	      else if (i.types[dest].bitfield.ymmword
-		       || i.types[source].bitfield.ymmword)
-		i.has_regymm = TRUE;
-	      else
-		i.has_regxmm = TRUE;
-	    }
 	  set_rex_vrex (i.op[dest].regs, REX_R, i.tm.opcode_modifier.sse2avx);
 	  set_rex_vrex (i.op[source].regs, REX_B, FALSE);
 	}
@@ -8319,33 +8324,16 @@  build_modrm_byte (void)
 	  unsigned int vex_reg = ~0;
 
 	  for (op = 0; op < i.operands; op++)
-	    {
-	      if (i.types[op].bitfield.class == Reg
-		  || i.types[op].bitfield.class == RegBND
-		  || i.types[op].bitfield.class == RegMask
-		  || i.types[op].bitfield.class == SReg
-		  || i.types[op].bitfield.class == RegCR
-		  || i.types[op].bitfield.class == RegDR
-		  || i.types[op].bitfield.class == RegTR)
-		break;
-	      if (i.types[op].bitfield.class == RegSIMD)
-		{
-		  if (i.types[op].bitfield.tmmword)
-		    i.has_regtmm = TRUE;
-		  else if (i.types[op].bitfield.zmmword)
-		    i.has_regzmm = TRUE;
-		  else if (i.types[op].bitfield.ymmword)
-		    i.has_regymm = TRUE;
-		  else
-		    i.has_regxmm = TRUE;
-		  break;
-		}
-	      if (i.types[op].bitfield.class == RegMMX)
-		{
-		  i.has_regmmx = TRUE;
-		  break;
-		}
-	    }
+	    if (i.types[op].bitfield.class == Reg
+		|| i.types[op].bitfield.class == RegBND
+		|| i.types[op].bitfield.class == RegMask
+		|| i.types[op].bitfield.class == SReg
+		|| i.types[op].bitfield.class == RegCR
+		|| i.types[op].bitfield.class == RegDR
+		|| i.types[op].bitfield.class == RegTR
+		|| i.types[op].bitfield.class == RegSIMD
+		|| i.types[op].bitfield.class == RegMMX)
+	      break;
 
 	  if (vex_3_sources)
 	    op = dest;
@@ -9177,22 +9165,15 @@  output_insn (void)
 	  || i.tm.cpu_flags.bitfield.cpu687
 	  || i.tm.cpu_flags.bitfield.cpufisttp)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_X87;
-      if (i.has_regmmx
+      if ((i.xstate & xstate_mmx)
 	  || i.tm.base_opcode == 0xf77 /* emms */
-	  || i.tm.base_opcode == 0xf0e /* femms */
-	  || i.tm.base_opcode == 0xf2a /* cvtpi2ps */
-	  || i.tm.base_opcode == 0x660f2a /* cvtpi2pd */)
+	  || i.tm.base_opcode == 0xf0e /* femms */)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_MMX;
-      if (i.has_regxmm)
+      if ((i.xstate & xstate_xmm))
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_XMM;
-      if (i.has_regymm
-	  || (i.has_regxmm
-	      && (i.tm.opcode_modifier.vex
-		  || i.tm.opcode_modifier.evex)))
+      if ((i.xstate & xstate_ymm) == xstate_ymm)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_YMM;
-      if (i.has_regzmm
-	  || ((i.has_regxmm || i.has_regymm)
-	      && i.tm.opcode_modifier.evex))
+      if ((i.xstate & xstate_zmm) == xstate_zmm)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_ZMM;
       if (i.tm.cpu_flags.bitfield.cpufxsr)
 	x86_feature_2_used |= GNU_PROPERTY_X86_FEATURE_2_FXSR;
diff --git a/gas/testsuite/gas/i386/property-3.d b/gas/testsuite/gas/i386/property-3.d
index 36d215584e..477ebbb4d9 100644
--- a/gas/testsuite/gas/i386/property-3.d
+++ b/gas/testsuite/gas/i386/property-3.d
@@ -5,5 +5,5 @@ 
 Displaying notes found in: .note.gnu.property
 [ 	]+Owner[ 	]+Data size[ 	]+Description
   GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
-      Properties: x86 ISA used: SSE
-	x86 feature used: x86, MMX, XMM
+      Properties: x86 ISA used: SSE, AVX512F
+	x86 feature used: x86, MMX, XMM, YMM, ZMM
diff --git a/gas/testsuite/gas/i386/property-3.s b/gas/testsuite/gas/i386/property-3.s
index c42bdcbcdc..5e03d4ce1e 100644
--- a/gas/testsuite/gas/i386/property-3.s
+++ b/gas/testsuite/gas/i386/property-3.s
@@ -1,2 +1,3 @@ 
 	.text
 	cvtpi2ps (%eax), %xmm0
+	kmovw	%k1, %k2
diff --git a/gas/testsuite/gas/i386/x86-64-property-3.d b/gas/testsuite/gas/i386/x86-64-property-3.d
index aa116e0fbc..190e8bfc62 100644
--- a/gas/testsuite/gas/i386/x86-64-property-3.d
+++ b/gas/testsuite/gas/i386/x86-64-property-3.d
@@ -6,5 +6,5 @@ 
 Displaying notes found in: .note.gnu.property
 [ 	]+Owner[ 	]+Data size[ 	]+Description
   GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
-      Properties: x86 ISA used: SSE
-	x86 feature used: x86, MMX, XMM
+      Properties: x86 ISA used: SSE, AVX512F
+	x86 feature used: x86, MMX, XMM, YMM, ZMM
-- 
2.26.2