[AsmMatcher] Extend PredicateMethod with optional DiagnosticPredicate

[sdesmalen]

Summary

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

[rengolin]

Can't remember if it was @olista01 or @SjoerdMeijer who was doing the partial match in the assembler, but the idea sounds good to me. I'll let them (or someone else closer to the changes) to approve.

[fhahn]

IMO this leads to a nice improvement for the assembler diagnostics as shown D45880. I think it would be great if we could also make the recent improvements to usability more visible, so they can get wider usage across backends.

include/llvm/MC/MCParser/MCTargetAsmParser.h

136 ↗

(On Diff #143305)

enum class ?

[SjoerdMeijer]

Looks good to me too, just some comments about typos/nits inlined from my side. I will let Oliver approve as he did most of the work in this area.

include/llvm/MC/MCParser/MCTargetAsmParser.h

142 ↗

(On Diff #143305)

nit: try -> try to

143 ↗

(On Diff #143305)

nit: that operand -> that the operand

161 ↗

(On Diff #143305)

typo: diagnostice

[sdesmalen]

• Changed DiagnosticPredicateTy from 'enum' to 'enum class'.
• Made the constructor for 'DiagnosticPredicate(bool) explicit'.
• Fixed typos.

[SjoerdMeijer]

include/llvm/MC/MCParser/MCTargetAsmParser.h

158 ↗

(On Diff #143700)

Sorry, one more nit while we wait for Oliver's lgtm: is "decoration suffix" the right terminology here? Should this e.g. be "access suffix", or something else?

[sdesmalen]

include/llvm/MC/MCParser/MCTargetAsmParser.h

158 ↗

(On Diff #143700)

Agreed, 'decoration suffix' may not be the right terminology, although I'm not sure if 'access suffix' is much clearer. How about just using 'shift/extend' ?

[SjoerdMeijer]

include/llvm/MC/MCParser/MCTargetAsmParser.h

158 ↗

(On Diff #143700)

Agreed, that looks better.

[olista01]

LGTM

llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h

 };
 
 enum OperandMatchResultTy {
   MatchOperand_Success,  // operand matched successfully
   MatchOperand_NoMatch,  // operand did not match
   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
 // each of these "near-miss" encodings. This struct contains information about
 // one such encoding, and why it did not match the parsed instruction.
 class NearMissInfo {
 public:
   enum NearMissKind {

llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp

static void emitValidateOperandClass(AsmMatcherInfo &Info,
   // actually matching against one of them.
   OS << "  switch (Kind) {\n"
         "  default: break;\n";
   for (const auto &CI : Info.Classes) {
     if (!CI.isUserClass())
       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";
 
   // Check for register operands, including sub-classes.
   OS << "  if (Operand.isReg()) {\n";
   OS << "    MatchClassKind OpKind;\n";
   OS << "    switch (Operand.getReg()) {\n";
   OS << "    default: OpKind = InvalidMatchClass; break;\n";