From ce4a3355d96971e7edcbff3c1975f83e1ddcb8f2 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Mon, 6 Sep 2010 22:11:18 +0000 Subject: [PATCH] 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 --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 3 +- lib/Target/X86/AsmParser/X86AsmParser.cpp | 27 ++++++++++++----- utils/TableGen/AsmMatcherEmitter.cpp | 37 +++++++++++++++++------ 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index 8507070fc66..f2099d29386 100644 --- 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 &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 20ed232eccd..2aa632d4298 100644 --- 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 4d8a7957c61..238cdd9269c 100644 --- 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" - << " &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" << " &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";