[PATCH 4/7] x86: improve match_template()'s diagnostics

Jan Beulich jbeulich@suse.com
Tue Aug 16 07:32:03 GMT 2022


At the example of

	extractps $0, %xmm0, %xmm0
	insertps $0, %xmm0, %eax

(both having respectively the same mistake of using the wrong kind of
destination register) it is easy to see that current behavior is far
from ideal: The former results in "unsupported instruction" for 32-bit
code simply because the 2nd template we have is a Cpu64 one. Instead we
should aim at emitting the "best" possible error, which will typically
be the one where we passed the largest number of checks. Generalize the
original "specific_error" approach by making it apply to the entire
matching loop, utilizing that line numbers increase as we pass further
checks.
---
As to the inval-tls testcase: Why is KMOV special? Are e.g. VMOV or
other vector insns (legacy or EVEX-encoded) any different? Shouldn't the
use of the respective reloc types be limited to _exactly_ the insns they
are intended to be used with? Furthermore having this check in
match_template() is unhelpful, as the resulting diagnostic isn't aiding
in understanding what's wrong. Template matching should be left alone
here, and the issue be diagnosed later, say directly in md_assemble()
(alongside the various further consistency checks there) or in
process_operands().

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2083,12 +2083,7 @@ operand_size_match (const insn_template
     }
 
   if (!t->opcode_modifier.d)
-    {
-    mismatch:
-      if (!match)
-	i.error = operand_size_mismatch;
-      return match;
-    }
+    return match;
 
   /* Check reverse.  */
   gas_assert ((i.operands >= 2 && i.operands <= 3)
@@ -2105,19 +2100,19 @@ operand_size_match (const insn_template
 
       if (t->operand_types[j].bitfield.class == Reg
 	  && !match_operand_size (t, j, given))
-	goto mismatch;
+	return match;
 
       if (t->operand_types[j].bitfield.class == RegSIMD
 	  && !match_simd_size (t, j, given))
-	goto mismatch;
+	return match;
 
       if (t->operand_types[j].bitfield.instance == Accum
 	  && (!match_operand_size (t, j, given)
 	      || !match_simd_size (t, j, given)))
-	goto mismatch;
+	return match;
 
       if ((i.flags[given] & Operand_Mem) && !match_mem_size (t, j, given))
-	goto mismatch;
+	return match;
     }
 
   return match | MATCH_REVERSE;
@@ -6386,6 +6381,17 @@ VEX_check_encoding (const insn_template
   return 0;
 }
 
+/* Helper function for the progress() macro in match_template().  */
+static INLINE enum i386_error progress (enum i386_error new,
+					enum i386_error last,
+					unsigned int line, unsigned int *line_p)
+{
+  if (line <= *line_p)
+    return last;
+  *line_p = line;
+  return new;
+}
+
 static const insn_template *
 match_template (char mnem_suffix)
 {
@@ -6397,8 +6403,9 @@ match_template (char mnem_suffix)
   i386_opcode_modifier suffix_check;
   i386_operand_type operand_types [MAX_OPERANDS];
   int addr_prefix_disp;
-  unsigned int j, size_match, check_register;
-  enum i386_error specific_error = 0;
+  unsigned int j, size_match, check_register, errline = __LINE__;
+  enum i386_error specific_error = number_of_operands_mismatch;
+#define progress(err) progress(err, specific_error, __LINE__, &errline)
 
 #if MAX_OPERANDS != 5
 # error "MAX_OPERANDS must be 5."
@@ -6436,36 +6443,33 @@ match_template (char mnem_suffix)
 	suffix_check.no_ldsuf = 1;
     }
 
-  /* Must have right number of operands.  */
-  i.error = number_of_operands_mismatch;
-
   for (t = current_templates->start; t < current_templates->end; t++)
     {
       addr_prefix_disp = -1;
       found_reverse_match = 0;
 
+      /* Must have right number of operands.  */
       if (i.operands != t->operands)
 	continue;
 
       /* Check processor support.  */
-      i.error = unsupported;
+      specific_error = progress (unsupported);
       if (cpu_flags_match (t) != CPU_FLAGS_PERFECT_MATCH)
 	continue;
 
       /* Check Pseudo Prefix.  */
-      i.error = unsupported;
       if (t->opcode_modifier.pseudovexprefix
 	  && !(i.vec_encoding == vex_encoding_vex
 	      || i.vec_encoding == vex_encoding_vex3))
 	continue;
 
       /* Check AT&T mnemonic.   */
-      i.error = unsupported_with_intel_mnemonic;
+      specific_error = progress (unsupported_with_intel_mnemonic);
       if (intel_mnemonic && t->opcode_modifier.attmnemonic)
 	continue;
 
       /* Check AT&T/Intel syntax.  */
-      i.error = unsupported_syntax;
+      specific_error = progress (unsupported_syntax);
       if ((intel_syntax && t->opcode_modifier.attsyntax)
 	  || (!intel_syntax && t->opcode_modifier.intelsyntax))
 	continue;
@@ -6491,7 +6495,7 @@ match_template (char mnem_suffix)
 	}
 
       /* Check the suffix.  */
-      i.error = invalid_instruction_suffix;
+      specific_error = progress (invalid_instruction_suffix);
       if ((t->opcode_modifier.no_bsuf && suffix_check.no_bsuf)
 	  || (t->opcode_modifier.no_wsuf && suffix_check.no_wsuf)
 	  || (t->opcode_modifier.no_lsuf && suffix_check.no_lsuf)
@@ -6500,6 +6504,7 @@ match_template (char mnem_suffix)
 	  || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf))
 	continue;
 
