[PATCH 0/3] x86: assorted tidying of opcode/operand handling

public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed

* [PATCH 0/3] x86: assorted tidying of opcode/operand handling
@ 2023-01-20  9:58 Jan Beulich
  2023-01-20 10:00 ` [PATCH 1/3] x86: use ModR/M for FPU insns with operands Jan Beulich
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jan Beulich @ 2023-01-20  9:58 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

The only connection between these, besides perhaps contextual dependencies,
is that I've spotted the latter two oddities while working towards patch 1.

1: use ModR/M for FPU insns with operands
2: drop dead SSE2AVX-related code
3: move reg_operands adjustment

Jan

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

* [PATCH 1/3] x86: use ModR/M for FPU insns with operands
  2023-01-20  9:58 [PATCH 0/3] x86: assorted tidying of opcode/operand handling Jan Beulich
@ 2023-01-20 10:00 ` Jan Beulich
  2023-01-20 10:00 ` [PATCH 2/3] x86: drop dead SSE2AVX-related code Jan Beulich
  2023-01-20 10:00 ` [PATCH 3/3] x86: move reg_operands adjustment Jan Beulich
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2023-01-20 10:00 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

This is the correct way of expressing things; encoding the ModR/M byte
directly in base_opcode has always been bogus.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6995,8 +6995,10 @@ match_template (char mnem_suffix)
 		{
 		  if (t->opcode_modifier.operandconstraint != UGH)
 		    found_reverse_match = Opcode_FloatD;
+		  else
+		    found_reverse_match = ~0;
 		  /* FSUB{,R} and FDIV{,R} may need a 2nd bit flipped.  */
-		  if ((t->base_opcode & 0x20)
+		  if ((t->extension_opcode & 4)
 		      && (intel_syntax || intel_mnemonic))
 		    found_reverse_match |= Opcode_FloatR;
 		}
@@ -7131,6 +7133,12 @@ match_template (char mnem_suffix)
     case 0:
       break;
 
+    case Opcode_FloatR:
+    case Opcode_FloatR | Opcode_FloatD:
+      i.tm.extension_opcode ^= Opcode_FloatR >> 3;
+      found_reverse_match &= Opcode_FloatD;
+
+      /* Fall through.  */
     default:
       /* If we found a reverse match we must alter the opcode direction
 	 bit and clear/flip the regmem modifier one.  found_reverse_match
@@ -8027,10 +8035,15 @@ process_operands (void)
 	 process_immext ();
     }
   else if (i.tm.operand_types[0].bitfield.instance == Accum
-	   && i.tm.operand_types[0].bitfield.xmmword)
+	   && i.tm.opcode_modifier.modrm)
     {
       unsigned int j;
 
+      /* This needs to account for the adjustment already done ahead of
+	 calling process_operands().  */
+      if (i.tm.operand_types[0].bitfield.xmmword)
+	i.reg_operands--;
+
       for (j = 1; j < i.operands; j++)
 	{
 	  i.op[j - 1] = i.op[j];
@@ -8044,7 +8057,6 @@ process_operands (void)
 	}
 
       i.operands--;
-      i.reg_operands--;
       i.tm.operands--;
     }
   else if (i.tm.opcode_modifier.operandconstraint == IMPLICIT_QUAD_GROUP)
@@ -8092,6 +8104,24 @@ process_operands (void)
 	 index base bytes based on all the info we've collected.  */
 
       default_seg = build_modrm_byte ();
