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

Superscalar metrics #61

Merged
merged 12 commits into from
Mar 4, 2025
Merged

Superscalar metrics #61

merged 12 commits into from
Mar 4, 2025

Conversation

tilk
Copy link
Member

@tilk tilk commented Feb 26, 2025

This PR modifies metrics to enable superscalar use - changing a metric multiple times in a single clock cycle. This is needed to be able to create superscalar circuits which utilize metrics.

The PR introduces a small syntactic change to method calls: the enable signal is renamed to enable_call and can be used as a keyword argument. This means that enable_call is now a reserved argument name. This was done to be able to remove method wrapper functions from metrics modules, which utilized an optional cond argument - which is basically enable. I'm not extra happy with the solution, but it gets the job done.

TODO:

  • Update metrics tests to test superscalar use.

@tilk tilk added the enhancement New feature or request label Feb 26, 2025
@tilk tilk marked this pull request as draft February 26, 2025 13:58
@tilk tilk marked this pull request as ready for review February 28, 2025 11:53
@tilk tilk requested review from lekcyjna123 and xThaid February 28, 2025 11:53

Should be called in the body of either a transaction or a method.
incr: Methods
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that something went wrong here. Why this docstring is in such weird place?

Copy link
Member Author

Choose a reason for hiding this comment

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

Docstring for attributes is written after the attribute, as PEP 257 describes. An alternative would be to remove the method docstrings entirely and add info at the beginning of the class, as we do in other places in Transactron and Coreblocks. Still, this style seems to have some benefits, including being shown contextually in IDEs.

@tilk tilk merged commit 89d889c into kuznia-rdzeni:master Mar 4, 2025
3 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants