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

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

Hello @sdesmalen. Thanks for your review.

> If the operands are all valid but the instruction is not in the feature set, it will return `Match_MissingFeature`.

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.

The limitation you pointed out was actually an intention. I wanted to completely ignore operands when searching for instruction mnemonic.
Why? Because if a mnemonic is not supported for a given set of features, it does not matter which operands were specified and error message should be "instruction not supported for this feature set". I believe that reporting an invalid operand would be misleading in this case.

Below are a few examples to illustrate my point. The examples show typical errors and how they can be handled.

  invalid_opcode           operand, ...

If opcode mnemonic is invalid, `MatchInstructionImpl` returns `Match_MnemonicFail`. This allows reliable identification of invalid instruction mnemonics. This case is usualluy reported as "invalid instruction, did you mean ...?"

  supported_opcode         invalid_operand, ...

`MatchInstructionImpl` may return any match error status, including `Match_MissingFeature`. In the latter case it is impossible to know what the reason is - unsupported opcode or unsupported operand.

  unsupported_opcode       valid_operand, ...

This case should result in "unsupported opcode" message. However even if `MatchInstructionImpl` returns `Match_MissingFeature` it is not possible to know what the reason is - unsupported opcode or unsupported operand.

  unsupported_opcode       invalid_operand, ...

Should result in "unsupported opcode" message. However `MatchInstructionImpl` may return any match error status, not only `Match_MissingFeature`. Consequently assembler may report something like "invalid operand".

The proposed function is needed to improve error messages in cases 2, 3 and 4. The function should be used to report unsupported opcodes before handling error codes returned by `MatchInstructionImpl`.

I could not figure out how your change in `MatchOperandParserImpl` could help with issues described above.


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

https://reviews.llvm.org/D88214