[mips] Remove the last usage of parseRegister from MipsAsmParser.

[matheusalmeida]

Summary

Added negative test case so that we can be sure we handle erroneous situations
while parsing the .cpsetup directive.

[dsanders]

LGTM with one nit about the placement of TmpReg.clear()

You can also use unique_ptr to avoid the manual delete statements if you want to.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2366

It's probably best to use std::unique_ptr so you don't have to manually delete the object.

2379–2380

Move this up to the 'delete FuncRegOpnd'

[matheusalmeida]

Use of std::unique_ptr to avoid having to manually delete the MipsOperands created while
parsing the directive.

[matheusalmeida]

Trying to fix the previous mistake when I forgot to specify HEAD^...

[matheusalmeida]

I know you LGTM'd this already but I made the change you suggested (use of std::unique_ptr). Do you want to skim read this patch again just to make sure I implemented exactly what you suggested ?

Matheus

[dsanders]

The unique_ptr would be better as part of the 'SmallVector<MCParsedAsmOperand *, 1> TmpReg'. At the moment, there are two pointers to TmpReg[0] so it's not really unique.

[matheusalmeida]

Test.

[matheusalmeida]

Fix use of std::unique_ptr.

[dsanders]

LGTM

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

#include "MipsGenAsmMatcher.inc"
 
   bool isMicroMips() const {
     return STI.getFeatureBits() & Mips::FeatureMicroMips;
   }
 
   bool hasMips4() const { return STI.getFeatureBits() & Mips::FeatureMips4; }
   bool hasMips32() const { return STI.getFeatureBits() & Mips::FeatureMips32; }
 
-  bool parseRegister(unsigned &RegNum);
-
   bool eatComma(StringRef ErrorStr);
 
   int matchCPURegisterName(StringRef Symbol);
 
   int matchRegisterByNumber(unsigned RegNum, unsigned RegClass);
 
   int matchFPURegisterName(StringRef Name);
 
bool MipsAsmParser::parseSetFeature(uint64_t Feature) {
   case Mips::FeatureMips64r2:
     setFeatureBits(Mips::FeatureMips64r2, "mips64r2");
     getTargetStreamer().emitDirectiveSetMips64R2();
     break;
   }
   return false;
 }
 
-bool MipsAsmParser::parseRegister(unsigned &RegNum) {
-  if (!getLexer().is(AsmToken::Dollar))
-    return false;
-
-  Parser.Lex();
-
-  const AsmToken &Reg = Parser.getTok();
-  if (Reg.is(AsmToken::Identifier)) {
-    RegNum = matchCPURegisterName(Reg.getIdentifier());
-  } else if (Reg.is(AsmToken::Integer)) {
-    RegNum = Reg.getIntVal();
-  } else {
-    return false;
-  }
-
-  Parser.Lex();
-  return true;
-}
-
 bool MipsAsmParser::eatComma(StringRef ErrorStr) {
   if (getLexer().isNot(AsmToken::Comma)) {
     SMLoc Loc = getLexer().getLoc();
     Parser.eatToEndOfStatement();
     return Error(Loc, ErrorStr);
   }
 
   Parser.Lex(); // Eat the comma.
bool MipsAsmParser::parseDirectiveCPLoad(SMLoc Loc) {
   return false;
 }
 
 bool MipsAsmParser::parseDirectiveCPSetup() {
   unsigned FuncReg;
   unsigned Save;
   bool SaveIsReg = true;
 
-  if (!parseRegister(FuncReg))
-    return reportParseError("expected register containing function address");
-  FuncReg = getGPR(FuncReg);
+  SmallVector<std::unique_ptr<MCParsedAsmOperand>, 1> TmpReg;
+  OperandMatchResultTy ResTy = ParseAnyRegister(TmpReg);
+  if (ResTy == MatchOperand_NoMatch) {
+    reportParseError("expected register containing function address");
+    Parser.eatToEndOfStatement();
+    return false;
+  }
+
+  MipsOperand &FuncRegOpnd = static_cast<MipsOperand &>(*TmpReg[0]);
+  if (!FuncRegOpnd.isGPRAsmReg()) {
+    reportParseError(FuncRegOpnd.getStartLoc(), "invalid register");
+    Parser.eatToEndOfStatement();
+    return false;
+  }
+
+  FuncReg = FuncRegOpnd.getGPR32Reg();
+  TmpReg.clear();
 
   if (!eatComma("expected comma parsing directive"))
     return true;
 
-  if (!parseRegister(Save)) {
+  ResTy = ParseAnyRegister(TmpReg);
+  if (ResTy == MatchOperand_NoMatch) {
     const AsmToken &Tok = Parser.getTok();
     if (Tok.is(AsmToken::Integer)) {
       Save = Tok.getIntVal();
       SaveIsReg = false;
       Parser.Lex();
-    } else
-      return reportParseError("expected save register or stack offset");
-  } else
-    Save = getGPR(Save);
+    } else {
+      reportParseError("expected save register or stack offset");
+      Parser.eatToEndOfStatement();
+      return false;
+    }
+  } else {
+    MipsOperand &SaveOpnd = static_cast<MipsOperand &>(*TmpReg[0]);
+    if (!SaveOpnd.isGPRAsmReg()) {
+      reportParseError(SaveOpnd.getStartLoc(), "invalid register");
+      Parser.eatToEndOfStatement();
+      return false;
+    }
+    Save = SaveOpnd.getGPR32Reg();
+  }
 
   if (!eatComma("expected comma parsing directive"))
     return true;
 
   StringRef Name;
   if (Parser.parseIdentifier(Name))
     reportParseError("expected identifier");
   MCSymbol *Sym = getContext().GetOrCreateSymbol(Name);

test/MC/Mips/cpsetup-bad.s

+# RUN: not llvm-mc %s -triple mips64-unknown-unknown 2>%t1
+# RUN:   FileCheck %s < %t1 -check-prefix=ASM
+
+        .text
+        .option pic2
+t1:
+        .cpsetup $bar, 8, __cerror
+# ASM: :[[@LINE-1]]:18: error: expected register containing function address
+        .cpsetup $33, 8, __cerror
+# ASM: :[[@LINE-1]]:18: error: invalid register
+        .cpsetup $31, foo, __cerror
+# ASM: :[[@LINE-1]]:23: error: expected save register or stack offset
+        .cpsetup $31, $32, __cerror
+# ASM: :[[@LINE-1]]:23: error: invalid register