Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix kext warning #366

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Fix kext warning #366

merged 1 commit into from
Dec 6, 2023

Conversation

Alasdair
Copy link
Collaborator

@Alasdair Alasdair commented Dec 2, 2023

Use fixed-length vectors for AES tables. This fixes a warning about an incomplete match, and should be much more efficient than using linked lists. It also makes the Sail->SystemVerilog translation happy as a nice bonus.

Note that because we have default Order dec, and Sail vectors are indexed the same as bitvectors in decreasing order, the indexing in sbox_lookup becomes

table[255 - unsigned(x)]

rather than

table[unsigned(x)]

The alternative would be to reverse all the table literals.

Copy link

github-actions bot commented Dec 2, 2023

Test Results

712 tests  ±0   712 ✔️ ±0   0s ⏱️ ±0s
    6 suites ±0       0 💤 ±0 
    1 files   ±0       0 ±0 

Results for commit 9889dc7. ± Comparison against base commit 153f983.

♻️ This comment has been updated with latest results.

Use fixed-length vectors for AES tables

Note that because we have `default Order dec`, Sail vectors
are indexed the same as bitvectors, in decreasing order, hence why
the indexing in sbox_lookup becomes
```
table[255 - unsigned(x)]
```
rather than
```
table[unsigned(x)]
```
The alternative would be to flip all the literals
@Timmmm
Copy link
Collaborator

Timmmm commented Dec 6, 2023

Could you use vector(256, inc, bits(8)) instead? Not sure that would be better tbh even if it does work.

Copy link
Collaborator

@billmcspadden-riscv billmcspadden-riscv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement.

nit - it would be nice to see the tables formed as 16x16 or 32x8.

@billmcspadden-riscv billmcspadden-riscv merged commit 393e7ed into riscv:master Dec 6, 2023
2 checks passed
@Alasdair
Copy link
Collaborator Author

Alasdair commented Dec 6, 2023

Could you use vector(256, inc, bits(8)) instead? Not sure that would be better tbh even if it does work.

The Order parameter on vectors no longer does anything (all vectors and bitvectors have the default order). It never worked well and was a big source of internal complexity and headaches.

@Alasdair Alasdair deleted the kext_fix branch December 15, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants