[PATCH] D148483: RISC-V Zvk (vector crypto) specification update to 0.5.1 (Zvbb/Zvbc/Zvkt/Zvkng/Zvksg)

Wed Apr 19 09:24:54 PDT 2023

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

In D148483#4278220 <https://reviews.llvm.org/D148483#4278220>, @ego wrote:

> Many thanks for catching so many real issues. As usual something weird happened since the patch was perfect before upload :-).

That happens to me a lot too ;) All pretty minor stuff anyway.

All my previous comments were addressed and I stepped through the Zvbb and Zvbc definitions and the instruction+encoding definitions and couldn't spot any issues.

So LGTM - thanks!

You'll need to rebase before landing as some minor conflicting changes landed. Just a heads up that I tweaked what we do for external links in the experimental extension section https://reviews.llvm.org/rG0386546b088a (basically - using `__` rather than `_` for the links.

I don't really have a strong view about alphabetical vs other orderings, as long as it's consistent. For extension names, they seem to be listed alphabetically in the spec so that likely makes sense. There's not really a clear "right" ordering for instruction definitions - I used the order in the encoding table in the RISC-V spec for the base scalar instructions, but the quickref table in the vector crypto spec is nowhere near as easily readable so less useful for cross-checking. I'm neutral on that.

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

https://reviews.llvm.org/D148483