+      specific_error = progress (operand_size_mismatch);
       size_match = operand_size_match (t);
       if (!size_match)
 	continue;
@@ -6510,11 +6515,9 @@ match_template (char mnem_suffix)
 
 	 as the case of a missing * on the operand is accepted (perhaps with
 	 a warning, issued further down).  */
+      specific_error = progress (operand_type_mismatch);
       if (i.jumpabsolute && t->opcode_modifier.jump != JUMP_ABSOLUTE)
-	{
-	  i.error = operand_type_mismatch;
-	  continue;
-	}
+	continue;
 
       for (j = 0; j < MAX_OPERANDS; j++)
 	operand_types[j] = t->operand_types[j];
@@ -6522,6 +6525,8 @@ match_template (char mnem_suffix)
       /* In general, don't allow
 	 - 64-bit operands outside of 64-bit mode,
 	 - 32-bit operands on pre-386.  */
+      specific_error = progress (mnem_suffix ? invalid_instruction_suffix
+					     : operand_size_mismatch);
       j = i.imm_operands + (t->operands > i.imm_operands + 1);
       if (((i.suffix == QWORD_MNEM_SUFFIX
 	    && flag_code != CODE_64BIT
@@ -6550,7 +6555,7 @@ match_template (char mnem_suffix)
 	{
 	  if (VEX_check_encoding (t))
 	    {
-	      specific_error = i.error;
+	      specific_error = progress (i.error);
 	      continue;
 	    }
 
@@ -6711,6 +6716,8 @@ match_template (char mnem_suffix)
 						   i.types[1],
 						   operand_types[1])))
 	    {
+	      specific_error = progress (i.error);
+
 	      /* Check if other direction is valid ...  */
 	      if (!t->opcode_modifier.d)
 		continue;
@@ -6735,6 +6742,7 @@ match_template (char mnem_suffix)
 						       operand_types[0])))
 		{
 		  /* Does not match either direction.  */
+		  specific_error = progress (i.error);
 		  continue;
 		}
 	      /* found_reverse_match holds which of D or FloatR
@@ -6773,7 +6781,10 @@ match_template (char mnem_suffix)
 						       operand_types[3],
 						       i.types[4],
 						       operand_types[4]))
-		    continue;
+		    {
+		      specific_error = progress (i.error);
+		      continue;
+		    }
 		  /* Fall through.  */
 		case 4:
 		  overlap3 = operand_type_and (i.types[3], operand_types[3]);
@@ -6788,7 +6799,10 @@ match_template (char mnem_suffix)
 							    operand_types[2],
 							    i.types[3],
 							    operand_types[3])))
-		    continue;
+		    {
+		      specific_error = progress (i.error);
+		      continue;
+		    }
 		  /* Fall through.  */
 		case 3:
 		  overlap2 = operand_type_and (i.types[2], operand_types[2]);
@@ -6803,7 +6817,10 @@ match_template (char mnem_suffix)
 							    operand_types[1],
 							    i.types[2],
 							    operand_types[2])))
-		    continue;
+		    {
+		      specific_error = progress (i.error);
+		      continue;
+		    }
 		  break;
 		}
 	    }
