[PATCH] D88214: [TableGen] Added a function to identify unsupported opcodes

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

> Please note that MatchInstructionImpl may also return Match_MissingFeature if an operand does not match selected feature set. So we cannot use MatchInstructionImpl to find out if an instruction mnemonic is supported or not - Match_MissingFeature may be returned for other reasons

Did you try out my suggestion though? Because if I make this local change in AMDGPUAsmParser:

  -  OperandMatchResultTy ResTy = MatchOperandParserImpl(Operands, Mnemonic);
  +  OperandMatchResultTy ResTy = MatchOperandParserImpl(Operands, Mnemonic, /*ParseForAllFeatures=*/true);

then most of the cases you fixed in D88211 <https://reviews.llvm.org/D88211> are fixed for free and I don't see the case happening that you mention here. If the above change doesn't fix that, can you please point me to the test for which that happens?
(If the above change does fix it, we should probably consider making 'true' the default for MatchOperandParserImpl)

The only case that would still give a diagnostic for one of the operands is if both the instruction+operands are not valid for _any_ feature-set. The mnemonic checker could give a slightly improved diagnostic here, but I would think this scenario is quite limited. If you prefer the mnemonic checker to solve that specific case, that's fine, I mostly wanted to make you aware that a large portion of this problem can be solved with a one-line change.



================
Comment at: llvm/utils/TableGen/AsmMatcherEmitter.cpp:3977
+
+  emitMnemonicChecker(OS, Target, VariantCount,
+                      HasMnemonicFirst, HasMnemonicAliases);
----------------
Should this be called by `MatchInstructionImpl` so that this feature comes for free for all AsmParsers?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88214/new/

https://reviews.llvm.org/D88214