search

riscv / riscv-bitmanip

Working draft of the proposed RISC-V Bitmanipulation extension
https://jira.riscv.org/browse/RVG-122
Creative Commons Attribution 4.0 International
204 stars 65 forks source link

Spec contains Zbkb instructions which do not match the scalar crypto spec #175

Open Wren6991 opened 2 years ago

Wren6991 commented 2 years ago

Hi,

I just had a go at implementing the Zbkb instructions as listed in the latest revision of this spec, cf7c568. I quickly discovered that the bit pattern used for e.g. zip in this spec does not match that used by the latest binutils. Here is a screencap of the listing for zip in the latest revision of this spec:

image

I then found that the latest scalar crypto spec also has a listing for Zbkb instructions, and in this case it seems to match the opcodes used by the latest binutils. Using zip as an example once again:

image

These differ at least in bit 24.

It's undoubtedly my mistake to work from the latest draft of this spec rather than the actual tagged release of the crypto spec. However, looking at this spec, there is nothing to indicate that it is not the most up-to-date version of the Zbkb instructions, nor that another version exists in a different spec. I came looking here because it seemed like the most sensible place for a Zb-prefixed extension.

Would it be reasonable to either delete the incorrect instruction listings from this repo (with a pointer to the crypto spec), or update them to match the ones in the scalar crypto spec? I am happy to raise a PR for either.

ptomsich commented 2 years ago

/assign @ben-marshall

@ben-marshall Could you help to reconcile this?

Wren6991 commented 2 years ago

The other differences I noticed are:

I have only looked at Zbkb. Is this copy just out of date?

jim-wilson commented 2 years ago

This repo is not actively maintained anymore, and should not be used as a reference anymore. If the crypto spec and binutils agree, then there is no problem to fix. The only docs that can be used as a reference are the ones here https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions

The instructions you mentioned are not part of the ratified bitmanip subset, so they are not documented here. They are only documented in the ratified crypto spec.

There may be a bitmanip v2 spec at some point, but until then anything here is useless and should be ignored.

Wren6991 commented 2 years ago

This repo is not actively maintained anymore, and should not be used as a reference anymore

Thank you for clarifying. The link is also helpful.

The instructions you mentioned are not part of the ratified bitmanip subset, so they are not documented here.

However, cloning and building produces something that looks like a specification for Zbkb. Clearly I have made a mistake, but others might easily make the same mistake, and that may take up more of your time.

Would it be possible to add a README note saying that Zbk* are only described in the crypto spec, and/or clarifying that any content outside of the tagged releases should be ignored?