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

[WIP] Add Display statement. #871

Closed
wants to merge 2 commits into from
Closed

[WIP] Add Display statement. #871

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 22, 2023

Intent: fixes #432.

  • Rebase of dc6a805, which adds the Display statement with support in simulation.
  • Minimally working RTLIL backend support.
    • Confirmed this works in CXXRTL.

TODO:

  • Synchronous Display statements
  • Lots of cleaning up and poking-around testing.
  • Unit tests.
  • Documentation.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #871 (90b6583) into main (c7da6c1) will decrease coverage by 0.74%.
The diff coverage is 44.02%.

@@            Coverage Diff             @@
##             main     #871      +/-   ##
==========================================
- Coverage   83.73%   82.99%   -0.74%     
==========================================
  Files          54       54              
  Lines        7681     7788     +107     
  Branches     1868     1896      +28     
==========================================
+ Hits         6432     6464      +32     
- Misses       1048     1123      +75     
  Partials      201      201              
Files Coverage Δ
amaranth/__init__.py 85.71% <ø> (ø)
amaranth/hdl/__init__.py 100.00% <100.00%> (ø)
amaranth/hdl/dsl.py 99.73% <100.00%> (ø)
amaranth/sim/_pyrtl.py 96.07% <88.23%> (-0.53%) ⬇️
amaranth/back/rtlil.py 76.97% <25.00%> (-0.34%) ⬇️
amaranth/hdl/xfrm.py 95.37% <79.16%> (-0.85%) ⬇️
amaranth/hdl/ast.py 84.74% <25.28%> (-5.25%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@whitequark
Copy link
Member

whitequark commented Feb 26, 2024

Thank you for this contribution. Project policy requires that substantial changes (such as this one) go through the RFC process first. As this feature (along with its sister feature #427, which will use the same kind of format strings) is a high-priority one on the roadmap, I will go ahead and write the RFC for it @wanda-phi ended up writing the RFC. As always, your feedback in the RFC process will be appreciated.

@whitequark
Copy link
Member

Thank you for submitting this pull request. We have iterated on the proposed design; as such, support for this feature has been accepted via RFC 50 and implementation is tracked in issue #1186. Since it significantly differs from this PR I'll go ahead and close it, noting that we may still choose to reuse some of the implementation.

@whitequark whitequark closed this Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Print in simulation
2 participants