[llvm] [RISCV] Merge Xqci Decoder Tables (PR #128140)
Sam Elliott via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 22 23:01:35 PST 2025
lenary 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.
https://github.com/llvm/llvm-project/pull/128140
More information about the llvm-commits
mailing list