Skip to content

Conversation

@jthackray
Copy link
Contributor

Currently, when printing SYS aliases, the first instruction operand is compared with the string constant "all" to decide if a register needs to be parsed as the next operand.

For example, TLBI VMALLE1IS contains "all" so no register is expected, but TLBI IPAS2E1IS doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the (already provided) explicit NeedsReg bit flag provided in tablegen to check if a register is required to be parsed. This is already used by the code in AArch64InstPrinter.cpp, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change, and all AArch64 test cases continue to pass after this change.

…m tablegen (NFC)

Currently, when printing SYS aliases, the first instruction operand
is compared with the string constant "all" to decide if a register
needs to be parsed as the next operand.

For example, `TLBI VMALLE1IS` contains "all" so no register is expected,
but `TLBI IPAS2E1IS` doesn't match, so a register is expected.

Future AArch64 SYS aliases won't match this pattern, so use the (already
provided) explicit `NeedsReg` bit flag provided in tablegen to check if a
register is required to be parsed. This is already used by the code in
`AArch64InstPrinter.cpp`, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change,
and all AArch64 test cases continue to pass after this change.
@llvmbot
Copy link
Member

llvmbot commented May 18, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Jonathan Thackray (jthackray)

Changes

Currently, when printing SYS aliases, the first instruction operand is compared with the string constant "all" to decide if a register needs to be parsed as the next operand.

For example, TLBI VMALLE1IS contains "all" so no register is expected, but TLBI IPAS2E1IS doesn't match, so a register is expected.

Future AArch64 SYS aliases won't always match this pattern, so use the (already provided) explicit NeedsReg bit flag provided in tablegen to check if a register is required to be parsed. This is already used by the code in AArch64InstPrinter.cpp, so now we are consistent in this source file too.

No test files have been changed, since this is a non-functional change, and all AArch64 test cases continue to pass after this change.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp (+3-1)
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 00e8140807735..e0bd2e55cd959 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -3917,6 +3917,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
   const AsmToken &Tok = getTok();
   StringRef Op = Tok.getString();
   SMLoc S = Tok.getLoc();
+  bool ExpectRegister = true;
 
   if (Mnemonic == "ic") {
     const AArch64IC::IC *IC = AArch64IC::lookupICByName(Op);
@@ -3927,6 +3928,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
       setRequiredFeatureString(IC->getRequiredFeatures(), Str);
       return TokError(Str);
     }
+    ExpectRegister = IC->NeedsReg;
     createSysAlias(IC->Encoding, Operands, S);
   } else if (Mnemonic == "dc") {
     const AArch64DC::DC *DC = AArch64DC::lookupDCByName(Op);
@@ -3957,6 +3959,7 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
       setRequiredFeatureString(TLBI->getRequiredFeatures(), Str);
       return TokError(Str);
     }
+    ExpectRegister = TLBI->NeedsReg;
     createSysAlias(TLBI->Encoding, Operands, S);
   } else if (Mnemonic == "cfp" || Mnemonic == "dvp" || Mnemonic == "cpp" || Mnemonic == "cosp") {
 
@@ -3987,7 +3990,6 @@ bool AArch64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
 
   Lex(); // Eat operand.
 
-  bool ExpectRegister = !Op.contains_insensitive("all");
   bool HasRegister = false;
 
   // Check for the optional register operand.

Copy link
Contributor

@Stylie777 Stylie777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jthackray jthackray requested a review from pratlucas May 19, 2025 15:04
@jthackray jthackray merged commit e8a2ce1 into main May 19, 2025
13 checks passed
@jthackray jthackray deleted the users/jthackray/use-needsreg branch May 19, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants