Index: lib/Target/ARM/ARM.td =================================================================== --- lib/Target/ARM/ARM.td +++ lib/Target/ARM/ARM.td @@ -867,6 +867,10 @@ bit isMCAsmWriter = 1; } +def ARMAsmParser : AsmParser { + bit ReportMultipleNearMisses = 1; +} + def ARMAsmParserVariant : AsmParserVariant { int Variant = 0; string Name = "ARM"; @@ -877,5 +881,6 @@ // Pull in Instruction Info: let InstructionSet = ARMInstrInfo; let AssemblyWriters = [ARMAsmWriter]; + let AssemblyParsers = [ARMAsmParser]; let AssemblyParserVariants = [ARMAsmParserVariant]; } Index: lib/Target/ARM/AsmParser/ARMAsmParser.cpp =================================================================== --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp +++ lib/Target/ARM/AsmParser/ARMAsmParser.cpp @@ -12,6 +12,7 @@ #include "MCTargetDesc/ARMBaseInfo.h" #include "MCTargetDesc/ARMMCExpr.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSwitch.h" @@ -67,6 +68,11 @@ clEnumValN(ImplicitItModeTy::ThumbOnly, "thumb", "Warn in ARM, emit implicit ITs in Thumb"))); +cl::opt +DevDiags("arm-asm-parser-dev-diags", cl::init(false), + cl::desc("Use extended diagnostics, which include implementation " + "details useful for development")); + class ARMOperand; enum VectorLaneTy { NoLanes, AllLanes, IndexedLane }; @@ -561,8 +567,22 @@ uint64_t &ErrorInfo, bool MatchingInlineAsm) override; unsigned MatchInstruction(OperandVector &Operands, MCInst &Inst, - uint64_t &ErrorInfo, bool MatchingInlineAsm, - bool &EmitInITBlock, MCStreamer &Out); + SmallVectorImpl &NearMisses, + bool MatchingInlineAsm, bool &EmitInITBlock, + MCStreamer &Out); + + struct NearMissMessage { + SMLoc Loc; + SmallString<128> Message; + }; + + const char *getOperandMatchFailDiag(ARMMatchResultTy Error); + void FilterNearMisses(SmallVectorImpl &NearMissesIn, + SmallVectorImpl &NearMissesOut, + SMLoc IDLoc, OperandVector &Operands); + void ReportNearMisses(SmallVectorImpl &NearMisses, SMLoc IDLoc, + OperandVector &Operands); + void onLabelParsed(MCSymbol *Symbol) override; }; } // end anonymous namespace @@ -9062,19 +9082,19 @@ } unsigned ARMAsmParser::MatchInstruction(OperandVector &Operands, MCInst &Inst, - uint64_t &ErrorInfo, + SmallVectorImpl &NearMisses, bool MatchingInlineAsm, bool &EmitInITBlock, MCStreamer &Out) { // If we can't use an implicit IT block here, just match as normal. if (inExplicitITBlock() || !isThumbTwo() || !useImplicitITThumb()) - return MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm); + return MatchInstructionImpl(Operands, Inst, &NearMisses, MatchingInlineAsm); // Try to match the instruction in an extension of the current IT block (if // there is one). if (inImplicitITBlock()) { extendImplicitITBlock(ITState.Cond); - if (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm) == + if (MatchInstructionImpl(Operands, Inst, nullptr, MatchingInlineAsm) == Match_Success) { // The match succeded, but we still have to check that the instruction is // valid in this implicit IT block. @@ -9100,7 +9120,7 @@ // Finish the current IT block, and try to match outside any IT block. flushPendingInstructions(Out); unsigned PlainMatchResult = - MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm); + MatchInstructionImpl(Operands, Inst, &NearMisses, MatchingInlineAsm); if (PlainMatchResult == Match_Success) { const MCInstrDesc &MCID = MII.get(Inst.getOpcode()); if (MCID.isPredicable()) { @@ -9127,7 +9147,7 @@ // condition, so we create an IT block with a dummy condition, and fix it up // once we know the actual condition. startImplicitITBlock(); - if (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm) == + if (MatchInstructionImpl(Operands, Inst, nullptr, MatchingInlineAsm) == Match_Success) { const MCInstrDesc &MCID = MII.get(Inst.getOpcode()); if (MCID.isPredicable()) { @@ -9155,7 +9175,8 @@ unsigned MatchResult; bool PendConditionalInstruction = false; - MatchResult = MatchInstruction(Operands, Inst, ErrorInfo, MatchingInlineAsm, + SmallVector NearMisses; + MatchResult = MatchInstruction(Operands, Inst, NearMisses, MatchingInlineAsm, PendConditionalInstruction, Out); switch (MatchResult) { @@ -9205,94 +9226,12 @@ Out.EmitInstruction(Inst, getSTI()); } return false; - case Match_MissingFeature: { - assert(ErrorInfo && "Unknown missing feature!"); - // Special case the error message for the very common case where only - // a single subtarget feature is missing (Thumb vs. ARM, e.g.). - std::string Msg = "instruction requires:"; - uint64_t Mask = 1; - for (unsigned i = 0; i < (sizeof(ErrorInfo)*8-1); ++i) { - if (ErrorInfo & Mask) { - Msg += " "; - Msg += getSubtargetFeatureName(ErrorInfo & Mask); - } - Mask <<= 1; - } - return Error(IDLoc, Msg); - } - case Match_InvalidOperand: { - SMLoc ErrorLoc = IDLoc; - if (ErrorInfo != ~0ULL) { - if (ErrorInfo >= Operands.size()) - return Error(IDLoc, "too few operands for instruction"); - - ErrorLoc = ((ARMOperand &)*Operands[ErrorInfo]).getStartLoc(); - if (ErrorLoc == SMLoc()) ErrorLoc = IDLoc; - } - - return Error(ErrorLoc, "invalid operand for instruction"); - } + case Match_NearMisses: + ReportNearMisses(NearMisses, IDLoc, Operands); + return true; case Match_MnemonicFail: return Error(IDLoc, "invalid instruction", ((ARMOperand &)*Operands[0]).getLocRange()); - case Match_RequiresNotITBlock: - return Error(IDLoc, "flag setting instruction only valid outside IT block"); - case Match_RequiresITBlock: - return Error(IDLoc, "instruction only valid inside IT block"); - case Match_RequiresV6: - return Error(IDLoc, "instruction variant requires ARMv6 or later"); - case Match_RequiresThumb2: - return Error(IDLoc, "instruction variant requires Thumb2"); - case Match_RequiresV8: - return Error(IDLoc, "instruction variant requires ARMv8 or later"); - case Match_RequiresFlagSetting: - return Error(IDLoc, "no flag-preserving variant of this instruction available"); - case Match_ImmRange0_15: { - SMLoc ErrorLoc = ((ARMOperand &)*Operands[ErrorInfo]).getStartLoc(); - if (ErrorLoc == SMLoc()) ErrorLoc = IDLoc; - return Error(ErrorLoc, "immediate operand must be in the range [0,15]"); - } - case Match_ImmRange0_239: { - SMLoc ErrorLoc = ((ARMOperand &)*Operands[ErrorInfo]).getStartLoc(); - if (ErrorLoc == SMLoc()) ErrorLoc = IDLoc; - return Error(ErrorLoc, "immediate operand must be in the range [0,239]"); - } - case Match_AlignedMemoryRequiresNone: - case Match_DupAlignedMemoryRequiresNone: - case Match_AlignedMemoryRequires16: - case Match_DupAlignedMemoryRequires16: - case Match_AlignedMemoryRequires32: - case Match_DupAlignedMemoryRequires32: - case Match_AlignedMemoryRequires64: - case Match_DupAlignedMemoryRequires64: - case Match_AlignedMemoryRequires64or128: - case Match_DupAlignedMemoryRequires64or128: - case Match_AlignedMemoryRequires64or128or256: - { - SMLoc ErrorLoc = ((ARMOperand &)*Operands[ErrorInfo]).getAlignmentLoc(); - if (ErrorLoc == SMLoc()) ErrorLoc = IDLoc; - switch (MatchResult) { - default: - llvm_unreachable("Missing Match_Aligned type"); - case Match_AlignedMemoryRequiresNone: - case Match_DupAlignedMemoryRequiresNone: - return Error(ErrorLoc, "alignment must be omitted"); - case Match_AlignedMemoryRequires16: - case Match_DupAlignedMemoryRequires16: - return Error(ErrorLoc, "alignment must be 16 or omitted"); - case Match_AlignedMemoryRequires32: - case Match_DupAlignedMemoryRequires32: - return Error(ErrorLoc, "alignment must be 32 or omitted"); - case Match_AlignedMemoryRequires64: - case Match_DupAlignedMemoryRequires64: - return Error(ErrorLoc, "alignment must be 64 or omitted"); - case Match_AlignedMemoryRequires64or128: - case Match_DupAlignedMemoryRequires64or128: - return Error(ErrorLoc, "alignment must be 64, 128 or omitted"); - case Match_AlignedMemoryRequires64or128or256: - return Error(ErrorLoc, "alignment must be 64, 128, 256 or omitted"); - } - } } llvm_unreachable("Implement any new match types added!"); @@ -10302,6 +10241,213 @@ #define GET_MATCHER_IMPLEMENTATION #include "ARMGenAsmMatcher.inc" +const char *ARMAsmParser::getOperandMatchFailDiag(ARMMatchResultTy Error) { + switch (Error) { + case Match_ImmRange0_15: + return "immediate operand must be in the range [0,15]"; + case Match_ImmRange0_239: + return "immediate operand must be in the range [0,239]"; + case Match_AlignedMemoryRequiresNone: + case Match_DupAlignedMemoryRequiresNone: + return "alignment must be omitted"; + case Match_AlignedMemoryRequires16: + case Match_DupAlignedMemoryRequires16: + return "alignment must be 16 or omitted"; + case Match_AlignedMemoryRequires32: + case Match_DupAlignedMemoryRequires32: + return "alignment must be 32 or omitted"; + case Match_AlignedMemoryRequires64: + case Match_DupAlignedMemoryRequires64: + return "alignment must be 64 or omitted"; + case Match_AlignedMemoryRequires64or128: + case Match_DupAlignedMemoryRequires64or128: + return "alignment must be 64, 128 or omitted"; + case Match_AlignedMemoryRequires64or128or256: + return "alignment must be 64, 128, 256 or omitted"; + default: + return nullptr; + } +} + +// Process the list of near-misses, throwing away ones we don't want to report +// to the user, and converting the rest to a source location and string that +// should be reported. +void +ARMAsmParser::FilterNearMisses(SmallVectorImpl &NearMissesIn, + SmallVectorImpl &NearMissesOut, + SMLoc IDLoc, OperandVector &Operands) { + // TODO: If operand didn't match, sub in a dummy one and run target + // predicate, so that we can avoid reporting near-misses that are invalid? + // TODO: Many operand types dont have SuperClasses set, so we report + // redundant ones. + // TODO: Some operands are superclasses of registers (e.g. + // MCK_RegShiftedImm), we don't have any way to represent that currently. + // TODO: This is not all ARM-specific, can some of it be factored out? + + // Record some information about near-misses that we have already seen, so + // that we can avoid reporting redundant ones. For example, if there are + // variants of an instruction that take 8- and 16-bit immediates, we want + // to only report the widest one. + std::multimap OperandMissesSeen; + SmallSet FeatureMissesSeen; + + // Process the near-misses in reverse order, so that we see more general ones + // first, and so can avoid emitting more specific ones. + for (NearMissInfo &I : reverse(NearMissesIn)) { + switch (I.getKind()) { + case NearMissInfo::NearMissOperand: { + SMLoc OperandLoc = + ((ARMOperand &)*Operands[I.getOperandIndex()]).getStartLoc(); + const char *OperandDiag = + getOperandMatchFailDiag((ARMMatchResultTy)I.getOperandError()); + + // If we have already emitted a message for a superclass, don't also report + // the sub-class. We consider all operand classes that we don't have a + // specialised diagnostic for to be equal for the propose of this check, + // so that we don't report the generic error multiple times on the same + // operand. + unsigned DupCheckMatchClass = OperandDiag ? I.getOperandClass() : ~0U; + auto PrevReports = OperandMissesSeen.equal_range(I.getOperandIndex()); + if (std::any_of(PrevReports.first, PrevReports.second, + [DupCheckMatchClass]( + const std::pair Pair) { + if (DupCheckMatchClass == ~0U) + return Pair.second == ~0U; + else + return isSubclass((MatchClassKind)DupCheckMatchClass, + (MatchClassKind)Pair.second); + })) + break; + OperandMissesSeen.insert( + std::make_pair(I.getOperandIndex(), DupCheckMatchClass)); + + NearMissMessage Message; + Message.Loc = OperandLoc; + raw_svector_ostream OS(Message.Message); + if (OperandDiag) { + OS << OperandDiag; + } else if (I.getOperandClass() == InvalidMatchClass) { + OS << "too many operands for instruction"; + } else { + OS << "invalid operand for instruction"; + if (DevDiags) { + OS << " class" << I.getOperandClass() << ", error " + << I.getOperandError() << ", opcode " + << MII.getName(I.getOpcode()); + } + } + NearMissesOut.emplace_back(Message); + break; + } + case NearMissInfo::NearMissFeature: { + uint64_t MissingFeatures = I.getFeatures(); + // Don't report the same set of features twice. + if (FeatureMissesSeen.count(MissingFeatures)) + break; + FeatureMissesSeen.insert(MissingFeatures); + + // Special case: don't report a feature set which includes arm-mode for + // targets that don't have ARM mode. + if ((MissingFeatures & Feature_IsARM) && !hasARM()) + break; + // Don't report any near-misses that both require switching instruction + // set, and adding other subtarget features. + if (isThumb() && (MissingFeatures & Feature_IsARM) && (MissingFeatures & ~Feature_IsARM)) + break; + if (!isThumb() && (MissingFeatures & Feature_IsThumb) && (MissingFeatures & ~Feature_IsThumb)) + break; + + NearMissMessage Message; + Message.Loc = IDLoc; + raw_svector_ostream OS(Message.Message); + + OS << "instruction requires:"; + uint64_t Mask = 1; + bool FirstFeature = true; + for (unsigned MaskPos = 0; MaskPos < (sizeof(MissingFeatures) * 8 - 1); + ++MaskPos) { + if (MissingFeatures & Mask) { + if (!FirstFeature) + OS << ","; + FirstFeature = false; + OS << " " << getSubtargetFeatureName(MissingFeatures & Mask); + } + Mask <<= 1; + } + NearMissesOut.emplace_back(Message); + + break; + } + case NearMissInfo::NearMissPredicate: { + NearMissMessage Message; + Message.Loc = IDLoc; + switch (I.getPredicateError()) { + case Match_RequiresNotITBlock: + Message.Message = "flag setting instruction only valid outside IT block"; + break; + case Match_RequiresITBlock: + Message.Message = "instruction only valid inside IT block"; + break; + case Match_RequiresV6: + Message.Message = "instruction variant requires ARMv6 or later"; + break; + case Match_RequiresThumb2: + Message.Message = "instruction variant requires Thumb2"; + break; + case Match_RequiresV8: + Message.Message = "instruction variant requires ARMv8 or later"; + break; + case Match_RequiresFlagSetting: + Message.Message = "no flag-preserving variant of this instruction available"; + break; + case Match_InvalidOperand: + Message.Message = "invalid operand for instruction"; + break; + default: + llvm_unreachable("Unhandled target predicate error"); + break; + } + NearMissesOut.emplace_back(Message); + break; + } + case NearMissInfo::NearMissTooFewOperands: { + SMLoc EndLoc = ((ARMOperand &)*Operands.back()).getEndLoc(); + NearMissesOut.emplace_back( + NearMissMessage{ EndLoc, StringRef("too few operands for instruction") }); + break; + } + case NearMissInfo::NoNearMiss: + // This should never leave the matcher. + llvm_unreachable("not a near-miss"); + break; + } + } +} + +void ARMAsmParser::ReportNearMisses(SmallVectorImpl &NearMisses, + SMLoc IDLoc, OperandVector &Operands) { + SmallVector Messages; + FilterNearMisses(NearMisses, Messages, IDLoc, Operands); + + if (Messages.size() == 0) { + // No near-misses were found, so the best we can do is "invalid + // instruction". + Error(IDLoc, "invalid instruction"); + } else if (Messages.size() == 1) { + // One near miss was found, report it as the sole error. + Error(Messages[0].Loc, Messages[0].Message); + } else { + // More than one near miss, so report a generic "invalid instruction" + // error, followed by notes for each of the near-misses. + Error(IDLoc, "invalid instruction, multiple near-miss encodings found"); + for (auto &M : Messages) { + SmallString<128> NoteMessage("for one encoding: "); + NoteMessage.append(M.Message); + Note(M.Loc, NoteMessage); + } + } +} + // FIXME: This structure should be moved inside ARMTargetParser // when we start to table-generate them, and we can use the ARM // flags below, that were generated by table-gen.