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

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-arm

Author: Alfie Richards (AlfieRichardsArm)

<details>
<summary>Changes</summary>

This was broken by https://github.com/llvm/llvm-project/pull/83436 as in optional operands meant when the CC operand is provided the `parsePKHImm` parser is applied to register operands, which previously produced an error.

This patch changes `parsePKHImm` to provide `NoMatch` instead. However this unfortunately means the default parsing happens which combines the shift and the base operator which causes the diagnostics to be nonsense.

To fix this I added a special case diagnostic to this default parsing which is little 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.

---
Full diff: https://github.com/llvm/llvm-project/pull/85765.diff


3 Files Affected:

- (modified) llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (+19-2) 
- (modified) llvm/test/MC/ARM/basic-arm-instructions.s (+4) 
- (modified) llvm/test/MC/ARM/diagnostics.s (+2-2) 


``````````diff
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:                           ^
 

``````````

</details>


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