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

Eliminate Rcon array to save space #43

Open
rillig opened this issue Dec 2, 2016 · 5 comments
Open

Eliminate Rcon array to save space #43

rillig opened this issue Dec 2, 2016 · 5 comments

Comments

@rillig
Copy link

rillig commented Dec 2, 2016

The Rcon array currently takes 255 bytes of ROM, but only its first 11 elements are accessed, since there are only 10 rounds. Therefore the remaining 244 elements should be removed. The first element can also be removed when the array index is adjusted by -1.

The whole array can be replaced by starting a local variable uint8_t rcon = 0x01 and applying xtime after each round, which is a little slower but eliminates the need for fixed constants.

@kokke
Copy link
Owner

kokke commented Dec 9, 2016

Duly noted, thank you :) I think another issue-opener also comments on this exact issue.

@EliotWealth
Copy link

This rom elements can be used to store xtime computed values instead to increase performance which is not the best.

@kokke
Copy link
Owner

kokke commented Dec 12, 2016

@EliotWealth the performance is definitely not optimal in this module.
The main goal of the project is a small code size.
There is a lot of potential for performance-optimizations, at the cost of a slightly larger binary size :)

@kokke
Copy link
Owner

kokke commented Jul 13, 2017

This issue has been addressed after removing some of the elements in the Rcon array, as suggested in PR #12.

I'm keeping the issue open, because I like the description of how to avoid the array.
I want to move the architectural comments / suggestions into a wiki.
A lot of people have mentioned small changes that affect architecture or performance in a different way.
There are usually some tradeoffs to be made, which is why I won't always accept the solution, but will place a comment in the source, suggesting the changes.
I would like to put all that info in a wiki some day.

@RKTRIP
Copy link

RKTRIP commented Jan 18, 2018

To make better performance at Nano seconds you can replace For loop at multiple place by memset :)

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

No branches or pull requests

4 participants