[llvm] r199068 - correct target directive handling error handling

Saleem Abdulrasool compnerd at compnerd.org
Sun Jan 12 17:15:40 PST 2014


Author: compnerd
Date: Sun Jan 12 19:15:39 2014
New Revision: 199068

URL: http://llvm.org/viewvc/llvm-project?rev=199068&view=rev
Log:
correct target directive handling error handling

The target specific parser should return `false' if the target AsmParser handles
the directive, and `true' if the generic parser should handle the directive.
Many of the target specific directive handlers would `return Error' which does
not follow these semantics.  This change simply changes the target specific
routines to conform to the semantis of the ParseDirective correctly.

Conformance to the semantics improves diagnostics emitted for the invalid
directives.  X86 is taken as a sample to ensure that multiple diagnostics are
not presented for a single error.

Added:
    llvm/trunk/test/MC/X86/x86-target-directives.s
Modified:
    llvm/trunk/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
    llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
    llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
    llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
    llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp

Modified: llvm/trunk/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp?rev=199068&r1=199067&r2=199068&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp Sun Jan 12 19:15:39 2014
@@ -2291,7 +2291,7 @@ bool AArch64AsmParser::ParseDirectiveWor
     for (;;) {
       const MCExpr *Value;
       if (getParser().parseExpression(Value))
-        return true;
+        return false;
 
       getParser().getStreamer().EmitValue(Value, Size);
 
@@ -2299,8 +2299,10 @@ bool AArch64AsmParser::ParseDirectiveWor
         break;
 
       // FIXME: Improve diagnostic.
-      if (getLexer().isNot(AsmToken::Comma))
-        return Error(L, "unexpected token in directive");
+      if (getLexer().isNot(AsmToken::Comma)) {
+        Error(L, "unexpected token in directive");
+        return false;
+      }
       Parser.Lex();
     }
   }
@@ -2313,8 +2315,10 @@ bool AArch64AsmParser::ParseDirectiveWor
 //   ::= .tlsdesccall symbol
 bool AArch64AsmParser::ParseDirectiveTLSDescCall(SMLoc L) {
   StringRef Name;
-  if (getParser().parseIdentifier(Name))
-    return Error(L, "expected symbol after directive");
+  if (getParser().parseIdentifier(Name)) {
+    Error(L, "expected symbol after directive");
+    return false;
+  }
 
   MCSymbol *Sym = getContext().GetOrCreateSymbol(Name);
   const MCSymbolRefExpr *Expr = MCSymbolRefExpr::Create(Sym, getContext());

Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=199068&r1=199067&r2=199068&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Sun Jan 12 19:15:39 2014
@@ -8072,7 +8072,7 @@ bool ARMAsmParser::parseDirectiveWord(un
     for (;;) {
       const MCExpr *Value;
       if (getParser().parseExpression(Value))
-        return true;
+        return false;
 
       getParser().getStreamer().EmitValue(Value, Size);
 
@@ -8080,8 +8080,10 @@ bool ARMAsmParser::parseDirectiveWord(un
         break;
 
       // FIXME: Improve diagnostic.
-      if (getLexer().isNot(AsmToken::Comma))
-        return Error(L, "unexpected token in directive");
+      if (getLexer().isNot(AsmToken::Comma)) {
+        Error(L, "unexpected token in directive");
+        return false;
+      }
       Parser.Lex();
     }
   }
@@ -8093,12 +8095,16 @@ bool ARMAsmParser::parseDirectiveWord(un
 /// parseDirectiveThumb
 ///  ::= .thumb
 bool ARMAsmParser::parseDirectiveThumb(SMLoc L) {
-  if (getLexer().isNot(AsmToken::EndOfStatement))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
   Parser.Lex();
 
-  if (!hasThumb())
-    return Error(L, "target does not support Thumb mode");
+  if (!hasThumb()) {
+    Error(L, "target does not support Thumb mode");
+    return false;
+  }
 
   if (!isThumb())
     SwitchMode();
@@ -8109,12 +8115,16 @@ bool ARMAsmParser::parseDirectiveThumb(S
 /// parseDirectiveARM
 ///  ::= .arm
 bool ARMAsmParser::parseDirectiveARM(SMLoc L) {
-  if (getLexer().isNot(AsmToken::EndOfStatement))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
   Parser.Lex();
 
-  if (!hasARM())
-    return Error(L, "target does not support ARM mode");
+  if (!hasARM()) {
+    Error(L, "target does not support ARM mode");
+    return false;
+  }
 
   if (isThumb())
     SwitchMode();
@@ -8140,8 +8150,11 @@ bool ARMAsmParser::parseDirectiveThumbFu
   if (isMachO) {
     const AsmToken &Tok = Parser.getTok();
     if (Tok.isNot(AsmToken::EndOfStatement)) {
-      if (Tok.isNot(AsmToken::Identifier) && Tok.isNot(AsmToken::String))
-        return Error(L, "unexpected token in .thumb_func directive");
+      if (Tok.isNot(AsmToken::Identifier) && Tok.isNot(AsmToken::String)) {
+        Error(L, "unexpected token in .thumb_func directive");
+        return false;
+      }
+
       MCSymbol *Func =
           getParser().getContext().GetOrCreateSymbol(Tok.getIdentifier());
       getParser().getStreamer().EmitThumbFunc(Func);
@@ -8150,11 +8163,12 @@ bool ARMAsmParser::parseDirectiveThumbFu
     }
   }
 
-  if (getLexer().isNot(AsmToken::EndOfStatement))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
 
   NextSymbolIsThumb = true;
-
   return false;
 }
 
@@ -8162,15 +8176,21 @@ bool ARMAsmParser::parseDirectiveThumbFu
 ///  ::= .syntax unified | divided
 bool ARMAsmParser::parseDirectiveSyntax(SMLoc L) {
   const AsmToken &Tok = Parser.getTok();
-  if (Tok.isNot(AsmToken::Identifier))
-    return Error(L, "unexpected token in .syntax directive");
+  if (Tok.isNot(AsmToken::Identifier)) {
+    Error(L, "unexpected token in .syntax directive");
+    return false;
+  }
+
   StringRef Mode = Tok.getString();
-  if (Mode == "unified" || Mode == "UNIFIED")
+  if (Mode == "unified" || Mode == "UNIFIED") {
     Parser.Lex();
-  else if (Mode == "divided" || Mode == "DIVIDED")
-    return Error(L, "'.syntax divided' arm asssembly not supported");
-  else
-    return Error(L, "unrecognized syntax mode in .syntax directive");
+  } else if (Mode == "divided" || Mode == "DIVIDED") {
+    Error(L, "'.syntax divided' arm asssembly not supported");
+    return false;
+  } else {
+    Error(L, "unrecognized syntax mode in .syntax directive");
+    return false;
+  }
 
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     Error(Parser.getTok().getLoc(), "unexpected token in directive");
@@ -8676,7 +8696,7 @@ bool ARMAsmParser::parseDirectiveRegSave
 
   // Parse the register list
   if (parseRegisterList(CO.Operands))
-    return true;
+    return false;
   ARMOperand *Op = (ARMOperand*)CO.Operands[0];
   if (!IsVector && !Op->isRegList()) {
     Error(L, ".save expects GPR registers");

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=199068&r1=199067&r2=199068&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp Sun Jan 12 19:15:39 2014
@@ -2407,13 +2407,17 @@ bool MipsAsmParser::parseDirectiveMipsHa
     reportParseError("expected identifier");
 
   MCSymbol *Sym = getContext().GetOrCreateSymbol(Name);
-  if (getLexer().isNot(AsmToken::Comma))
-    return TokError("unexpected token");
+  if (getLexer().isNot(AsmToken::Comma)) {
+    TokError("unexpected token");
+    return false;
+  }
   Lex();
 
   int64_t Flags = 0;
-  if (Parser.parseAbsoluteExpression(Flags))
-    return TokError("unexpected token");
+  if (Parser.parseAbsoluteExpression(Flags)) {
+    TokError("unexpected token");
+    return false;
+  }
 
   getTargetStreamer().emitMipsHackSTOCG(Sym, Flags);
   return false;
@@ -2421,8 +2425,10 @@ bool MipsAsmParser::parseDirectiveMipsHa
 
 bool MipsAsmParser::parseDirectiveMipsHackELFFlags() {
   int64_t Flags = 0;
-  if (Parser.parseAbsoluteExpression(Flags))
-    return TokError("unexpected token");
+  if (Parser.parseAbsoluteExpression(Flags)) {
+    TokError("unexpected token");
+    return false;
+  }
 
   getTargetStreamer().emitMipsHackELFFlags(Flags);
   return false;
@@ -2499,7 +2505,6 @@ bool MipsAsmParser::parseDirectiveOption
 }
 
 bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {
-
   StringRef IDVal = DirectiveID.getString();
 
   if (IDVal == ".ent") {

Modified: llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp?rev=199068&r1=199067&r2=199068&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp Sun Jan 12 19:15:39 2014
@@ -1373,7 +1373,7 @@ bool PPCAsmParser::ParseDirectiveWord(un
     for (;;) {
       const MCExpr *Value;
       if (getParser().parseExpression(Value))
-        return true;
+        return false;
 
       getParser().getStreamer().EmitValue(Value, Size);
 
@@ -1397,8 +1397,10 @@ bool PPCAsmParser::ParseDirectiveTC(unsi
   while (getLexer().isNot(AsmToken::EndOfStatement)
          && getLexer().isNot(AsmToken::Comma))
     Parser.Lex();
-  if (getLexer().isNot(AsmToken::Comma))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::Comma)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
   Parser.Lex();
 
   // Align to word size.
@@ -1412,8 +1414,10 @@ bool PPCAsmParser::ParseDirectiveTC(unsi
 ///  ::= .machine [ cpu | "push" | "pop" ]
 bool PPCAsmParser::ParseDirectiveMachine(SMLoc L) {
   if (getLexer().isNot(AsmToken::Identifier) &&
-      getLexer().isNot(AsmToken::String))
-    return Error(L, "unexpected token in directive");
+      getLexer().isNot(AsmToken::String)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
 
   StringRef CPU = Parser.getTok().getIdentifier();
   Parser.Lex();
@@ -1423,11 +1427,15 @@ bool PPCAsmParser::ParseDirectiveMachine
   // Implement ".machine any" (by doing nothing) for the benefit
   // of existing assembler code.  Likewise, we can then implement
   // ".machine push" and ".machine pop" as no-op.
-  if (CPU != "any" && CPU != "push" && CPU != "pop")
-    return Error(L, "unrecognized machine type");
+  if (CPU != "any" && CPU != "push" && CPU != "pop") {
+    Error(L, "unrecognized machine type");
+    return false;
+  }
 
-  if (getLexer().isNot(AsmToken::EndOfStatement))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
 
   return false;
 }
@@ -1436,8 +1444,10 @@ bool PPCAsmParser::ParseDirectiveMachine
 ///  ::= .machine cpu-identifier
 bool PPCAsmParser::ParseDarwinDirectiveMachine(SMLoc L) {
   if (getLexer().isNot(AsmToken::Identifier) &&
-      getLexer().isNot(AsmToken::String))
-    return Error(L, "unexpected token in directive");
+      getLexer().isNot(AsmToken::String)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
 
   StringRef CPU = Parser.getTok().getIdentifier();
   Parser.Lex();
@@ -1445,16 +1455,24 @@ bool PPCAsmParser::ParseDarwinDirectiveM
   // FIXME: this is only the 'default' set of cpu variants.
   // However we don't act on this information at present, this is simply
   // allowing parsing to proceed with minimal sanity checking.
-  if (CPU != "ppc7400" && CPU != "ppc" && CPU != "ppc64")
-    return Error(L, "unrecognized cpu type");
+  if (CPU != "ppc7400" && CPU != "ppc" && CPU != "ppc64") {
+    Error(L, "unrecognized cpu type");
+    return false;
+  }
 
-  if (isPPC64() && (CPU == "ppc7400" || CPU == "ppc"))
-    return Error(L, "wrong cpu type specified for 64bit");
-  if (!isPPC64() && CPU == "ppc64")
-    return Error(L, "wrong cpu type specified for 32bit");
+  if (isPPC64() && (CPU == "ppc7400" || CPU == "ppc")) {
+    Error(L, "wrong cpu type specified for 64bit");
+    return false;
+  }
+  if (!isPPC64() && CPU == "ppc64") {
+    Error(L, "wrong cpu type specified for 32bit");
+    return false;
+  }
 
-  if (getLexer().isNot(AsmToken::EndOfStatement))
-    return Error(L, "unexpected token in directive");
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    Error(L, "unexpected token in directive");
+    return false;
+  }
 
   return false;
 }

Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp?rev=199068&r1=199067&r2=199068&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
+++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Sun Jan 12 19:15:39 2014
@@ -2693,11 +2693,9 @@ bool X86AsmParser::ParseDirective(AsmTok
   } else if (IDVal.startswith(".intel_syntax")) {
     getParser().setAssemblerDialect(1);
     if (getLexer().isNot(AsmToken::EndOfStatement)) {
-      if(Parser.getTok().getString() == "noprefix") {
-        // FIXME : Handle noprefix
+      // FIXME: Handle noprefix
+      if (Parser.getTok().getString() == "noprefix")
         Parser.Lex();
-      } else
-        return true;
     }
     return false;
   }
@@ -2711,7 +2709,7 @@ bool X86AsmParser::ParseDirectiveWord(un
     for (;;) {
       const MCExpr *Value;
       if (getParser().parseExpression(Value))
-        return true;
+        return false;
 
       getParser().getStreamer().EmitValue(Value, Size);
 
@@ -2719,8 +2717,10 @@ bool X86AsmParser::ParseDirectiveWord(un
         break;
 
       // FIXME: Improve diagnostic.
-      if (getLexer().isNot(AsmToken::Comma))
-        return Error(L, "unexpected token in directive");
+      if (getLexer().isNot(AsmToken::Comma)) {
+        Error(L, "unexpected token in directive");
+        return false;
+      }
       Parser.Lex();
     }
   }
@@ -2738,7 +2738,7 @@ bool X86AsmParser::ParseDirectiveCode(St
       SwitchMode(X86::Mode16Bit);
       getParser().getStreamer().EmitAssemblerFlag(MCAF_Code16);
     }
-  } else  if (IDVal == ".code32") {
+  } else if (IDVal == ".code32") {
     Parser.Lex();
     if (!is32BitMode()) {
       SwitchMode(X86::Mode32Bit);
@@ -2751,7 +2751,8 @@ bool X86AsmParser::ParseDirectiveCode(St
       getParser().getStreamer().EmitAssemblerFlag(MCAF_Code64);
     }
   } else {
-    return Error(L, "unexpected directive " + IDVal);
+    Error(L, "unknown directive " + IDVal);
+    return false;
   }
 
   return false;

Added: llvm/trunk/test/MC/X86/x86-target-directives.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-target-directives.s?rev=199068&view=auto
==============================================================================
--- llvm/trunk/test/MC/X86/x86-target-directives.s (added)
+++ llvm/trunk/test/MC/X86/x86-target-directives.s Sun Jan 12 19:15:39 2014
@@ -0,0 +1,7 @@
+# RUN: not llvm-mc -triple i386 -filetype asm -o - %s 2>&1 | FileCheck %s
+
+	.code42
+
+# CHECK: unknown directive .code42
+# CHECK-NOT: unknown directive
+





More information about the llvm-commits mailing list