public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: binutils@sourceware.org
Cc: Richard Sandiford <richard.sandiford@arm.com>
Subject: [PATCH 34/43] aarch64: Tweak priorities of parsing-related errors
Date: Thu, 30 Mar 2023 11:23:50 +0100	[thread overview]
Message-ID: <20230330102359.3327695-35-richard.sandiford@arm.com> (raw)
In-Reply-To: <20230330102359.3327695-1-richard.sandiford@arm.com>

There are three main kinds of error reported during parsing,
in increasing order of priority:

- AARCH64_OPDE_RECOVERABLE (register seen instead of immediate)
- AARCH64_OPDE_SYNTAX_ERROR
- AARCH64_OPDE_FATAL_SYNTAX_ERROR

This priority makes sense when comparing errors reported against the
same operand.  But if we get to operand 3 (say) and see a register
instead of an immediate, that's likely to be a better match than
something that fails with a syntax error at operand 1.

The idea of this patch is to prioritise parsing-related errors
based on operand index first, then by error code.  Post-parsing
errors still win over parsing errors, and their relative priorities
don't change.
---
 gas/config/tc-aarch64.c                   | 50 ++++++++++++++++++++---
 gas/testsuite/gas/aarch64/sme-8-illegal.l | 12 +++---
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 1851f83ad05..c8e37623d9e 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -5769,6 +5769,44 @@ output_operand_error_record (const operand_error_record *record, char *str)
     }
 }
 
+/* Return true if the presence of error A against an instruction means
+   that error B should not be reported.  This is only used as a first pass,
+   to pick the kind of error that we should report.  */
+
+static bool
+better_error_p (operand_error_record *a, operand_error_record *b)
+{
+  /* For errors reported during parsing, prefer errors that relate to
+     later operands, since that implies that the earlier operands were
+     syntactically valid.
+
+     For example, if we see a register R instead of an immediate in
+     operand N, we'll report that as a recoverable "immediate operand
+     required" error.  This is because there is often another opcode
+     entry that accepts a register operand N, and any errors about R
+     should be reported against the register forms of the instruction.
+     But if no such register form exists, the recoverable error should
+     still win over a syntax error against operand N-1.
+
+     For these purposes, count an error reported at the end of the
+     assembly string as equivalent to an error reported against the
+     final operand.  This means that opcode entries that expect more
+     operands win over "unexpected characters following instruction".  */
+  if (a->detail.kind <= AARCH64_OPDE_FATAL_SYNTAX_ERROR
+      && b->detail.kind <= AARCH64_OPDE_FATAL_SYNTAX_ERROR)
+    {
+      int a_index = (a->detail.index < 0
+		     ? aarch64_num_of_operands (a->opcode) - 1
+		     : a->detail.index);
+      int b_index = (b->detail.index < 0
+		     ? aarch64_num_of_operands (b->opcode) - 1
+		     : b->detail.index);
+      if (a_index != b_index)
+	return a_index > b_index;
+    }
+  return operand_error_higher_severity_p (a->detail.kind, b->detail.kind);
+}
+
 /* Process and output the error message about the operand mismatching.
 
    When this function is called, the operand error information had
@@ -5787,7 +5825,7 @@ output_operand_error_report (char *str, bool non_fatal_only)
   enum aarch64_operand_error_kind kind;
   operand_error_record *curr;
   operand_error_record *head = operand_error_report.head;
-  operand_error_record *record = NULL;
+  operand_error_record *record;
 
   /* No error to report.  */
   if (head == NULL)
@@ -5811,7 +5849,7 @@ output_operand_error_report (char *str, bool non_fatal_only)
 
   /* Find the error kind of the highest severity.  */
   DEBUG_TRACE ("multiple opcode entries with error kind");