@@ -6814,14 +6831,14 @@ match_template (char mnem_suffix)
       /* Check if vector operands are valid.  */
       if (check_VecOperands (t))
 	{
-	  specific_error = i.error;
+	  specific_error = progress (i.error);
 	  continue;
 	}
 
       /* Check if VEX/EVEX encoding requirements can be satisfied.  */
       if (VEX_check_encoding (t))
 	{
-	  specific_error = i.error;
+	  specific_error = progress (i.error);
 	  continue;
 	}
 
@@ -6829,11 +6846,13 @@ match_template (char mnem_suffix)
       break;
     }
 
+#undef progress
+
   if (t == current_templates->end)
     {
       /* We found no match.  */
       const char *err_msg;
-      switch (specific_error ? specific_error : i.error)
+      switch (specific_error)
 	{
 	default:
 	  abort ();
--- a/gas/testsuite/gas/i386/inval-tls.l
+++ b/gas/testsuite/gas/i386/inval-tls.l
@@ -1,3 +1,3 @@
 .*: Assembler messages:
-.*:3: Error: operand size mismatch for `kmovd'
-.*:4: Error: operand size mismatch for `kmovd'
+.*:3: Error: .* `kmovd'
+.*:4: Error: .* `kmovd'
--- a/gas/testsuite/gas/i386/noavx512-1.l
+++ b/gas/testsuite/gas/i386/noavx512-1.l
@@ -1,14 +1,14 @@
 .*: Assembler messages:
-.*:25: Error: .*unsupported instruction.*
+.*:25: Error: .*operand size mismatch.*
 .*:26: Error: .*unsupported masking.*
 .*:27: Error: .*unsupported masking.*
-.*:47: Error: .*unsupported instruction.*
+.*:47: Error: .*operand size mismatch.*
 .*:48: Error: .*unsupported masking.*
 .*:49: Error: .*unsupported masking.*
 .*:50: Error: .*not supported.*
 .*:51: Error: .*not supported.*
 .*:52: Error: .*not supported.*
-.*:69: Error: .*unsupported instruction.*
+.*:69: Error: .*operand size mismatch.*
 .*:70: Error: .*unsupported masking.*
 .*:71: Error: .*unsupported masking.*
 .*:72: Error: .*not supported.*
@@ -17,7 +17,7 @@
 .*:75: Error: .*not supported.*
 .*:76: Error: .*not supported.*
 .*:77: Error: .*not supported.*
-.*:91: Error: .*unsupported instruction.*
+.*:91: Error: .*operand size mismatch.*
 .*:92: Error: .*unsupported masking.*
 .*:93: Error: .*unsupported masking.*
 .*:94: Error: .*not supported.*
@@ -27,7 +27,7 @@
 .*:98: Error: .*not supported.*
 .*:99: Error: .*not supported.*
 .*:100: Error: .*not supported.*
-.*:113: Error: .*unsupported instruction.*
+.*:113: Error: .*operand size mismatch.*
 .*:114: Error: .*unsupported masking.*
 .*:115: Error: .*unsupported masking.*
 .*:116: Error: .*not supported.*
@@ -40,7 +40,7 @@
 .*:126: Error: .*not supported.*
 .*:127: Error: .*not supported.*
 .*:128: Error: .*not supported.*
-.*:135: Error: .*unsupported instruction.*
+.*:135: Error: .*operand size mismatch.*
 .*:136: Error: .*unsupported masking.*
 .*:137: Error: .*unsupported masking.*
 .*:138: Error: .*not supported.*
@@ -54,7 +54,7 @@
 .*:149: Error: .*not supported.*
 .*:150: Error: .*not supported.*
 .*:151: Error: .*not supported.*
-.*:157: Error: .*unsupported instruction.*
+.*:157: Error: .*operand size mismatch.*
 .*:158: Error: .*unsupported masking.*
 .*:159: Error: .*unsupported masking.*
 .*:160: Error: .*not supported.*
--- a/gas/testsuite/gas/i386/noavx512-2.l
+++ b/gas/testsuite/gas/i386/noavx512-2.l
@@ -1,12 +1,12 @@
 .*: Assembler messages:
-.*:26: Error: .*unsupported instruction.*
-.*:27: Error: .*unsupported instruction.*
+.*:26: Error: .*unsupported masking.*
+.*:27: Error: .*unsupported masking.*
 .*:29: Error: .*unsupported instruction.*
 .*:30: Error: .*unsupported instruction.*
 .*:32: Error: .*unsupported instruction.*
 .*:33: Error: .*unsupported instruction.*
-.*:36: Error: .*unsupported instruction.*
-.*:37: Error: .*unsupported instruction.*
+.*:36: Error: .*unsupported masking.*
+.*:37: Error: .*unsupported masking.*
 .*:39: Error: .*unsupported instruction.*
 .*:40: Error: .*unsupported instruction.*
 .*:43: Error: .*unsupported instruction.*
--- a/gas/testsuite/gas/i386/x86-64-branch-4.l
+++ b/gas/testsuite/gas/i386/x86-64-branch-4.l
@@ -1,19 +1,19 @@
 .*: Assembler messages:
 .*:2: Error: invalid instruction suffix for `call'
 .*:3: Error: invalid instruction suffix for `call'
-.*:4: Error: operand type mismatch for `jmp'
+.*:4: Error: operand (size|type) mismatch for `jmp'
 .*:5: Error: invalid instruction suffix for `jmp'
 .*:6: Error: invalid instruction suffix for `jmp'
 .*:7: Error: invalid instruction suffix for `ret'
 .*:8: Error: invalid instruction suffix for `ret'
-.*:11: Error: operand type mismatch for `call'
+.*:11: Error: operand (size|type) mismatch for `call'
 .*:12: Error: invalid instruction suffix for `call'
 .*:13: Error: invalid instruction suffix for `call'
-.*:14: Error: operand size mismatch for `call'
-.*:15: Error: operand type mismatch for `jmp'
+.*:14: Error: operand (size|type) mismatch for `call'
+.*:15: Error: operand (size|type) mismatch for `jmp'
 .*:16: Error: invalid instruction suffix for `jmp'
 .*:17: Error: invalid instruction suffix for `jmp'
-.*:18: Error: operand size mismatch for `jmp'
+.*:18: Error: operand (size|type) mismatch for `jmp'
 .*:19: Error: invalid instruction suffix for `ret'
 .*:20: Error: invalid instruction suffix for `ret'
 GAS LISTING .*
--- a/gas/testsuite/gas/i386/x86-64-branch-5.l
+++ b/gas/testsuite/gas/i386/x86-64-branch-5.l
@@ -1,19 +1,19 @@
 .*: Assembler messages:
-.*:2: Error: unsupported syntax for `lcall'
-.*:3: Error: unsupported syntax for `lfs'
-.*:4: Error: unsupported syntax for `lfs'
-.*:5: Error: unsupported syntax for `lgs'
-.*:6: Error: unsupported syntax for `lgs'
-.*:7: Error: unsupported syntax for `ljmp'
-.*:8: Error: unsupported syntax for `lss'
-.*:9: Error: unsupported syntax for `lss'
-.*:12: Error: unsupported syntax for `call'
-.*:13: Error: unsupported syntax for `lfs'
-.*:14: Error: unsupported syntax for `lfs'
-.*:15: Error: unsupported syntax for `lgs'
-.*:16: Error: unsupported syntax for `lgs'
-.*:17: Error: unsupported syntax for `jmp'
-.*:18: Error: unsupported syntax for `lss'
-.*:19: Error: unsupported syntax for `lss'
+.*:2: Error: invalid instruction suffix for `lcall'
+.*:3: Error: operand size mismatch for `lfs'
+.*:4: Error: invalid instruction suffix for `lfs'
+.*:5: Error: operand size mismatch for `lgs'
+.*:6: Error: invalid instruction suffix for `lgs'
+.*:7: Error: invalid instruction suffix for `ljmp'
+.*:8: Error: operand size mismatch for `lss'
+.*:9: Error: invalid instruction suffix for `lss'
+.*:12: Error: operand (size|type) mismatch for `call'
+.*:13: Error: operand size mismatch for `lfs'
+.*:14: Error: operand size mismatch for `lfs'
+.*:15: Error: operand size mismatch for `lgs'
+.*:16: Error: operand size mismatch for `lgs'
+.*:17: Error: operand (size|type) mismatch for `jmp'
+.*:18: Error: operand size mismatch for `lss'
+.*:19: Error: operand size mismatch for `lss'
 GAS LISTING .*
 #pass
--- a/gas/testsuite/gas/i386/x86-64-inval-tls.l
+++ b/gas/testsuite/gas/i386/x86-64-inval-tls.l
@@ -1,3 +1,3 @@
 .*: Assembler messages:
-.*:3: Error: operand size mismatch for `kmovq'
-.*:4: Error: operand size mismatch for `kmovq'
+.*:3: Error: .* `kmovq'
+.*:4: Error: .* `kmovq'



More information about the Binutils mailing list