[llvm-branch-commits] [llvm] [SPARC][IAS] Reject unknown/unavailable mnemonics early in ParseInstruction (PR #96021)

[via llvm-branch-commits]

koachan wrote:

Merging this for now, but to clarify a little:

> The downside of this approach is that you lose the ability to report the missing features (unless you implement it yourself, which would be a code duplication), that would allow to produce better diagnostics like "instruction requires: LeonCASA".

It seems that there are no autogenerated and/or common functions that can report what is exactly the missing feature. A quick look at the other backends also shows that the ones that do support reporting that implements it by themselves (unless I'm missing something here?).

> Also, sometimes a mnemonic is valid in both modes, but certain operand combinations are only allowed in V9, e.g. swapa with r+i addressing mode. In this case you still rely on MatchOperandParserImpl & MatchInstructionImpl, which can produce different / broken diagnostics.

Yeah, this is what is bugging me too.
As far as I understand it, MatchInstructionImpl is only called at the very end of the process, after all the operands have been parsed. On the other hand, since some operand combinations are only allowed in V9, operand parsing can fail early.
So this is what will happen when parsing, say, a prefetcha with r+i addressing mode when targeting V8 (e.g. `prefetcha  [%i1+1] %asi, 2`, invalid instruction with invalid operand):
1. MatchOperandParserImpl is called. This will return a NoMatch[1], which will cause IAS to continue to the generic matcher in parseOperand...
2. Which then fails because the `%asi` token in the r+i addressing isn't recognized in V8 mode.

In the end the error that will go out is one from the operand check, instead of the mnemonic check - since IAS haven't gotten far enough to reach MatchInstructionImpl yet - unless there is a way to defer the error from the operand check until just after MatchInstructionImpl is done (is it possible?).
Rejecting the mnemonic early should be okay since by the time IAS reaches MatchOperandParserImpl/MatchInstructionImpl we are already sure that we have a valid mnemonic, and any other errors that is going to be raised won't be related to mnemonics.

On cases like `swapa` the mnemonic check will succeed either way, so it is up to the operand checks to decide whether the full instruction makes sense for a given ISA or not.

[1] MatchOperandParserImpl return value does not differentiate between unavailable mnemonic, unknown mnemonic, or not being able to find a custom parser, so in all three cases parseOperand will just fall through to the generic matcher, so at least in this particular case there is no difference between ParseForAllFeatures being enabled or not.

https://github.com/llvm/llvm-project/pull/96021