[AsmMatcher] Add support for conflicting features

This helps avoiding diagnostics for instructions that could never be
selected and is required for RISC-V CHERI support.
An example here are the CHERI mode-dependent instructions where we have
loads/stores that are identical other than the register class for the
base register and have predicates that can never both be set. To avoid
nonsensical error messages, we should only use the candidate
instructions with the currently available feature bits.

For RVY (CHERI), loads and stores are mode-dependent, using either a
YLEN register or a XLEN register as the base. Prior to the
standardization process CHERI assembly used c-prefixed register names
for capabilities, so we had the following syntax for RISC-V compatible
mode and CHERI pure-capability mode:

lw x4, 0(c3)  # capability mode: use new `CLW` tablegen instruction
lw x4, 0(x3)  # integer mode: use existing `LW` tablegen instruction

During the standardization this was changed to keep the same register
name in both modes, so now we have `lw x4, 0(x3)` in both modes but we
have to select between two instructions: one using the normal GPR
register class and one using the YGPR register class.
We now have a choice between two instructions `LW` and `LW_Y` that have
predicates that can never both be true, so we should avoid reporting
missing predicates or wrong operands for the "unreachable" instruction.

This change was taken from Morello LLVM with a few minor comment
clarifications and changes to naming of variables.

Co-authored-by: Silviu Baranga <[email protected]>

Author: 2 people

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