+
+      if (!quiet_warnings && i.tm.opcode_modifier.operandconstraint == UGH)
+	{
+	  /* Warn about some common errors, but press on regardless.  */
+	  if (i.operands == 2)
+	    {
+	      /* Reversed arguments on faddp or fmulp.  */
+	      as_warn (_("translating to `%s %s%s,%s%s'"), insn_name (&i.tm),
+		       register_prefix, i.op[!intel_syntax].regs->reg_name,
+		       register_prefix, i.op[intel_syntax].regs->reg_name);
+	    }
+	  else if (i.tm.opcode_modifier.mnemonicsize == IGNORESIZE)
+	    {
+	      /* Extraneous `l' suffix on fp insn.  */
+	      as_warn (_("translating to `%s %s%s'"), insn_name (&i.tm),
+		       register_prefix, i.op[0].regs->reg_name);
+	    }
+	}
     }
   else if (i.types[0].bitfield.class == SReg)
     {
@@ -8126,8 +8156,7 @@ process_operands (void)
     }
   else if (i.short_form)
     {
-      /* The register or float register operand is in operand
-	 0 or 1.  */
+      /* The register operand is in operand 0 or 1.  */
       const reg_entry *r = i.op[0].regs;
 
       if (i.imm_operands
@@ -8137,23 +8166,6 @@ process_operands (void)
       i.tm.base_opcode |= r->reg_num;
       if ((r->reg_flags & RegRex) != 0)
 	i.rex |= REX_B;
-      if (!quiet_warnings && i.tm.opcode_modifier.operandconstraint == UGH)
-	{
-	  /* Warn about some common errors, but press on regardless.  */
-	  if (i.operands != 2)
-	    {
-	      /* Extraneous `l' suffix on fp insn.  */
-	      as_warn (_("translating to `%s %s%s'"), insn_name (&i.tm),
-		       register_prefix, i.op[0].regs->reg_name);
-	    }
-	  else if (i.op[0].regs->reg_type.bitfield.instance != Accum)
-	    {
-	      /* Reversed arguments on faddp or fmulp.  */
-	      as_warn (_("translating to `%s %s%s,%s%s'"), insn_name (&i.tm),
-		       register_prefix, i.op[!intel_syntax].regs->reg_name,
-		       register_prefix, i.op[intel_syntax].regs->reg_name);
-	    }
-	}
     }
 
   if ((i.seg[0] || i.prefix[SEG_PREFIX])
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -931,8 +931,8 @@ typedef struct insn_template
 #define Opcode_D	0x2 /* Direction bit:
 			       set if Reg --> Regmem;
 			       unset if Regmem --> Reg. */
-#define Opcode_FloatR	0x8 /* Bit to swap src/dest for float insns. */
-#define Opcode_FloatD 0x400 /* Direction bit for float insns. */
+#define Opcode_FloatR	0x8 /* ModR/M bit to swap src/dest for float insns. */
+#define Opcode_FloatD   0x4 /* Direction bit for float insns. */
 #define Opcode_ExtD	0x1 /* Direction bit for extended opcode space insns. */
 #define Opcode_SIMD_IntD 0x10 /* Direction bit for SIMD int insns. */
 /* The next value is arbitrary, as long as it's non-zero and distinct
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -596,9 +596,9 @@ verw, 0xf00/5, i286, Modrm|IgnoreSize|No
 // Floating point instructions.
 
 // load
-fld, 0xd9c0, FP, NoSuf, { FloatReg }
+fld, 0xd9/0, FP, Modrm|NoSuf, { FloatReg }
 fld, 0xd9/0, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
-fld, 0xd9c0, FP, IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
+fld, 0xd9/0, FP, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
 // Intel Syntax
 fld, 0xdb/5, FP, Modrm|NoSuf, { Tbyte|Unspecified|BaseIndex }
 fild, 0xdf/0, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
@@ -608,15 +608,15 @@ fldt, 0xdb/5, FP, Modrm|NoSuf, { Unspeci
 fbld, 0xdf/4, FP, Modrm|NoSuf, { Tbyte|Unspecified|BaseIndex }
 
 // store (no pop)
-fst, 0xddd0, FP, NoSuf, { FloatReg }
+fst, 0xdd/2, FP, Modrm|NoSuf, { FloatReg }
 fst, 0xd9/2, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
-fst, 0xddd0, FP, IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
+fst, 0xdd/2, FP, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
 fist, 0xdf/2, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
 // store (with pop)
-fstp, 0xddd8, FP, NoSuf, { FloatReg }
+fstp, 0xdd/3, FP, Modrm|NoSuf, { FloatReg }
 fstp, 0xd9/3, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
-fstp, 0xddd8, FP, IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
+fstp, 0xdd/3, FP, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
 // Intel Syntax
 fstp, 0xdb/7, FP, Modrm|NoSuf, { Tbyte|Unspecified|BaseIndex }
 fistp, 0xdf/3, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
@@ -626,32 +626,32 @@ fstpt, 0xdb/7, FP, Modrm|NoSuf, { Unspec
 fbstp, 0xdf/6, FP, Modrm|NoSuf, { Tbyte|Unspecified|BaseIndex }
 
 // exchange %st<n> with %st0
-fxch, 0xd9c8, FP, NoSuf, { FloatReg }
+fxch, 0xd9/1, FP, Modrm|NoSuf, { FloatReg }
 // alias for fxch %st(1)
 fxch, 0xd9c9, FP, NoSuf, {}
 
 // comparison (without pop)
-fcom, 0xd8d0, FP, NoSuf, { FloatReg }
+fcom, 0xd8/2, FP, Modrm|NoSuf, { FloatReg }
 // alias for fcom %st(1)
 fcom, 0xd8d1, FP, NoSuf, {}
 fcom, 0xd8/2, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
-fcom, 0xd8d0, FP, IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
+fcom, 0xd8/2, FP, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
 ficom, 0xde/2, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
 // comparison (with pop)
-fcomp, 0xd8d8, FP, NoSuf, { FloatReg }
+fcomp, 0xd8/3, FP, Modrm|NoSuf, { FloatReg }
 // alias for fcomp %st(1)
 fcomp, 0xd8d9, FP, NoSuf, {}
 fcomp, 0xd8/3, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
-fcomp, 0xd8d8, FP, IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
+fcomp, 0xd8/3, FP, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|Ugh, { FloatReg }
 ficomp, 0xde/3, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 fcompp, 0xded9, FP, NoSuf, {}
 
 // unordered comparison (with pop)
-fucom, 0xdde0, i387, NoSuf, { FloatReg }
+fucom, 0xdd/4, i387, Modrm|NoSuf, { FloatReg }
 // alias for fucom %st(1)
 fucom, 0xdde1, i387, NoSuf, {}
-fucomp, 0xdde8, i387, NoSuf, { FloatReg }
+fucomp, 0xdd/4, i387, Modrm|NoSuf, { FloatReg }
 // alias for fucomp %st(1)
 fucomp, 0xdde9, i387, NoSuf, {}
 fucompp, 0xdae9, i387, NoSuf, {}
@@ -671,94 +671,94 @@ fldz, 0xd9ee, FP, NoSuf, {}
 // Arithmetic.
 
 // add
-fadd, 0xd8c0, FP, D|NoSuf, { FloatReg, FloatAcc }
+fadd, 0xd8/0, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
 // alias for fadd %st(i), %st
-fadd, 0xd8c0, FP, NoSuf, { FloatReg }
+fadd, 0xd8/0, FP, Modrm|NoSuf, { FloatReg }
 // alias for faddp
 fadd, 0xdec1, FP, NoSuf|Ugh|ATTMnemonic, {}
 fadd, 0xd8/0, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fiadd, 0xde/0, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-faddp, 0xdec0, FP, D|NoSuf|Ugh, { FloatAcc, FloatReg }
-faddp, 0xdec0, FP, NoSuf, { FloatReg }
+faddp, 0xde/0, FP, D|Modrm|NoSuf|Ugh, { FloatAcc, FloatReg }
+faddp, 0xde/0, FP, Modrm|NoSuf, { FloatReg }
 // alias for faddp %st, %st(1)
 faddp, 0xdec1, FP, NoSuf, {}
 
 // subtract
-fsub, 0xd8e0, FP, NoSuf, { FloatReg }
-fsub, 0xd8e0, FP, D|NoSuf, { FloatReg, FloatAcc }
+fsub, 0xd8/4, FP, Modrm|NoSuf, { FloatReg }
+fsub, 0xd8/4, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
 // alias for fsubp
 fsub, 0xdee1, FP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
 fsub, 0xdee9, FP, NoSuf|Ugh|ATTMnemonic, {}
 fsub, 0xd8/4, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fisub, 0xde/4, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-fsubp, 0xdee0, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
-fsubp, 0xdee0, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
+fsubp, 0xde/4, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
+fsubp, 0xde/4, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
 fsubp, 0xdee1, FP, NoSuf|ATTMnemonic|ATTSyntax, {}
-fsubp, 0xdee8, FP, NoSuf, { FloatAcc, FloatReg }
-fsubp, 0xdee8, FP, NoSuf, { FloatReg }
+fsubp, 0xde/5, FP, Modrm|NoSuf, { FloatAcc, FloatReg }
+fsubp, 0xde/5, FP, Modrm|NoSuf, { FloatReg }
 fsubp, 0xdee9, FP, NoSuf, {}
 
 // subtract reverse
-fsubr, 0xd8e8, FP, NoSuf, { FloatReg }
-fsubr, 0xd8e8, FP, D|NoSuf, { FloatReg, FloatAcc }
+fsubr, 0xd8/5, FP, Modrm|NoSuf, { FloatReg }
+fsubr, 0xd8/5, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
 // alias for fsubrp
 fsubr, 0xdee9, FP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
 fsubr, 0xdee1, FP, NoSuf|Ugh|ATTMnemonic, {}
 fsubr, 0xd8/5, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fisubr, 0xde/5, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-fsubrp, 0xdee8, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
-fsubrp, 0xdee8, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
+fsubrp, 0xde/5, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
+fsubrp, 0xde/5, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
 fsubrp, 0xdee9, FP, NoSuf|ATTMnemonic|ATTSyntax, {}
-fsubrp, 0xdee0, FP, NoSuf, { FloatAcc, FloatReg }
-fsubrp, 0xdee0, FP, NoSuf, { FloatReg }
+fsubrp, 0xde/4, FP, Modrm|NoSuf, { FloatAcc, FloatReg }
+fsubrp, 0xde/4, FP, Modrm|NoSuf, { FloatReg }
 fsubrp, 0xdee1, FP, NoSuf, {}
 
 // multiply
-fmul, 0xd8c8, FP, D|NoSuf, { FloatReg, FloatAcc }
-fmul, 0xd8c8, FP, NoSuf, { FloatReg }
+fmul, 0xd8/1, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
+fmul, 0xd8/1, FP, Modrm|NoSuf, { FloatReg }
 // alias for fmulp
 fmul, 0xdec9, FP, NoSuf|Ugh|ATTMnemonic, {}
 fmul, 0xd8/1, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fimul, 0xde/1, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-fmulp, 0xdec8, FP, D|NoSuf|Ugh, { FloatAcc, FloatReg }
-fmulp, 0xdec8, FP, NoSuf, { FloatReg }
+fmulp, 0xde/1, FP, D|Modrm|NoSuf|Ugh, { FloatAcc, FloatReg }
+fmulp, 0xde/1, FP, Modrm|NoSuf, { FloatReg }
 // alias for fmulp %st, %st(1)
 fmulp, 0xdec9, FP, NoSuf, {}
 
 // divide
-fdiv, 0xd8f0, FP, NoSuf, { FloatReg }
-fdiv, 0xd8f0, FP, D|NoSuf, { FloatReg, FloatAcc }
+fdiv, 0xd8/6, FP, Modrm|NoSuf, { FloatReg }
+fdiv, 0xd8/6, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
 // alias for fdivp
 fdiv, 0xdef1, FP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
 fdiv, 0xdef9, FP, NoSuf|Ugh|ATTMnemonic, {}
 fdiv, 0xd8/6, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fidiv, 0xde/6, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-fdivp, 0xdef0, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
-fdivp, 0xdef0, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
+fdivp, 0xde/6, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
+fdivp, 0xde/6, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
 fdivp, 0xdef1, FP, NoSuf|ATTMnemonic|ATTSyntax, {}
-fdivp, 0xdef8, FP, NoSuf, { FloatAcc, FloatReg }
-fdivp, 0xdef8, FP, NoSuf, { FloatReg }
+fdivp, 0xde/7, FP, Modrm|NoSuf, { FloatAcc, FloatReg }
+fdivp, 0xde/7, FP, Modrm|NoSuf, { FloatReg }
 fdivp, 0xdef9, FP, NoSuf, {}
 
 // divide reverse
-fdivr, 0xd8f8, FP, NoSuf, { FloatReg }
-fdivr, 0xd8f8, FP, D|NoSuf, { FloatReg, FloatAcc }
+fdivr, 0xd8/7, FP, Modrm|NoSuf, { FloatReg }
+fdivr, 0xd8/7, FP, D|Modrm|NoSuf, { FloatReg, FloatAcc }
 // alias for fdivrp
 fdivr, 0xdef9, FP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
 fdivr, 0xdef1, FP, NoSuf|Ugh|ATTMnemonic, {}
 fdivr, 0xd8/7, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Dword|Qword|Unspecified|BaseIndex }
 fidivr, 0xde/7, FP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf, { Word|Dword|Unspecified|BaseIndex }
 
-fdivrp, 0xdef8, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
-fdivrp, 0xdef8, FP, NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
+fdivrp, 0xde/7, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatAcc, FloatReg }
+fdivrp, 0xde/7, FP, Modrm|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg }
 fdivrp, 0xdef9, FP, NoSuf|ATTMnemonic|ATTSyntax, {}
-fdivrp, 0xdef0, FP, NoSuf, { FloatAcc, FloatReg }
-fdivrp, 0xdef0, FP, NoSuf, { FloatReg }
+fdivrp, 0xde/6, FP, Modrm|NoSuf, { FloatAcc, FloatReg }
+fdivrp, 0xde/6, FP, Modrm|NoSuf, { FloatReg }
 fdivrp, 0xdef1, FP, NoSuf, {}
 
 f2xm1, 0xd9f0, FP, NoSuf, {}
@@ -811,9 +811,9 @@ fnsetpm, 0xdbe4, i287, NoSuf, {}
 fsetpm, 0xdbe4, i287, NoSuf|FWait, {}
 frstpm, 0xdbe5, i287, NoSuf, {}
 
-ffree, 0xddc0, FP, NoSuf, { FloatReg }
+ffree, 0xdd/0, FP, Modrm|NoSuf, { FloatReg }
 // P6:free st(i), pop st
-ffreep, 0xdfc0, i687, NoSuf, { FloatReg }
+ffreep, 0xdf/0, i687, Modrm|NoSuf, { FloatReg }
 fnop, 0xd9d0, FP, NoSuf, {}
 fwait, 0x9b, FP, NoSuf, {}
 
@@ -924,37 +924,37 @@ ud0, 0xfff, i186, Modrm|CheckOperandSize
 
 cmov<cc>, 0xf4<cc:opc>, CMOV, Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 
-fcmovb, 0xdac0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovnae, 0xdac0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmove, 0xdac8, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovbe, 0xdad0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovna, 0xdad0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovu, 0xdad8, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovae, 0xdbc0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovnb, 0xdbc0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovne, 0xdbc8, i687, NoSuf, { FloatReg, FloatAcc }
-fcmova, 0xdbd0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovnbe, 0xdbd0, i687, NoSuf, { FloatReg, FloatAcc }
-fcmovnu, 0xdbd8, i687, NoSuf, { FloatReg, FloatAcc }
+fcmovb, 0xda/0, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovnae, 0xda/0, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmove, 0xda/1, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovbe, 0xda/2, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovna, 0xda/2, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovu, 0xda/3, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovae, 0xdb/0, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovnb, 0xdb/0, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovne, 0xdb/1, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmova, 0xdb/2, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovnbe, 0xdb/2, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
+fcmovnu, 0xdb/3, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 
-fcomi, 0xdbf0, i687, NoSuf, { FloatReg, FloatAcc }
+fcomi, 0xdb/6, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fcomi, 0xdbf1, i687, NoSuf, {}
-fcomi, 0xdbf0, i687, NoSuf, { FloatReg }
-fucomi, 0xdbe8, i687, NoSuf, { FloatReg, FloatAcc }
+fcomi, 0xdb/6, i687, Modrm|NoSuf, { FloatReg }
+fucomi, 0xdb/5, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fucomi, 0xdbe9, i687, NoSuf, {}
-fucomi, 0xdbe8, i687, NoSuf, { FloatReg }
-fcomip, 0xdff0, i687, NoSuf, { FloatReg, FloatAcc }
+fucomi, 0xdb/5, i687, Modrm|NoSuf, { FloatReg }
+fcomip, 0xdf/6, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fcomip, 0xdff1, i687, NoSuf, {}
-fcomip, 0xdff0, i687, NoSuf, { FloatReg }
-fcompi, 0xdff0, i687, NoSuf, { FloatReg, FloatAcc }
+fcomip, 0xdf/6, i687, Modrm|NoSuf, { FloatReg }
+fcompi, 0xdf/6, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fcompi, 0xdff1, i687, NoSuf, {}
-fcompi, 0xdff0, i687, NoSuf, { FloatReg }
-fucomip, 0xdfe8, i687, NoSuf, { FloatReg, FloatAcc }
+fcompi, 0xdf/6, i687, Modrm|NoSuf, { FloatReg }
+fucomip, 0xdf/5, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fucomip, 0xdfe9, i687, NoSuf, {}
-fucomip, 0xdfe8, i687, NoSuf, { FloatReg }
-fucompi, 0xdfe8, i687, NoSuf, { FloatReg, FloatAcc }
+fucomip, 0xdf/5, i687, Modrm|NoSuf, { FloatReg }
+fucompi, 0xdf/5, i687, Modrm|NoSuf, { FloatReg, FloatAcc }
 fucompi, 0xdfe9, i687, NoSuf, {}
-fucompi, 0xdfe8, i687, NoSuf, { FloatReg }
+fucompi, 0xdf/5, i687, Modrm|NoSuf, { FloatReg }
 
 // Pentium4 extensions.
 


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

* [PATCH 2/3] x86: drop dead SSE2AVX-related code
  2023-01-20  9:58 [PATCH 0/3] x86: assorted tidying of opcode/operand handling Jan Beulich
  2023-01-20 10:00 ` [PATCH 1/3] x86: use ModR/M for FPU insns with operands Jan Beulich
@ 2023-01-20 10:00 ` Jan Beulich
  2023-01-20 10:00 ` [PATCH 3/3] x86: move reg_operands adjustment Jan Beulich
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2023-01-20 10:00 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

All (there are just two) SSE2AVX templates with %xmm0 as first operand
also specify VEX3SOURCES. Hence there's no need for an "else" to the
respective if(), and the if() itself can become an assertion.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7965,29 +7965,15 @@ process_operands (void)
       if (i.tm.operand_types[0].bitfield.instance == Accum
 	  && i.tm.operand_types[0].bitfield.xmmword)
 	{
-	  if (i.tm.opcode_modifier.vexsources == VEX3SOURCES)
-	    {
-	      /* Keep xmm0 for instructions with VEX prefix and 3
-		 sources.  */
-	      i.tm.operand_types[0].bitfield.instance = InstanceNone;
-	      i.tm.operand_types[0].bitfield.class = RegSIMD;
-	      goto duplicate;
-	    }
-	  else
-	    {
-	      /* We remove the first xmm0 and keep the number of
-		 operands unchanged, which in fact duplicates the
-		 destination.  */
-	      for (j = 1; j < i.operands; j++)
-		{
-		  i.op[j - 1] = i.op[j];
-		  i.types[j - 1] = i.types[j];
-		  i.tm.operand_types[j - 1] = i.tm.operand_types[j];
-		  i.flags[j - 1] = i.flags[j];
-		}
-	    }
+	  gas_assert (i.tm.opcode_modifier.vexsources == VEX3SOURCES);
+	  /* Keep xmm0 for instructions with VEX prefix and 3
+	     sources.  */
+	  i.tm.operand_types[0].bitfield.instance = InstanceNone;
+	  i.tm.operand_types[0].bitfield.class = RegSIMD;
+	  goto duplicate;
 	}
