Skip to content

Commit

Permalink
[AsmMatcher] Extend PredicateMethod with optional DiagnosticPredicate
Browse files Browse the repository at this point in the history
An optional, light-weight and backward-compatible mechanism to allow
specifying that a diagnostic _only_ applies to a partial mismatch (NearMiss),
rather than a full mismatch.

Patch [1/2] in a series to improve assembler diagnostics for SVE.
-  Patch [1/2]: https://reviews.llvm.org/D45879
-  Patch [2/2]: https://reviews.llvm.org/D45880

Reviewers: olista01, stoklund, craig.topper, mcrosier, rengolin, echristo, fhahn, SjoerdMeijer, evandro, javed.absar

Reviewed By: olista01

Differential Revision: https://reviews.llvm.org/D45879

llvm-svn: 330930
  • Loading branch information
sdesmalen-arm committed Apr 26, 2018
1 parent 466410b commit a2fb1d1
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
47 changes: 47 additions & 0 deletions llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
Expand Up @@ -133,6 +133,53 @@ enum OperandMatchResultTy {
MatchOperand_ParseFail // operand matched but had errors
};

enum class DiagnosticPredicateTy {
Match,
NearMatch,
NoMatch,
};

// When an operand is parsed, the assembler will try to iterate through a set of
// possible operand classes that the operand might match and call the
// corresponding PredicateMethod to determine that.
//
// If there are two AsmOperands that would give a specific diagnostic if there
// is no match, there is currently no mechanism to distinguish which operand is
// a closer match. The DiagnosticPredicate distinguishes between 'completely
// no match' and 'near match', so the assembler can decide whether to give a
// specific diagnostic, or use 'InvalidOperand' and continue to find a
// 'better matching' diagnostic.
//
// For example:
// opcode opnd0, onpd1, opnd2
//
// where:
// opnd2 could be an 'immediate of range [-8, 7]'
// opnd2 could be a 'register + shift/extend'.
//
// If opnd2 is a valid register, but with a wrong shift/extend suffix, it makes
// little sense to give a diagnostic that the operand should be an immediate
// in range [-8, 7].
//
// This is a light-weight alternative to the 'NearMissInfo' approach
// below which collects *all* possible diagnostics. This alternative
// is optional and fully backward compatible with existing
// PredicateMethods that return a 'bool' (match or no match).
struct DiagnosticPredicate {
DiagnosticPredicateTy Type;

explicit DiagnosticPredicate(bool Match)
: Type(Match ? DiagnosticPredicateTy::Match
: DiagnosticPredicateTy::NearMatch) {}
DiagnosticPredicate(DiagnosticPredicateTy T) : Type(T) {}
DiagnosticPredicate(const DiagnosticPredicate &) = default;

operator bool() const { return Type == DiagnosticPredicateTy::Match; }
bool isMatch() const { return Type == DiagnosticPredicateTy::Match; }
bool isNearMatch() const { return Type == DiagnosticPredicateTy::NearMatch; }
bool isNoMatch() const { return Type == DiagnosticPredicateTy::NoMatch; }
};

// When matching of an assembly instruction fails, there may be multiple
// encodings that are close to being a match. It's often ambiguous which one
// the programmer intended to use, so we want to report an error which mentions
Expand Down
14 changes: 10 additions & 4 deletions llvm/utils/TableGen/AsmMatcherEmitter.cpp
Expand Up @@ -2451,14 +2451,20 @@ static void emitValidateOperandClass(AsmMatcherInfo &Info,
continue;

OS << " // '" << CI.ClassName << "' class\n";
OS << " case " << CI.Name << ":\n";
OS << " if (Operand." << CI.PredicateMethod << "())\n";
OS << " case " << CI.Name << ": {\n";
OS << " DiagnosticPredicate DP(Operand." << CI.PredicateMethod
<< "());\n";
OS << " if (DP.isMatch())\n";
OS << " return MCTargetAsmParser::Match_Success;\n";
if (!CI.DiagnosticType.empty())
OS << " return " << Info.Target.getName() << "AsmParser::Match_"
if (!CI.DiagnosticType.empty()) {
OS << " if (DP.isNearMatch())\n";
OS << " return " << Info.Target.getName() << "AsmParser::Match_"
<< CI.DiagnosticType << ";\n";
OS << " break;\n";
}
else
OS << " break;\n";
OS << " }\n";
}
OS << " } // end switch (Kind)\n\n";

Expand Down

0 comments on commit a2fb1d1

Please sign in to comment.