@@ -353,6 +353,14 @@ class LLVM_ABI MCTargetAsmParser : public MCAsmParserExtension {
 
   /// AvailableFeatures - The current set of available features.
   FeatureBitset AvailableFeatures;
+  /// A set of features that conflict with each other. This helps improve
+  /// diagnostics by ignoring instruction variants that are mutually exclusive.
+  /// An example here are the CHERI mode-dependent instructions where we have
+  /// loads/stores that are identical other than the register class for the base
+  /// register and have predicates that can never both be set. To avoid
+  /// nonsensical error messages, we should only use the candidate instructions
+  /// with the currently available feature bits.
+  FeatureBitset ConflictingFeatures;
 
   /// ParsingMSInlineAsm - Are we parsing ms-style inline assembly?
   bool ParsingMSInlineAsm = false;
@@ -384,6 +392,15 @@ class LLVM_ABI MCTargetAsmParser : public MCAsmParserExtension {
     AvailableFeatures = Value;
   }
 
+  const FeatureBitset &getConflictingFeatures() const {
+    return ConflictingFeatures;
+  }
+  void setConflictingFeatures(const FeatureBitset &Value) {
+    assert((Value.none() || Value.count() == 2) &&
+       "Only zero or two conflicting features supported");
+    ConflictingFeatures = Value;
+  }
+
   bool isParsingMSInlineAsm () { return ParsingMSInlineAsm; }
   void setParsingMSInlineAsm (bool Value) { ParsingMSInlineAsm = Value; }
 

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

@@ -345,6 +345,15 @@ class RISCVAsmParser : public MCTargetAsmParser {
     Parser.addAliasForDirective(".word", ".4byte");
     Parser.addAliasForDirective(".dword", ".8byte");
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+    // Don't record errors based on the CapMode feature. XOR the two
+    // bit patterns to get something that has just CapMode and NotCapMode.
+    // Skipping mnemonics that are not available in the current assembler mode
+    // significantly improves diagnostics and allows reusing the same register
+    // names for capmode vs non-capmode.
+    FeatureBitset CapModeSet =
+        ComputeAvailableFeatures({RISCV::FeatureCapMode});
+    FeatureBitset NoCapModeSet = ComputeAvailableFeatures({});
+    // setConflictingFeatures(CapModeSet ^ NoCapModeSet);
 
     auto ABIName = StringRef(Options.ABIName);
     if (ABIName.ends_with("f") && !getSTI().hasFeature(RISCV::FeatureStdExtF)) {

llvm/utils/TableGen/AsmMatcherEmitter.cpp

@@ -3610,6 +3610,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "  // Get the current feature set.\n";
   OS << "  const FeatureBitset &AvailableFeatures = "
         "getAvailableFeatures();\n\n";
+  OS << "  // Get the set of features to not report as missing features\n";
+  OS << "  const FeatureBitset &ConflictingFeatures = getConflictingFeatures();\n\n";
 
   OS << "  // Get the instruction mnemonic, which is the first token.\n";
   if (HasMnemonicFirst) {
@@ -3676,7 +3678,10 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "  // Return a more specific error code if no mnemonics match.\n";
   OS << "  if (MnemonicRange.first == MnemonicRange.second)\n";
   OS << "    return Match_MnemonicFail;\n\n";
-
+  if (!ReportMultipleNearMisses)
+    OS << "  auto FirstValidMnemonic = MnemonicRange.first;\n";
+  OS << "  bool FoundValidMnemonic = false;\n";
+  OS << "  bool HasNonConflictingMnemonic = false;\n";
   OS << "  for (const MatchEntry *it = MnemonicRange.first, "
      << "*ie = MnemonicRange.second;\n";
   OS << "       it != ie; ++it) {\n";
@@ -3688,7 +3693,20 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
         "opcode \"\n";
   OS << "                                          << MII.getName(it->Opcode) "
         "<< \"\\n\");\n";
-
+  // If we have a mismatch because we're missing one of the features that are
+  // conflicting for diagnostics purposes, skip this mnemonic.
+  OS << "    if (!HasRequiredFeatures &&\n";
+  OS << "        (RequiredFeatures & ConflictingFeatures & ~AvailableFeatures).any()) {\n";
+  OS << "      DEBUG_WITH_TYPE(\"asm-matcher\", dbgs() << \"  Skipping mnemonic \"\n";
+  OS << "                       << MII.getName(it->Opcode) << \" due to conflicting features\\n\");\n";
+  OS << "      continue;\n";
+  OS << "    }\n";
+  OS << "    HasNonConflictingMnemonic = true;\n";
+  OS << "    if (!FoundValidMnemonic) {\n";
+  if (!ReportMultipleNearMisses)
+    OS << "      FirstValidMnemonic = it;\n";
+  OS << "      FoundValidMnemonic = true;\n";
+  OS << "    }\n";
   if (ReportMultipleNearMisses) {
     OS << "    // Some state to record ways in which this instruction did not "
           "match.\n";
@@ -3854,7 +3872,7 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
     OS << "      // If we already had a match that only failed due to a\n";
     OS << "      // target predicate, that diagnostic is preferred.\n";
     OS << "      if (!HadMatchOtherThanPredicate &&\n";
-    OS << "          (it == MnemonicRange.first || ErrorInfo <= ActualIdx)) "
+    OS << "          (it == FirstValidMnemonic || ErrorInfo <= ActualIdx)) "
           "{\n";
     OS << "        if (HasRequiredFeatures && (ErrorInfo != ActualIdx || Diag "
           "!= Match_InvalidOperand))\n";
@@ -3892,6 +3910,11 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "                        if (NewMissingFeatures[I])\n";
   OS << "                          dbgs() << ' ' << I;\n";
   OS << "                      dbgs() << \"\\n\");\n";
+  OS << "      if ((NewMissingFeatures & ConflictingFeatures).any()) {\n";
+  if (!ReportMultipleNearMisses)
+    OS << "        HadMatchOtherThanFeatures = false;\n";
+  OS << "        continue;\n";
+  OS << "      }\n";
   if (ReportMultipleNearMisses) {
     OS << "      FeaturesNearMiss = "
           "NearMissInfo::getMissedFeature(NewMissingFeatures);\n";
@@ -4102,6 +4125,11 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   OS << "    return Match_Success;\n";
   OS << "  }\n\n";
 
+  OS << "  if (!HasNonConflictingMnemonic) {\n";
+  OS << "    DEBUG_WITH_TYPE(\"asm-matcher\", dbgs() << \"Could not find non-conflicting mnemonic\\n\");\n";
+  OS << "    return Match_MnemonicFail;\n";
+  OS << "  }\n";
+
   if (ReportMultipleNearMisses) {
     OS << "  // No instruction variants matched exactly.\n";
     OS << "  return Match_NearMisses;\n";