-      else if (i.tm.opcode_modifier.operandconstraint == IMPLICIT_1ST_XMM0)
+
+      if (i.tm.opcode_modifier.operandconstraint == IMPLICIT_1ST_XMM0)
 	{
 	  gas_assert ((MAX_OPERANDS - 1) > dupl
 		      && (i.tm.opcode_modifier.vexsources


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

* [PATCH 3/3] x86: move reg_operands adjustment
  2023-01-20  9:58 [PATCH 0/3] x86: assorted tidying of opcode/operand handling Jan Beulich
  2023-01-20 10:00 ` [PATCH 1/3] x86: use ModR/M for FPU insns with operands Jan Beulich
  2023-01-20 10:00 ` [PATCH 2/3] x86: drop dead SSE2AVX-related code Jan Beulich
@ 2023-01-20 10:00 ` Jan Beulich
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2023-01-20 10:00 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

Ideally we'd do away with this somewhat questionable adjustment (leaving
i.types[] untouched). That's non-trivial though as it looks, so only
- move the logic into process_operands(), putting it closer to related
  logic and eliminating a conditional for operand-less insns,
- make it consistent (i.e. also affect %xmm0), eliminating an ugly
  special case later in the function.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5317,14 +5317,6 @@ md_assemble (char *line)
   if (i.types[0].bitfield.imm1)
     i.imm_operands = 0;	/* kludge for shift insns.  */
 
-  /* We only need to check those implicit registers for instructions
-     with 3 operands or less.  */
-  if (i.operands <= 3)
-    for (j = 0; j < i.operands; j++)
-      if (i.types[j].bitfield.instance != InstanceNone
-	  && !i.types[j].bitfield.xmmword)
-	i.reg_operands--;
-
   /* For insns with operands there are more diddles to do to the opcode.  */
   if (i.operands)
     {
@@ -7936,6 +7928,13 @@ process_operands (void)
      unnecessary segment overrides.  */
   const reg_entry *default_seg = NULL;
 
+  /* We only need to check those implicit registers for instructions
+     with 3 operands or less.  */
+  if (i.operands <= 3)
+    for (unsigned int j = 0; j < i.operands; j++)
+      if (i.types[j].bitfield.instance != InstanceNone)
+	i.reg_operands--;
+
   if (i.tm.opcode_modifier.sse2avx)
     {
       /* Legacy encoded insns allow explicit REX prefixes, so these prefixes
@@ -7970,6 +7969,7 @@ process_operands (void)
 	     sources.  */
 	  i.tm.operand_types[0].bitfield.instance = InstanceNone;
 	  i.tm.operand_types[0].bitfield.class = RegSIMD;
+	  i.reg_operands++;
 	  goto duplicate;
 	}
 
@@ -8025,11 +8025,6 @@ process_operands (void)
     {
       unsigned int j;
 
-      /* This needs to account for the adjustment already done ahead of
-	 calling process_operands().  */
-      if (i.tm.operand_types[0].bitfield.xmmword)
-	i.reg_operands--;
-
       for (j = 1; j < i.operands; j++)
 	{
 	  i.op[j - 1] = i.op[j];
@@ -8042,6 +8037,8 @@ process_operands (void)
 	  i.flags[j - 1] = i.flags[j];
 	}
 
+      /* No adjustment to i.reg_operands: This was already done at the top
+	 of the function.  */
       i.operands--;
       i.tm.operands--;
     }


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

end of thread, other threads:[~2023-01-20 10:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20  9:58 [PATCH 0/3] x86: assorted tidying of opcode/operand handling Jan Beulich
2023-01-20 10:00 ` [PATCH 1/3] x86: use ModR/M for FPU insns with operands Jan Beulich
2023-01-20 10:00 ` [PATCH 2/3] x86: drop dead SSE2AVX-related code Jan Beulich
2023-01-20 10:00 ` [PATCH 3/3] x86: move reg_operands adjustment 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).