[llvm] r268630 - Fix Mips Parser error reporting

Nirav Dave via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 07:15:47 PDT 2016


Author: niravd
Date: Thu May  5 09:15:46 2016
New Revision: 268630

URL: http://llvm.org/viewvc/llvm-project?rev=268630&view=rev
Log:
Fix Mips Parser error reporting

[mips] On error, ParseDirective should always return false to signify that the
directive was understood.

Reviewers: dsanders, vkalintiris, sdardis

Subscribers: dsanders, llvm-commits, sdardis

Differential Revision: http://reviews.llvm.org/D19929

Modified:
    llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
    llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s

Modified: llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp?rev=268630&r1=268629&r2=268630&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp Thu May  5 09:15:46 2016
@@ -4830,6 +4830,8 @@ bool MipsAsmParser::ParseInstruction(Par
   return false;
 }
 
+// FIXME: Given that these have the same name, these should both be
+// consistent on affecting the Parser.
 bool MipsAsmParser::reportParseError(Twine ErrorMsg) {
   MCAsmParser &Parser = getParser();
   SMLoc Loc = getLexer().getLoc();
@@ -5438,7 +5440,6 @@ bool MipsAsmParser::parseDirectiveCPSetu
   OperandMatchResultTy ResTy = parseAnyRegister(TmpReg);
   if (ResTy == MatchOperand_NoMatch) {
     reportParseError("expected register containing function address");
-    Parser.eatToEndOfStatement();
     return false;
   }
 
@@ -5985,13 +5986,22 @@ bool MipsAsmParser::parseFpABIValue(Mips
 }
 
 bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {
+  // This returns false if this function recognizes the directive
+  // regardless of whether it is successfully handles or reports an
+  // error. Otherwise it returns true to give the generic parser a
+  // chance at recognizing it.
+
   MCAsmParser &Parser = getParser();
   StringRef IDVal = DirectiveID.getString();
 
-  if (IDVal == ".cpload")
-    return parseDirectiveCpLoad(DirectiveID.getLoc());
-  if (IDVal == ".cprestore")
-    return parseDirectiveCpRestore(DirectiveID.getLoc());
+  if (IDVal == ".cpload") {
+    parseDirectiveCpLoad(DirectiveID.getLoc());
+    return false;
+  }
+  if (IDVal == ".cprestore") {
+    parseDirectiveCpRestore(DirectiveID.getLoc());
+    return false;
+  }
   if (IDVal == ".dword") {
     parseDataDirective(8, DirectiveID.getLoc());
     return false;
@@ -6148,7 +6158,8 @@ bool MipsAsmParser::ParseDirective(AsmTo
   }
 
   if (IDVal == ".set") {
-    return parseDirectiveSet();
+    parseDirectiveSet();
+    return false;
   }
 
   if (IDVal == ".mask" || IDVal == ".fmask") {
@@ -6232,8 +6243,10 @@ bool MipsAsmParser::ParseDirective(AsmTo
     return false;
   }
 
-  if (IDVal == ".option")
-    return parseDirectiveOption();
+  if (IDVal == ".option") {
+    parseDirectiveOption();
+    return false;
+  }
 
   if (IDVal == ".abicalls") {
     getTargetStreamer().emitDirectiveAbiCalls();
@@ -6246,25 +6259,34 @@ bool MipsAsmParser::ParseDirective(AsmTo
     return false;
   }
 
-  if (IDVal == ".cpsetup")
-    return parseDirectiveCPSetup();
-
-  if (IDVal == ".cpreturn")
-    return parseDirectiveCPReturn();
-
-  if (IDVal == ".module")
-    return parseDirectiveModule();
-
-  if (IDVal == ".llvm_internal_mips_reallow_module_directive")
-    return parseInternalDirectiveReallowModule();
-
-  if (IDVal == ".insn")
-    return parseInsnDirective();
-
-  if (IDVal == ".sbss")
-    return parseSSectionDirective(IDVal, ELF::SHT_NOBITS);
-  if (IDVal == ".sdata")
-    return parseSSectionDirective(IDVal, ELF::SHT_PROGBITS);
+  if (IDVal == ".cpsetup") {
+    parseDirectiveCPSetup();
+    return false;
+  }
+  if (IDVal == ".cpreturn") {
+    parseDirectiveCPReturn();
+    return false;
+  }
+  if (IDVal == ".module") {
+    parseDirectiveModule();
+    return false;
+  }
+  if (IDVal == ".llvm_internal_mips_reallow_module_directive") {
+    parseInternalDirectiveReallowModule();
+    return false;
+  }
+  if (IDVal == ".insn") {
+    parseInsnDirective();
+    return false;
+  }
+  if (IDVal == ".sbss") {
+    parseSSectionDirective(IDVal, ELF::SHT_NOBITS);
+    return false;
+  }
+  if (IDVal == ".sdata") {
+    parseSSectionDirective(IDVal, ELF::SHT_PROGBITS);
+    return false;
+  }
 
   return true;
 }

Modified: llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s?rev=268630&r1=268629&r2=268630&view=diff
==============================================================================
--- llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s (original)
+++ llvm/trunk/test/MC/Mips/set-push-pop-directives-bad.s Thu May  5 09:15:46 2016
@@ -1,23 +1,30 @@
 # RUN: not llvm-mc %s -triple=mipsel-unknown-linux -mcpu=mips32r2 2>%t1
 # RUN:   FileCheck %s < %t1
 
+# Check only one error per statement.
+
         .text
         .set pop
 # CHECK: :[[@LINE-1]]:14: error: .set pop with no .set push
+# CHECK-NOT: :[[@LINE-1]]:
         .set push
         .set pop
         .set pop
 # CHECK: :[[@LINE-1]]:14: error: .set pop with no .set push
+# CHECK-NOT: :[[@LINE-1]]:
         .set push foo
 # CHECK: :[[@LINE-1]]:19: error: unexpected token, expected end of statement
+# CHECK-NOT: :[[@LINE-1]]:
         .set pop bar
 # CHECK: :[[@LINE-1]]:18: error: unexpected token, expected end of statement
-
+# CHECK-NOT: :[[@LINE-1]]:
         .set hardfloat
         .set push
         .set softfloat
         add.s $f2, $f2, $f2
 # CHECK: :[[@LINE-1]]:9: error: instruction requires a CPU feature not currently enabled
+# CHECK-NOT: :[[@LINE-1]]:
         .set pop
         add.s $f2, $f2, $f2
 # CHECK-NOT: :[[@LINE-1]]:9: error: instruction requires a CPU feature not currently enabled
+# CHECK-NOT: :[[@LINE-1]]:




More information about the llvm-commits mailing list