-  kind = AARCH64_OPDE_NIL;
+  record = NULL;
   for (curr = head; curr != NULL; curr = curr->next)
     {
       gas_assert (curr->detail.kind != AARCH64_OPDE_NIL);
@@ -5832,14 +5870,16 @@ output_operand_error_report (char *str, bool non_fatal_only)
 	{
 	  DEBUG_TRACE ("\t%s", operand_mismatch_kind_names[curr->detail.kind]);
 	}
-      if (operand_error_higher_severity_p (curr->detail.kind, kind)
-	  && (!non_fatal_only || (non_fatal_only && curr->detail.non_fatal)))
-	kind = curr->detail.kind;
+      if ((!non_fatal_only || curr->detail.non_fatal)
+	  && (!record || better_error_p (curr, record)))
+	record = curr;
     }
 
+  kind = (record ? record->detail.kind : AARCH64_OPDE_NIL);
   gas_assert (kind != AARCH64_OPDE_NIL || non_fatal_only);
 
   /* Pick up one of errors of KIND to report.  */
+  record = NULL;
   for (curr = head; curr != NULL; curr = curr->next)
     {
       /* If we don't want to print non-fatal errors then don't consider them
diff --git a/gas/testsuite/gas/aarch64/sme-8-illegal.l b/gas/testsuite/gas/aarch64/sme-8-illegal.l
index ee9f76f3b9c..7123e8d9eac 100644
--- a/gas/testsuite/gas/aarch64/sme-8-illegal.l
+++ b/gas/testsuite/gas/aarch64/sme-8-illegal.l
@@ -1,7 +1,7 @@
 [^:]*: Assembler messages:
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstart x0'
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstart sa'
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstart zm'
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstop x0'
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstop sa'
-[^:]*:[0-9]+: Error: unexpected characters following instruction -- `smstop zm'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstart x0'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstart sa'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstart zm'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstop x0'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstop sa'
+[^:]*:[0-9]+: Error: unknown or missing PSTATE field name at operand 1 -- `smstop zm'
-- 
2.25.1


  parent reply	other threads:[~2023-03-30 10:24 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-30 10:23 [PATCH 00/43] aarch64: Groundwork for SME2 support Richard Sandiford
2023-03-30 10:23 ` [PATCH 01/43] aarch64: Fix PSEL opcode mask Richard Sandiford
2023-03-30 10:23 ` [PATCH 02/43] aarch64: Restrict range of PRFM opcodes Richard Sandiford
2023-03-30 10:23 ` [PATCH 03/43] aarch64: Fix SVE2 register/immediate distinction Richard Sandiford
2023-03-30 10:23 ` [PATCH 04/43] aarch64: Make SME instructions use F_STRICT Richard Sandiford
2023-03-30 10:23 ` [PATCH 05/43] aarch64: Use aarch64_operand_error more widely Richard Sandiford
2023-03-30 10:23 ` [PATCH 06/43] aarch64: Rename REG_TYPE_ZA* to REG_TYPE_ZAT* Richard Sandiford
2023-03-30 10:23 ` [PATCH 07/43] aarch64: Add REG_TYPE_ZATHV Richard Sandiford
2023-03-30 10:23 ` [PATCH 08/43] aarch64: Move vectype_to_qualifier further up Richard Sandiford
2023-03-30 10:23 ` [PATCH 09/43] aarch64: Rework parse_typed_reg interface Richard Sandiford
2023-03-30 10:23 ` [PATCH 10/43] aarch64: Reuse parse_typed_reg for ZA tiles Richard Sandiford
2023-03-30 10:23 ` [PATCH 11/43] aarch64: Consolidate ZA tile range checks Richard Sandiford
2023-03-30 10:23 ` [PATCH 12/43] aarch64: Treat ZA as a register Richard Sandiford
2023-03-30 10:23 ` [PATCH 13/43] aarch64: Rename za_tile_vector to za_index Richard Sandiford
2023-03-30 10:23 ` [PATCH 14/43] aarch64: Make indexed_za use 64-bit immediates Richard Sandiford
2023-03-30 10:23 ` [PATCH 15/43] aarch64: Pass aarch64_indexed_za to parsers Richard Sandiford
2023-03-30 10:23 ` [PATCH 16/43] aarch64: Move ZA range checks to aarch64-opc.c Richard Sandiford
2023-03-30 10:23 ` [PATCH 17/43] aarch64: Consolidate ZA slice parsing Richard Sandiford
2023-03-30 10:23 ` [PATCH 18/43] aarch64: Commonise index parsing Richard Sandiford
2023-03-30 10:23 ` [PATCH 19/43] aarch64: Move w12-w15 range check to libopcodes Richard Sandiford
2023-03-30 10:23 ` [PATCH 20/43] aarch64: Tweak error for missing immediate offset Richard Sandiford
2023-03-30 10:23 ` [PATCH 21/43] aarch64: Tweak errors for base & offset registers Richard Sandiford
2023-03-30 10:23 ` [PATCH 22/43] aarch64: Tweak parsing of integer & FP registers Richard Sandiford
2023-03-30 10:23 ` [PATCH 23/43] aarch64: Improve errors for malformed register lists Richard Sandiford
2023-03-30 10:23 ` [PATCH 24/43] aarch64: Try to avoid inappropriate default errors Richard Sandiford
2023-03-30 10:23 ` [PATCH 25/43] aarch64: Rework reporting of failed register checks Richard Sandiford
2023-03-30 10:23 ` [PATCH 26/43] aarch64: Update operand_mismatch_kind_names Richard Sandiford
2023-03-30 10:23 ` [PATCH 27/43] aarch64: Deprioritise AARCH64_OPDE_REG_LIST Richard Sandiford
2023-03-30 10:23 ` [PATCH 28/43] aarch64: Add an error code for out-of-range registers Richard Sandiford
2023-03-30 10:23 ` [PATCH 29/43] aarch64: Commonise checks for index operands Richard Sandiford
2023-03-30 10:23 ` [PATCH 30/43] aarch64: Add an operand class for SVE register lists Richard Sandiford
2023-03-30 10:23 ` [PATCH 31/43] aarch64: Make AARCH64_OPDE_REG_LIST take a bitfield Richard Sandiford
2023-03-30 10:23 ` [PATCH 32/43] aarch64: Tweak register list errors Richard Sandiford
2023-03-30 10:23 ` [PATCH 33/43] aarch64: Try to report invalid variants against the closest match Richard Sandiford
2023-03-30 10:23 ` Richard Sandiford [this message]
2023-03-30 10:23 ` [PATCH 35/43] aarch64: Rename aarch64-tbl.h OP_SME_* macros Richard Sandiford
2023-03-30 10:23 ` [PATCH 36/43] aarch64: Reorder some OP_SVE_* macros Richard Sandiford
2023-03-30 10:23 ` [PATCH 37/43] aarch64: Add a aarch64_cpu_supports_inst_p helper Richard Sandiford
2023-03-30 10:23 ` [PATCH 38/43] aarch64: Rename some of GAS's REG_TYPE_* macros Richard Sandiford
2023-03-30 10:23 ` [PATCH 39/43] aarch64: Regularise FLD_* suffixes Richard Sandiford
2023-03-30 10:23 ` [PATCH 40/43] aarch64: Resync field names Richard Sandiford
2023-03-30 10:23 ` [PATCH 41/43] aarch64: Sort fields alphanumerically Richard Sandiford
2023-03-30 10:23 ` [PATCH 42/43] aarch64: Add support for strided register lists Richard Sandiford
2023-03-30 15:50   ` Simon Marchi
2023-03-30 16:06     ` Richard Sandiford
2023-03-30 10:23 ` [PATCH 43/43] aarch64: Prefer register ranges & support wrapping Richard Sandiford

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=20230330102359.3327695-35-richard.sandiford@arm.com \
    --to=richard.sandiford@arm.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).