From ec6789f4f97ca1701c163132b6e3388366463090 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Mon, 6 Sep 2010 20:08:02 +0000 Subject: [PATCH] have tblgen detect when an instruction would have matched, but failed because a subtarget feature was not enabled. Use this to remove a bunch of hacks from the X86AsmParser for rejecting things like popfl in 64-bit mode. Previously these hacks weren't needed, but were important to get a message better than "invalid instruction" when used in the wrong mode. This also fixes bugs where pushal would not be rejected correctly in 32-bit mode (just pusha). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@113166 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/X86/AsmParser/X86AsmParser.cpp | 78 +++++++++++++---------- utils/TableGen/AsmMatcherEmitter.cpp | 17 +++-- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/lib/Target/X86/AsmParser/X86AsmParser.cpp b/lib/Target/X86/AsmParser/X86AsmParser.cpp index 10c2b9c4616..54288769a09 100644 --- a/lib/Target/X86/AsmParser/X86AsmParser.cpp +++ b/lib/Target/X86/AsmParser/X86AsmParser.cpp @@ -615,28 +615,13 @@ X86Operand *X86ATTAsmParser::ParseMemOperand(unsigned SegReg, SMLoc MemStart) { bool X86ATTAsmParser:: ParseInstruction(StringRef Name, SMLoc NameLoc, SmallVectorImpl &Operands) { - // The various flavors of pushf and popf use Requires and - // Requires, but the assembler doesn't yet implement that. - // For now, just do a manual check to prevent silent misencoding. - if (Is64Bit) { - if (Name == "popfl") - return Error(NameLoc, "popfl cannot be encoded in 64-bit mode"); - if (Name == "pushfl") - return Error(NameLoc, "pushfl cannot be encoded in 64-bit mode"); - if (Name == "pusha") - return Error(NameLoc, "pusha cannot be encoded in 64-bit mode"); - } else { - if (Name == "popfq") - return Error(NameLoc, "popfq cannot be encoded in 32-bit mode"); - if (Name == "pushfq") - return Error(NameLoc, "pushfq cannot be encoded in 32-bit mode"); - } // The "Jump if rCX Zero" form jcxz is not allowed in 64-bit mode and // the form jrcxz is not allowed in 32-bit mode. if (Is64Bit) { - if (Name == "jcxz") - return Error(NameLoc, "jcxz cannot be encoded in 64-bit mode"); + // FIXME: We can do jcxz/jecxz, we just don't have the encoding right yet. + if (Name == "jcxz" || Name == "jecxz") + return Error(NameLoc, Name + " cannot be encoded in 64-bit mode"); } else { if (Name == "jrcxz") return Error(NameLoc, "jrcxz cannot be encoded in 32-bit mode"); @@ -881,8 +866,15 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc, assert(!Operands.empty() && "Unexpect empty operand list!"); // First, try a direct match. - if (MatchInstructionImpl(Operands, Inst) == Match_Success) + switch (MatchInstructionImpl(Operands, Inst)) { + case Match_Success: return false; + case Match_MissingFeature: + Error(IDLoc, "instruction requires a CPU feature not currently enabled"); + return true; + default: + break; + } // FIXME: Ideally, we would only attempt suffix matches for things which are // valid prefixes, and we could just infer the right unambiguous @@ -901,13 +893,13 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc, // Check for the various suffix matches. Tmp[Base.size()] = 'b'; - bool MatchB = MatchInstructionImpl(Operands, Inst) != Match_Success; + MatchResultTy MatchB = MatchInstructionImpl(Operands, Inst); Tmp[Base.size()] = 'w'; - bool MatchW = MatchInstructionImpl(Operands, Inst) != Match_Success; + MatchResultTy MatchW = MatchInstructionImpl(Operands, Inst); Tmp[Base.size()] = 'l'; - bool MatchL = MatchInstructionImpl(Operands, Inst) != Match_Success; + MatchResultTy MatchL = MatchInstructionImpl(Operands, Inst); Tmp[Base.size()] = 'q'; - bool MatchQ = MatchInstructionImpl(Operands, Inst) != Match_Success; + MatchResultTy MatchQ = MatchInstructionImpl(Operands, Inst); // Restore the old token. Op->setTokenValue(Base); @@ -915,23 +907,26 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc, // If exactly one matched, then we treat that as a successful match (and the // instruction will already have been filled in correctly, since the failing // matches won't have modified it). - if (MatchB + MatchW + MatchL + MatchQ == 3) + unsigned NumSuccessfulMatches = + (MatchB == Match_Success) + (MatchW == Match_Success) + + (MatchL == Match_Success) + (MatchQ == Match_Success); + if (NumSuccessfulMatches == 1) return false; - // Otherwise, the match failed. + // Otherwise, the match failed, try to produce a decent error message. // If we had multiple suffix matches, then identify this as an ambiguous // match. - if (MatchB + MatchW + MatchL + MatchQ != 4) { + if (NumSuccessfulMatches > 1) { char MatchChars[4]; unsigned NumMatches = 0; - if (!MatchB) + if (MatchB == Match_Success) MatchChars[NumMatches++] = 'b'; - if (!MatchW) + if (MatchW == Match_Success) MatchChars[NumMatches++] = 'w'; - if (!MatchL) + if (MatchL == Match_Success) MatchChars[NumMatches++] = 'l'; - if (!MatchQ) + if (MatchQ == Match_Success) MatchChars[NumMatches++] = 'q'; SmallString<126> Msg; @@ -946,11 +941,26 @@ X86ATTAsmParser::MatchInstruction(SMLoc IDLoc, } OS << ")"; Error(IDLoc, OS.str()); - } else { - // FIXME: We should give nicer diagnostics about the exact failure. - Error(IDLoc, "unrecognized instruction"); + return true; } - + + unsigned NumMatchFailures = + (MatchB == Match_Fail) + (MatchW == Match_Fail) + + (MatchL == Match_Fail) + (MatchQ == Match_Fail); + + + // If one instruction matched with a missing feature, report this as a + // missing feature. + if ((MatchB == Match_MissingFeature) + (MatchW == Match_MissingFeature) + + (MatchL == Match_MissingFeature) + (MatchQ == Match_MissingFeature) == 1&& + NumMatchFailures == 3) { + Error(IDLoc, "instruction requires a CPU feature not currently enabled"); + return true; + } + + // If all of these were an outright failure, report it in a useless way. + // FIXME: We should give nicer diagnostics about the exact failure. + Error(IDLoc, "unrecognized instruction"); return true; } diff --git a/utils/TableGen/AsmMatcherEmitter.cpp b/utils/TableGen/AsmMatcherEmitter.cpp index 1ecc1327b40..c0d95fbc1ed 100644 --- a/utils/TableGen/AsmMatcherEmitter.cpp +++ b/utils/TableGen/AsmMatcherEmitter.cpp @@ -1549,7 +1549,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) { OS << " unsigned ComputeAvailableFeatures(const " << Target.getName() << "Subtarget *Subtarget) const;\n"; OS << " enum MatchResultTy {\n"; - OS << " Match_Success, Match_Fail\n"; + OS << " Match_Success, Match_Fail, Match_MissingFeature\n"; OS << " };\n"; OS << " MatchResultTy MatchInstructionImpl(const SmallVectorImpl" << " &Operands, MCInst &Inst);\n\n"; @@ -1678,21 +1678,24 @@ void AsmMatcherEmitter::run(raw_ostream &OS) { // Emit code to search the table. OS << " // Search the table.\n"; + OS << " bool HadMatchOtherThanFeatures = false;\n"; OS << " for (const MatchEntry *it = MatchTable, " << "*ie = MatchTable + " << Info.Instructions.size() << "; it != ie; ++it) {\n"; - // Emit check that the required features are available. - OS << " if ((AvailableFeatures & it->RequiredFeatures) " - << "!= it->RequiredFeatures)\n"; - OS << " continue;\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"; } + + // Emit check that the required features are available. + OS << " if ((AvailableFeatures & it->RequiredFeatures) " + << "!= it->RequiredFeatures) {\n"; + OS << " HadMatchOtherThanFeatures = true;\n"; + OS << " continue;\n"; + OS << " }\n"; OS << "\n"; OS << " ConvertToMCInst(it->ConvertFn, Inst, it->Opcode, Operands);\n"; @@ -1706,6 +1709,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) { OS << " return Match_Success;\n"; OS << " }\n\n"; + OS << " // Okay, we had no match. Try to return a useful error code.\n"; + OS << " if (HadMatchOtherThanFeatures) return Match_MissingFeature;\n"; OS << " return Match_Fail;\n"; OS << "}\n\n"; -- 2.34.1