[llvm] [ARM] FIX: change `pkhtb` custom parsing produce NoMatch rather than Error (PR #85765)

[Alfie Richards via llvm-commits]

https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/85765

>From 61ee60c6bb5da85619789a5fb1b10f6a46cc2b87 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Tue, 19 Mar 2024 10:02:43 +0000
Subject: [PATCH] [ARM] Fix PKHImm parsing.

This was broken by https://github.com/llvm/llvm-project/pull/83436 as in
optional operands meant sometimes the parsePKHImm parser is applied to
operands in other positions, which previously produced an error.

Now this instead fails the parse. However this unfortunately means
the default parsing happens which combines the shift and the base operator
which causes the diagnostics to be non-sense.

To fix this I had to add a special case diagnostic which is unfortunate.
Ideally these two methods for handling operands should be unified and the
TableGen matching mechanisms should be used for validaing parsed shifts rather
than special cases in parsing.
---
 .../lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 21 +++++++++++++++++--
 llvm/test/MC/ARM/basic-arm-instructions.s     |  4 ++++
 llvm/test/MC/ARM/diagnostics.s                |  4 ++--
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 026d4ca06e87b9..e21310c53f45a0 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -4318,6 +4318,23 @@ int ARMAsmParser::tryParseShiftRegister(OperandVector &Operands) {
     }
   }
 
+  auto MnemonicOp = static_cast<ARMOperand &>(*Operands[0]);
+  // These instructions handle shift operands as seperate operators to the
+  // register. In these cases doing the default conversion makes nonsense
+  // diagnostics.
+  // FIXME: Unify the different methods for handling shift operators
+  // and use TableGen matching mechanisms to do the validation.
+  static const DenseSet<StringRef> NoDefaultConvertSet{"pkhbt", "pkhtb"};
+  bool ShouldDefaultConvert =
+      (MnemonicOp.isToken() &&
+       !NoDefaultConvertSet.contains(MnemonicOp.getToken()));
+  if (!ShouldDefaultConvert) {
+    // If we get to this point the parsing for the shift operator in
+    // parsePKHLSLImm has failed. So we can generate a diagnostic here.
+    Error(S, "shift operator is malformed for this instruction");
+    return -1;
+  }
+
   if (ShiftReg && ShiftTy != ARM_AM::rrx)
     Operands.push_back(ARMOperand::CreateShiftedRegister(ShiftTy, SrcReg,
                                                          ShiftReg, Imm,
@@ -5289,12 +5306,12 @@ ParseStatus ARMAsmParser::parsePKHImm(OperandVector &Operands, StringRef Op,
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   if (Tok.isNot(AsmToken::Identifier))
-    return Error(Parser.getTok().getLoc(), Op + " operand expected.");
+    return ParseStatus::NoMatch;
   StringRef ShiftName = Tok.getString();
   std::string LowerOp = Op.lower();
   std::string UpperOp = Op.upper();
   if (ShiftName != LowerOp && ShiftName != UpperOp)
-    return Error(Parser.getTok().getLoc(), Op + " operand expected.");
+    return ParseStatus::NoMatch;
   Parser.Lex(); // Eat shift type token.
 
   // There must be a '#' and a shift amount.
diff --git a/llvm/test/MC/ARM/basic-arm-instructions.s b/llvm/test/MC/ARM/basic-arm-instructions.s
index 055f3ce316153d..84a7cf52fa30e0 100644
--- a/llvm/test/MC/ARM/basic-arm-instructions.s
+++ b/llvm/test/MC/ARM/basic-arm-instructions.s
@@ -1774,6 +1774,9 @@ Lforward:
         pkhtb r2, r2, r3, asr #31
         pkhtb r2, r2, r3, asr #15
 
+        it ne
+        pkhtbne r2, r2, r3, asr #15
+
 @ CHECK: pkhbt	r2, r2, r3              @ encoding: [0x13,0x20,0x82,0xe6]
 @ CHECK: pkhbt	r2, r2, r3, lsl #31     @ encoding: [0x93,0x2f,0x82,0xe6]
 @ CHECK: pkhbt	r2, r2, r3              @ encoding: [0x13,0x20,0x82,0xe6]
@@ -1782,6 +1785,7 @@ Lforward:
 @ CHECK: pkhbt	r2, r3, r2              @ encoding: [0x12,0x20,0x83,0xe6]
 @ CHECK: pkhtb	r2, r2, r3, asr #31     @ encoding: [0xd3,0x2f,0x82,0xe6]
 @ CHECK: pkhtb	r2, r2, r3, asr #15     @ encoding: [0xd3,0x27,0x82,0xe6]
+@ CHECK: pkhtbne r2, r2, r3, asr #15    @ encoding: [0xd3,0x27,0x82,0x16]
 
 @------------------------------------------------------------------------------
 @ FIXME: PLD
diff --git a/llvm/test/MC/ARM/diagnostics.s b/llvm/test/MC/ARM/diagnostics.s
index fa23a7da1e4048..b6d011e5bdef8f 100644
--- a/llvm/test/MC/ARM/diagnostics.s
+++ b/llvm/test/MC/ARM/diagnostics.s
@@ -235,10 +235,10 @@
 @ CHECK-ERRORS: error: immediate value out of range
 @ CHECK-ERRORS:         pkhtb r2, r2, r3, asr #33
 @ CHECK-ERRORS:                                ^
-@ CHECK-ERRORS: error: lsl operand expected.
+@ CHECK-ERRORS: error: shift operator is malformed for this instruction
 @ CHECK-ERRORS:         pkhbt r2, r2, r3, asr #3
 @ CHECK-ERRORS:                           ^
-@ CHECK-ERRORS: error: asr operand expected.
+@ CHECK-ERRORS: error: shift operator is malformed for this instruction
 @ CHECK-ERRORS:         pkhtb r2, r2, r3, lsl #3
 @ CHECK-ERRORS:                           ^