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.

Sign up for GitHub

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

Jump to bottom

[ECC] add_sub_mod_alter ready not aligned with result #194

Closed
ludwigatlubis opened this issue Aug 25, 2023 · 1 comment · Fixed by #594
Closed

[ECC] add_sub_mod_alter ready not aligned with result #194

ludwigatlubis opened this issue Aug 25, 2023 · 1 comment · Fixed by #594
Assignees
@ludwigatlubis @mojtaba-bisheh
Labels
2.0 feature request Approved enhancement New feature or request

Comments

@ludwigatlubis
Copy link
Contributor

Hello,

We found an issue with the ready signal which does not align with the computed result. The expected behaviour of the ready signal is to be set when we have the correct result. In the current implementation the ready arrives 1-2 cycles, if and only if the enable signal is kept low.

@mojtaba-bisheh confirmed this issue, however, the design works fine with the current implementation, because it does not affect the computation itself. It will be updated in future releases.

We adde two sets of constraints for this module. One that includes the intended input behaviour and one that fixes the current issues appended with _bugfix. Please remove that constraint after the RTL design updates are done to check for correctness.

Best
Tobias
@ludwigatlubis ludwigatlubis added bug Something isn't working enhancement New feature or request labels Aug 25, 2023
@bharatpillilli bharatpillilli removed the bug Something isn't working label Aug 25, 2023
@mojtaba-bisheh
Copy link
Contributor

mojtaba-bisheh commented Aug 25, 2023 •
edited
Loading

We did not use the ready signal in the design because we hard coded the arithmetic scheduling. But we have fixed this issue in our internal repo and we will update it here soon. (https://github.com/chipsalliance/caliptra-rtl/pull/193/commits has this fix)
The next enhancement for the adder is to remove the constraint that requires constant inputs during the computation. This will make the architecture more reusable for other designs.

@mojtaba-bisheh mojtaba-bisheh mentioned this issue Sep 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ludwigatlubis ludwigatlubis

@mojtaba-bisheh mojtaba-bisheh

Labels
2.0 feature request Approved enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Crypto enhancement chipsalliance/caliptra-rtl
3 participants
@bharatpillilli @ludwigatlubis @mojtaba-bisheh