[llvm] [AArch64][MC] Improve `isSImm` Diagnostic Fidelity (PR #127255)
Sam Elliott via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 14 12:07:58 PST 2025
https://github.com/lenary created https://github.com/llvm/llvm-project/pull/127255
With the change in #126653, it became clear that there was an implicit coercion from DiagnosticPredicate bool in `isSimm`, which is then undone in the TableGen'd code that uses the predicate, converting from a bool back to a DiagnosticPredicate.
This roundtrip conversion loses information - if `isSImmScaled` returns `DiagnosticPredicate::NoMatch` then it is converted to `DiagnosticPredicate::NearMatch`, which both will cause an error, but have different behaviour for which diagnostic is used.
This change removes the boolean conversions and directly returns the DiagnosticPredicate. This causes some changes to error messages, most of which are that when a register provided but a specific immediate was expected, now the error is "invalid operand for instruction" and not an indication that the immediate was out of range.
The only change not like this is to LDR (Register), which seems to just choose a different immediate-related diagnostic when a 64-bit register is used - which was equally inaccurate as the previous diagnostic. This is because there are many other places in the parser that do not (yet?) use DiagnosticPredicate, in this case the issue seems to be `isUImm12Offset`, which still returns a `bool`.
>From cd301a35e76cb602b4bcda5465f8c34ef3860820 Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Fri, 14 Feb 2025 11:52:23 -0800
Subject: [PATCH] [AArch64][MC] Improve `isSImm` Diagnostic Fidelity
With the change in #126653, it became clear that there was an implicit
coercion from DiagnosticPredicate bool in `isSimm`, which is then undone
in the TableGen'd code that uses the predicate, converting from a bool
back to a DiagnosticPredicate.
This roundtrip conversion loses information - if `isSImmScaled` returns
`DiagnosticPredicate::NoMatch` then it is converted to
`DiagnosticPredicate::NearMatch`, which both will cause an error, but
have different behaviour for which diagnostic is used.
This change removes the boolean conversions and directly returns the
DiagnosticPredicate. This causes some changes to error messages, most of
which are that when a register provided but a specific immediate was
expected, now the error is "invalid operand for instruction" and not an
indication that the immediate was out of range.
The only change not like this is to LDR (Register), which seems to just
choose a different immediate-related diagnostic when a 64-bit register
is used - which was equally inaccurate as the previous error. This is
because there are many other places in the parser that do not (yet?) use
DiagnosticPredicate, in this case the issue seems to be
`isUImm12Offset`, which still returns a `bool`.
---
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp | 4 ++--
llvm/test/MC/AArch64/SME/addspl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/SME/addsvl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/SME/rdsvl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/SVE/addpl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/SVE/addvl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/SVE/index-diagnostics.s | 4 ++--
llvm/test/MC/AArch64/SVE/rdvl-diagnostics.s | 2 +-
llvm/test/MC/AArch64/arm64-diags.s | 2 +-
9 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 11fb746042e98..f2d206b864f35 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -814,8 +814,8 @@ class AArch64Operand : public MCParsedAsmOperand {
return (Val >= 0 && Val < 64);
}
- template <int Width> bool isSImm() const {
- return bool(isSImmScaled<Width, 1>());
+ template <int Width> DiagnosticPredicate isSImm() const {
+ return isSImmScaled<Width, 1>();
}
template <int Bits, int Scale> DiagnosticPredicate isSImmScaled() const {
diff --git a/llvm/test/MC/AArch64/SME/addspl-diagnostics.s b/llvm/test/MC/AArch64/SME/addspl-diagnostics.s
index 02b8f6ca765aa..96e926e327b36 100644
--- a/llvm/test/MC/AArch64/SME/addspl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SME/addspl-diagnostics.s
@@ -8,6 +8,6 @@ addspl x19, x14, #32
// addspl requires an immediate, not a register.
addspl x19, x14, x15
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: addspl x19, x14, x15
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SME/addsvl-diagnostics.s b/llvm/test/MC/AArch64/SME/addsvl-diagnostics.s
index d420894bc174c..0a3867952fc25 100644
--- a/llvm/test/MC/AArch64/SME/addsvl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SME/addsvl-diagnostics.s
@@ -8,6 +8,6 @@ addsvl x3, x5, #32
// addsvl requires an immediate, not a register.
addsvl x3, x5, x6
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: addsvl x3, x5, x6
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SME/rdsvl-diagnostics.s b/llvm/test/MC/AArch64/SME/rdsvl-diagnostics.s
index 0ea7d9beaef3b..8e5e85b089102 100644
--- a/llvm/test/MC/AArch64/SME/rdsvl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SME/rdsvl-diagnostics.s
@@ -8,6 +8,6 @@ rdsvl x9, #32
// rdsvl requires an immediate, not a register.
rdsvl x9, x10
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: rdsvl x9, x10
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SVE/addpl-diagnostics.s b/llvm/test/MC/AArch64/SVE/addpl-diagnostics.s
index eb5a80aa4ea9f..266d58abdca33 100644
--- a/llvm/test/MC/AArch64/SVE/addpl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SVE/addpl-diagnostics.s
@@ -8,6 +8,6 @@ addpl x19, x14, #32
// addpl requires an immediate, not a register.
addpl x19, x14, x15
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: addpl x19, x14, x15
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SVE/addvl-diagnostics.s b/llvm/test/MC/AArch64/SVE/addvl-diagnostics.s
index 6c04176d0bd1b..1dc540f501e43 100644
--- a/llvm/test/MC/AArch64/SVE/addvl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SVE/addvl-diagnostics.s
@@ -8,6 +8,6 @@ addvl x3, x5, #32
// addvl requires an immediate, not a register.
addvl x3, x5, x6
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: addvl x3, x5, x6
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SVE/index-diagnostics.s b/llvm/test/MC/AArch64/SVE/index-diagnostics.s
index 3b2a4aa656fd3..2600089cd01c0 100644
--- a/llvm/test/MC/AArch64/SVE/index-diagnostics.s
+++ b/llvm/test/MC/AArch64/SVE/index-diagnostics.s
@@ -48,12 +48,12 @@ index z17.d, x9, #16
// Invalid register
index z17.s, x9, w7
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-16, 15].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: index z17.s, x9, w7
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
index z17.d, w9, w7
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-16, 15].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: index z17.d, w9, w7
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/SVE/rdvl-diagnostics.s b/llvm/test/MC/AArch64/SVE/rdvl-diagnostics.s
index 0af37a60e38b0..e78b5f1292bd4 100644
--- a/llvm/test/MC/AArch64/SVE/rdvl-diagnostics.s
+++ b/llvm/test/MC/AArch64/SVE/rdvl-diagnostics.s
@@ -8,6 +8,6 @@ rdvl x9, #32
// rdvl requires an immediate, not a register.
rdvl x9, x10
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: index must be an integer in range [-32, 31].
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
// CHECK-NEXT: rdvl x9, x10
// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
diff --git a/llvm/test/MC/AArch64/arm64-diags.s b/llvm/test/MC/AArch64/arm64-diags.s
index 591ff64eb3389..5766791748ee3 100644
--- a/llvm/test/MC/AArch64/arm64-diags.s
+++ b/llvm/test/MC/AArch64/arm64-diags.s
@@ -121,7 +121,7 @@ ldr q1, [x3, w3, sxtw #1]
; CHECK-ERRORS: error: expected 'uxtw' or 'sxtw' with optional shift of #0 or #3
; CHECK-ERRORS: str d1, [x3, w3, sxtx #3]
; CHECK-ERRORS: ^
-; CHECK-ERRORS: error: index must be an integer in range [-256, 255].
+; CHECK-ERRORS: error: index must be a multiple of 4 in range [0, 16380].
; CHECK-ERRORS: ldr s1, [x3, d3, sxtx #2]
; CHECK-ERRORS: ^
More information about the llvm-commits
mailing list