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

Refactor kratos backend #507

Merged
merged 2 commits into from
Dec 10, 2019
Merged

Refactor kratos backend #507

merged 2 commits into from
Dec 10, 2019

Conversation

Kuree
Copy link
Contributor

@Kuree Kuree commented Dec 9, 2019

What:

  • Passing in ast nodes directly into the code gen backend instead of dump it into a text file and load back. This also allows comments and other multi-line coding style as long as during the pre-processing the lineno is preserved (added in this PR)
  • Refactor the code to reuse both combinational and sequential logics.
  • Construct proper reset logic instead of string generation

Discussion:

  1. For now I assume everything is posedge reset, this may not be the case for ASIC design and it would be better to have an option.
  2. There is no clock gating/chip enable signal to the sequential block., It would be great if we can have the port so that the sequential logic can be properly clock gated.

Some hacks on debugging:

  • If you look at how I dump debug information for testing sequential logic, the generators are not collected automatically. I'm not sure if the code used for combinational works here.

@coveralls
Copy link

coveralls commented Dec 9, 2019

Pull Request Test Coverage Report for Build 2412

  • 24 of 27 (88.89%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 74.043%

Changes Missing Coverage Covered Lines Changed/Added Lines %
magma/syntax/verilog.py 24 27 88.89%
Files with Coverage Reduction New Missed Lines %
magma/ast_utils.py 1 72.15%
Totals Coverage Status
Change from base Build 2410: -0.02%
Covered Lines: 4741
Relevant Lines: 6403

💛 - Coveralls

Copy link
Collaborator

@leonardt leonardt left a comment

Choose a reason for hiding this comment

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

LGTM,
we could add options for posedge/negedge reset as well as clock enable signal either as arguments to the decorator or as class attributes in the definition (first pass we can have all the internal registers share an enable signal, second pass we can think about how we could expose an interface to provide multiple enable signals for different registers.

@leonardt leonardt merged commit 7d005da into phanrahan:master Dec 10, 2019
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