[SPARC][IAS] Rework ASI/Prefetch tag matching in prep for ParseForAllFeatures

[koachan]

Unify parts of ASI and Prefetch tag matching at parseASITag
and parsePrefetchTag to use a common function to parse any immediate
expressions. This introduces a slight regression to error messages,
but is needed so we can enable ParseForAllFeatures
in MatchOperandParserImpl in a future patch.

[llvmbot]

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-sparc

Author: Koakuma (koachan)

Changes

This changes the ASI tag matching at parseASITag to use a similar
implementation to parsePrefetchTag. This introduces a slight regression
to error messages, but is needed so we can enable ParseForAllFeatures
in MatchOperandParserImpl in a future patch.

---

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

3 Files Affected:

• (modified) llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp (+23-13)
• (modified) llvm/test/MC/Sparc/sparc-mem-asi-instructions.s (+2-2)
• (modified) llvm/test/MC/Sparc/sparcv9-instructions.s (+2-2)

diff --git a/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp b/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
index f0a3a4e88b30c..3d8637bb8c359 100644
--- a/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
+++ b/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
@@ -1085,13 +1085,24 @@ ParseStatus SparcAsmParser::parseASITag(OperandVector &Operands) {
   SMLoc E = Parser.getTok().getEndLoc();
   int64_t ASIVal = 0;
 
-  if (is64Bit() && (getLexer().getKind() == AsmToken::Hash)) {
+  switch (getLexer().getKind()) {
+  case AsmToken::LParen:
+  case AsmToken::Integer:
+  case AsmToken::Identifier:
+  case AsmToken::Plus:
+  case AsmToken::Minus:
+  case AsmToken::Tilde:
+    if (getParser().parseAbsoluteExpression(ASIVal) || !isUInt<8>(ASIVal))
+      return Error(S, "invalid ASI number, must be between 0 and 255");
+    break;
+  case AsmToken::Hash: {
     // For now we only support named tags for 64-bit/V9 systems.
     // TODO: add support for 32-bit/V8 systems.
     SMLoc TagStart = getLexer().peekTok(false).getLoc();
     Parser.Lex(); // Eat the '#'.
-    auto ASIName = Parser.getTok().getString();
-    auto ASITag = SparcASITag::lookupASITagByName(ASIName);
+    const StringRef ASIName = Parser.getTok().getString();
+    const SparcASITag::ASITag *ASITag =
+        SparcASITag::lookupASITagByName(ASIName);
     if (!ASITag)
       ASITag = SparcASITag::lookupASITagByAltName(ASIName);
     Parser.Lex(); // Eat the identifier token.
@@ -1100,15 +1111,10 @@ ParseStatus SparcAsmParser::parseASITag(OperandVector &Operands) {
       return Error(TagStart, "unknown ASI tag");
 
     ASIVal = ASITag->Encoding;
-  } else if (!getParser().parseAbsoluteExpression(ASIVal)) {
-    if (!isUInt<8>(ASIVal))
-      return Error(S, "invalid ASI number, must be between 0 and 255");
-  } else {
-    return Error(
-        S, is64Bit()
-               ? "malformed ASI tag, must be %asi, a constant integer "
-                 "expression, or a named tag"
-               : "malformed ASI tag, must be a constant integer expression");
+    break;
+  }
+  default:
+    return ParseStatus::NoMatch;
   }
 
   Operands.push_back(SparcOperand::CreateASITag(ASIVal, S, E));
@@ -1230,8 +1236,12 @@ ParseStatus SparcAsmParser::parseOperand(OperandVector &Operands,
     // Parse an optional address-space identifier after the address.
     // This will be either an immediate constant expression, or, on 64-bit
     // processors, the %asi register.
-    if (is64Bit() && getLexer().is(AsmToken::Percent)) {
+    if (getLexer().is(AsmToken::Percent)) {
       SMLoc S = Parser.getTok().getLoc();
+      if (!is64Bit())
+        return Error(
+            S, "malformed ASI tag, must be a constant integer expression");
+
       Parser.Lex(); // Eat the %.
       const AsmToken Tok = Parser.getTok();
       if (Tok.is(AsmToken::Identifier) && Tok.getString() == "asi") {
diff --git a/llvm/test/MC/Sparc/sparc-mem-asi-instructions.s b/llvm/test/MC/Sparc/sparc-mem-asi-instructions.s
index 39abe7b99cb40..8b8503caf4ba0 100644
--- a/llvm/test/MC/Sparc/sparc-mem-asi-instructions.s
+++ b/llvm/test/MC/Sparc/sparc-mem-asi-instructions.s
@@ -1,9 +1,9 @@
 ! RUN: not llvm-mc %s -triple=sparc   -show-encoding 2>&1 | FileCheck %s --check-prefix=V8
 ! RUN: not llvm-mc %s -triple=sparcv9 -show-encoding 2>&1 | FileCheck %s --check-prefix=V9
 
-! V8: error: malformed ASI tag, must be a constant integer expression
+! V8: error: expected absolute expression
 ! V8-NEXT: lduba [%i0] asi, %o2
-! V9: error: malformed ASI tag, must be %asi, a constant integer expression, or a named tag
+! V9: error: unexpected token
 ! V9-NEXT: lduba [%i0] asi, %o2
 lduba [%i0] asi, %o2
 
diff --git a/llvm/test/MC/Sparc/sparcv9-instructions.s b/llvm/test/MC/Sparc/sparcv9-instructions.s
index 1b11171b7074d..a7761c10c509b 100644
--- a/llvm/test/MC/Sparc/sparcv9-instructions.s
+++ b/llvm/test/MC/Sparc/sparcv9-instructions.s
@@ -657,12 +657,12 @@
         ! V9: prefetcha [%i1+3968] %asi, #one_read    ! encoding: [0xc3,0xee,0x6f,0x80]
         prefetcha  [ %i1 + 0xf80 ] %asi, #one_read
 
-        ! V8:      error: malformed ASI tag, must be a constant integer expression
+        ! V8:      error: invalid operand for instruction
         ! V8-NEXT: prefetcha  [ %i1 + %i2 ] #ASI_SNF, 1
         ! V9: prefetcha [%i1+%i2] #ASI_SNF, #one_read ! encoding: [0xc3,0xee,0x50,0x7a]
         prefetcha  [ %i1 + %i2 ] #ASI_SNF, 1
 
-        ! V8:      error: malformed ASI tag, must be a constant integer expression
+        ! V8:      error: unexpected token
         ! V8-NEXT: prefetcha  [ %i1 + %i2 ] #ASI_SNF, #one_read
         ! V9: prefetcha [%i1+%i2] #ASI_SNF, #one_read ! encoding: [0xc3,0xee,0x50,0x7a]
         prefetcha  [ %i1 + %i2 ] #ASI_SNF, #one_read

[s-barannikov]

I'd suggest to extract these cases into a helper function that returns true if the token can be a start of expression.

Also, optionally, add a parseExpression method that returns NoMatch if the first token can't start an expression and Success/Failure if it can and expression parsing succeeded/failed.

[koachan]

Also, optionally, add a parseExpression method that returns NoMatch if the first token can't start an expression and Success/Failure if it can and expression parsing succeeded/failed.

Hmm, not sure how would I use this in the ASI/Prefetch tag parsing...
I'm thinking about calling parseExpression then checking if the return value is a Success before further checking if the returned value is in range for each tag type, but apparently comparing ParseStatuses is an error?

[clang] (typecheck_invalid_operands) Invalid operands to binary expression ('ParseStatus' and 'const StatusTy')

[s-barannikov]

It should be examined using its public methods (isSuccess(), isFailure() or isNoMatch()).

[s-barannikov]

Suggested change

	bool isPossibleExpression(AsmToken &Token);
	bool isPossibleExpression(const AsmToken &Token);

[s-barannikov]

Can you use this method in parsePrefetchTag too?
Operands is unused.