[llvm] [RISCV] Merge Xqci Decoder Tables (PR #128140)

[Craig Topper via llvm-commits]

topperc wrote:

> > If I'm reading this right, that would be a non-conforming extension. The conflict wouldn't be with other qc extensions, but with the standard ones right? If so, having a single qc table should still be fine, you just need to check it before the standard table. Though, having the qc table split into conforming and non-conforming also doesn't seem unreasonable, which is what I think you meant. So either option works. :)
> 
> Sorry, I was afk on Friday. Xqci and its sub-extensions are conforming, as far as I can tell.
> 
> The problem is somewhat harder to describe, but @svs-quic found me an example which illustrates things better than I've been able to before.
> 
> We have `def uimm5gt3` for `qc.shladd`'s fourth operand (the shift amount). This is a 5-bit field (`Inst{29-25}`), but only the values 4 through 31 are valid for this instruction. We need to not decode the instructions where that 5-bit field has value `0`-`3` as `qc.shladd`, because it isn't `qc.shladd`.
> 
> This is even more of an issue, in another pair of instructions from the Xqci spec - `qc.c.extu` has a 5-bit immediate field (`Insn{6-2}`), which represents the `width` operand, but only when `width` is more than 5. `qc.c.dir` reuses the encodings where `Insn{6-2}` is all zeroes.
> 
> You get similar issues for e.g. GPRNoX0 fields where the encoding of all zeroes for that 5-bit field is used for something else, but I don't have a motivating example to hand.
> 
> LLVM already contains the right logic for dealing with this - we just need to start using it - it's the `hasCompleteDecoder` logic. We also need to make improvements to decoders which only accept a specific range, to ensure they fail when out of that range. On tip of tree you can decode bytes into invalid instructions like `qc.shladd x1, x2, x3, 3`.
> 
> I think if we solve all of this for Xqci (which does seem to have a lot of these "N-bit fields but not these specific encoding values" fields), this should make our decoders more accurate, and also will make it easier for other people to merge tables.
> 
> One of the difficulties in the short term is not all of Xqci is upstream, so it's hard to know where/when this will bite us. That's the big reason I deprioritised this patch wrt others we're working on.

I think for the GPRNoX0 case, if the field is constant a 0 in another instruction decoder should handle that correctly without `hasCompleteDecoder`.

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