From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Subject: [PATCH 4/7] x86: improve match_template()'s diagnostics
Date: Tue, 16 Aug 2022 09:32:03 +0200 [thread overview]
Message-ID: <0ac5121d-5318-b065-57c6-b94085ed6b23@suse.com> (raw)
In-Reply-To: <32216291-fd1f-4579-87de-d24cb7190894@suse.com>
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'
next prev parent reply other threads:[~2022-08-16 7:32 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-16 7:27 [PATCH 0/7] x86: suffix handling changes Jan Beulich
2022-08-16 7:30 ` [PATCH 1/7] x86/Intel: restrict suffix derivation Jan Beulich
2022-08-17 19:19 ` H.J. Lu
2022-08-18 6:07 ` Jan Beulich
2022-08-18 14:46 ` H.J. Lu
2022-08-19 8:19 ` Jan Beulich
2022-08-19 14:23 ` H.J. Lu
2022-08-19 14:49 ` Jan Beulich
2022-08-19 17:00 ` H.J. Lu
2022-08-22 9:34 ` Jan Beulich
2022-08-22 14:38 ` H.J. Lu
2022-08-16 7:30 ` [PATCH 2/7] x86: insert "no error" enumerator in i386_error enumeration Jan Beulich
2022-08-17 19:19 ` H.J. Lu
2022-08-16 7:31 ` [PATCH 3/7] x86: move / quiesce pre-386 non-16-bit warning Jan Beulich
2022-08-17 19:21 ` H.J. Lu
2022-08-18 7:21 ` Jan Beulich
2022-08-18 15:30 ` H.J. Lu
2022-08-19 6:13 ` Jan Beulich
2022-08-19 14:18 ` H.J. Lu
2022-08-16 7:32 ` Jan Beulich [this message]
2022-08-17 20:24 ` [PATCH 4/7] x86: improve match_template()'s diagnostics H.J. Lu
2022-08-18 6:14 ` Jan Beulich
2022-08-18 14:51 ` H.J. Lu
2022-08-16 7:32 ` [PATCH 5/7] x86: re-work insn/suffix recognition Jan Beulich
2022-08-17 20:29 ` H.J. Lu
2022-08-18 6:24 ` Jan Beulich
2022-08-18 15:14 ` H.J. Lu
2022-08-19 8:28 ` Jan Beulich
2022-08-23 2:00 ` H.J. Lu
2022-08-26 9:26 ` Jan Beulich
2022-08-26 18:46 ` H.J. Lu
2022-09-06 6:40 ` Jan Beulich
2022-09-06 21:53 ` H.J. Lu
2022-09-07 7:17 ` Jan Beulich
2022-09-26 23:52 ` H.J. Lu
2022-09-28 12:49 ` Jan Beulich
2022-09-28 19:33 ` H.J. Lu
2022-09-29 8:08 ` Jan Beulich
2022-09-29 16:00 ` H.J. Lu
2022-09-29 16:06 ` Jan Beulich
2022-09-29 16:20 ` H.J. Lu
2022-08-16 7:33 ` [PATCH 6/7] x86-64: further re-work insn/suffix recognition to also cover MOVSL Jan Beulich
2022-08-16 7:34 ` [PATCH 7/7] ix86: don't recognize/derive Q suffix in the common case Jan Beulich
2022-08-17 20:36 ` H.J. Lu
2022-08-18 6:29 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0ac5121d-5318-b065-57c6-b94085ed6b23@suse.com \
--to=jbeulich@suse.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).