in the case where an instruction only has one implementation

of a mneumonic, report operand errors with better location
info.  For example, we now report:

t.s:6:14: error: invalid operand for instruction
        cwtl $1
             ^

but we fail for common cases like:

t.s:11:4: error: invalid operand for instruction
   addl $1, $1
   ^

because we don't know if this is supposed to be the reg/imm or imm/reg
form.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@113178 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 8507070fc6615d729014a17913d4c2ccc7f025ba..f2099d29386924c68110b77006fa44c6f3137414 100644 (file)
--- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -83,7 +83,8 @@ private:
   bool MatchInstruction(SMLoc IDLoc,
                         const SmallVectorImpl<MCParsedAsmOperand*> &Operands,
                         MCInst &Inst) {
-    if (MatchInstructionImpl(Operands, Inst) == Match_Success)
+    unsigned ErrorInfo;
+    if (MatchInstructionImpl(Operands, Inst, ErrorInfo) == Match_Success)
       return false;
 
     // FIXME: We should give nicer diagnostics about the exact failure.

diff --git a/lib/Target/X86/AsmParser/X86AsmParser.cpp b/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 20ed232eccd1afb5462e226f6d8ef3d74646d3bd..2aa632d42980ef6b8a607b973278a9fe6fcbbc38 100644 (file)
--- a/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -863,9 +863,10 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc,
   assert(!Operands.empty() && "Unexpect empty operand list!");
 
   bool WasOriginallyInvalidOperand = false;
+  unsigned OrigErrorInfo;
   
   // First, try a direct match.
-  switch (MatchInstructionImpl(Operands, Inst)) {
+  switch (MatchInstructionImpl(Operands, Inst, OrigErrorInfo)) {
   case Match_Success:
     return false;
   case Match_MissingFeature:
@@ -895,13 +896,14 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc,
 
   // Check for the various suffix matches.
   Tmp[Base.size()] = 'b';
-  MatchResultTy MatchB = MatchInstructionImpl(Operands, Inst);
+  unsigned BErrorInfo, WErrorInfo, LErrorInfo, QErrorInfo;
+  MatchResultTy MatchB = MatchInstructionImpl(Operands, Inst, BErrorInfo);
   Tmp[Base.size()] = 'w';
-  MatchResultTy MatchW = MatchInstructionImpl(Operands, Inst);
+  MatchResultTy MatchW = MatchInstructionImpl(Operands, Inst, WErrorInfo);
   Tmp[Base.size()] = 'l';
-  MatchResultTy MatchL = MatchInstructionImpl(Operands, Inst);
+  MatchResultTy MatchL = MatchInstructionImpl(Operands, Inst, LErrorInfo);
   Tmp[Base.size()] = 'q';
-  MatchResultTy MatchQ = MatchInstructionImpl(Operands, Inst);
+  MatchResultTy MatchQ = MatchInstructionImpl(Operands, Inst, QErrorInfo);
 
   // Restore the old token.
   Op->setTokenValue(Base);
@@ -952,10 +954,19 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc,
   // mnemonic was invalid.
   if ((MatchB == Match_MnemonicFail) && (MatchW == Match_MnemonicFail) &&
       (MatchL == Match_MnemonicFail) && (MatchQ == Match_MnemonicFail)) {
-    if (WasOriginallyInvalidOperand)
-      Error(IDLoc, "invalid operand for instruction");
-    else
+    if (!WasOriginallyInvalidOperand) {
       Error(IDLoc, "invalid instruction mnemonic '" + Base + "'"); 
+      return true;
+    }
+
+    // Recover location info for the operand if we know which was the problem.
+    SMLoc ErrorLoc = IDLoc;
+    if (OrigErrorInfo != ~0U) {
+      ErrorLoc = ((X86Operand*)Operands[OrigErrorInfo])->getStartLoc();
+      if (ErrorLoc == SMLoc()) ErrorLoc = IDLoc;
+    }
+
+    Error(ErrorLoc, "invalid operand for instruction");
     return true;
   }
   

diff --git a/utils/TableGen/AsmMatcherEmitter.cpp b/utils/TableGen/AsmMatcherEmitter.cpp
index 4d8a7957c61b5364bef9e8f30ce79a09c8c16d69..238cdd9269c6e777c36b5dc4f7d36596a6541cd2 100644 (file)
--- a/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1563,7 +1563,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "    Match_MissingFeature\n";
   OS << "  };\n";
   OS << "  MatchResultTy MatchInstructionImpl(const SmallVectorImpl<MCParsedAsmOperand*>"
-     << " &Operands, MCInst &Inst);\n\n";
+     << " &Operands, MCInst &Inst, unsigned &ErrorInfo);\n\n";
   OS << "#endif // GET_ASSEMBLER_HEADER_INFO\n\n";
 
   
@@ -1679,16 +1679,19 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
      << Target.getName() << ClassName << "::\n"
      << "MatchInstructionImpl(const SmallVectorImpl<MCParsedAsmOperand*>"
      << " &Operands,\n";
-  OS << "                     MCInst &Inst) {\n";
+  OS << "                     MCInst &Inst, unsigned &ErrorInfo) {\n";
 
   // Emit code to get the available features.
   OS << "  // Get the current feature set.\n";
   OS << "  unsigned AvailableFeatures = getAvailableFeatures();\n\n";
+  OS << "  ErrorInfo = 0;\n";
 
   // Emit code to compute the class list for this operand vector.
   OS << "  // Eliminate obvious mismatches.\n";
-  OS << "  if (Operands.size() > " << MaxNumOperands << "+1)\n";
-  OS << "    return Match_InvalidOperand;\n\n";
+  OS << "  if (Operands.size() > " << (MaxNumOperands+1) << ") {\n";
+  OS << "    ErrorInfo = " << (MaxNumOperands+1) << ";\n";
+  OS << "    return Match_InvalidOperand;\n";
+  OS << "  }\n\n";
 
   OS << "  // Compute the class list for this operand vector.\n";
   OS << "  MatchClassKind Classes[" << MaxNumOperands << "];\n";
@@ -1696,8 +1699,10 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "    Classes[i-1] = ClassifyOperand(Operands[i]);\n\n";
 
   OS << "    // Check for invalid operands before matching.\n";
-  OS << "    if (Classes[i-1] == InvalidMatchClass)\n";
+  OS << "    if (Classes[i-1] == InvalidMatchClass) {\n";
+  OS << "      ErrorInfo = i;\n";
   OS << "      return Match_InvalidOperand;\n";
+  OS << "    }\n";
   OS << "  }\n\n";
 
   OS << "  // Mark unused classes.\n";
@@ -1729,11 +1734,22 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "    assert(Mnemonic == it->Mnemonic);\n";
   
   // Emit check that the subclasses match.
-  for (unsigned i = 0; i != MaxNumOperands; ++i) {
-    OS << "    if (!IsSubclass(Classes[" 
-       << i << "], it->Classes[" << i << "]))\n";
-    OS << "      continue;\n";
-  }
+  OS << "    bool OperandsValid = true;\n";
+  OS << "    for (unsigned i = 0; i != " << MaxNumOperands << "; ++i) {\n";
+  OS << "      if (IsSubclass(Classes[i], it->Classes[i]))\n";
+  OS << "        continue;\n";
+  OS << "      // If there is only one instruction with this opcode, report\n";
+  OS << "      // this as an operand error with location info.\n";
+  OS << "      if (MnemonicRange.first+1 == ie) {\n";
+  OS << "        ErrorInfo = i+1;\n";
+  OS << "        return Match_InvalidOperand;\n";
+  OS << "      }\n";
+  OS << "      // Otherwise, just reject this instance of the mnemonic.\n";
+  OS << "      OperandsValid = false;\n";
+  OS << "      break;\n";
+  OS << "    }\n\n";
+  
+  OS << "    if (!OperandsValid) continue;\n";
 
   // Emit check that the required features are available.
   OS << "    if ((AvailableFeatures & it->RequiredFeatures) "
@@ -1756,6 +1772,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
 
   OS << "  // Okay, we had no match.  Try to return a useful error code.\n";
   OS << "  if (HadMatchOtherThanFeatures) return Match_MissingFeature;\n";
+  OS << "  ErrorInfo = ~0U;\n";
   OS << "  return Match_InvalidOperand;\n";
   OS << "}\n\n";