Skip to content

Conversation

@fdxmw
Copy link
Member

@fdxmw fdxmw commented Jul 23, 2025

  • Any Gate with a user-specified name is never inlined.
  • Unnamed constant Gates are always inlined.
  • Unnamed non-constant Gates are inlined if:
    • The Gate has one user, AND
    • The Gate is not a MemBlock read, AND
    • The Gate is not an arg to a bit-select Gate.

This significantly reduces the amount of generated Verilog code.

Also:

  • Update output_verilog_testbench to use GateGraph.
  • Define GateGraph.__iter__ to make it easier to iterate over all Gates.
  • Delete some disabled tests in test_aes.py.

- Any Gate with a user-specified name is never inlined.
- Unnamed constant Gates are always inlined.
- Unnamed non-constant Gates are inlined if:
  - The Gate has one user, AND
  - The Gate is not a MemBlock read, AND
  - The Gate is not an arg to a bit-select Gate.

This significantly reduces the amount of generated Verilog code.

Also:

- Update `output_verilog_testbench` to use `GateGraph`.
- Define `GateGraph.__iter__` to make it easier to iterate over all Gates.
- Delete some disabled tests in `test_aes.py`.
@codecov
Copy link

codecov bot commented Jul 23, 2025

Codecov Report

❌ Patch coverage is 94.08284% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.4%. Comparing base (3deeedd) to head (a3bc638).
⚠️ Report is 6 commits behind head on development.

Files with missing lines Patch % Lines
pyrtl/importexport.py 94.7% 16 Missing ⚠️
pyrtl/core.py 84.0% 4 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           development    #471     +/-   ##
=============================================
+ Coverage         91.3%   91.4%   +0.1%     
=============================================
  Files               25      25             
  Lines             6967    7048     +81     
=============================================
+ Hits              6359    6436     +77     
- Misses             608     612      +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fdxmw
Copy link
Member Author

fdxmw commented Jul 23, 2025

I'm still running more tests on this change. It's potentially controversial, so I'm publishing it in its current state so anyone interested can discuss.

…duplicated

code:

Update `_NameSanitizer` to try cleaning up an invalid name before giving up and
auto-generating a new name. The cleaning process just replaces non-word
characters `[A-Za-z0-9_]` with underscores, and also tries adding a prefix
underscore. This makes the generated Verilog output easier to read, especially
when using `wire_struct` or `wire_matrix`, which both add non-word characters
to wire names.

Remove `_NameSanitizer`'s `map_valid_vals` and `allow_duplicates` options,
which don't seem to be used.

Bugs fixed:

- `output_verilog_testbench` should not re-initialize RomBlocks. This appears
  to be an existing bug.
- Consts inputs to bit-slices must be declared.

Add tests for these bugs.

Also move the Verilog export code to a new `class _VerilogOutputter`. This lets
`output_to_verilog` and `output_verilog_testbench` share more code.

Also remove `OutputToVerilog`. It is deprecated, and the forwarding function
was broken, so it's pretty clear nobody uses this.
fdxmw added 2 commits July 24, 2025 13:54
Simplify `SimulationTracer`'s handling of `register_value_map` and
`memory_value_map`. These maps are now just passed straight through from
`Simulation`'s constructor. This fixes a bug where `FastSimulation` handled
them inconsistently. Added tests.

Fix a bug where `Simulation` and `FastSimulation` mutated the provided
`memory_value_map`.

Specify bitwidths for Verilog initial register and memory values. They were
previously unsized constants, which are implicitly 32-bit signed, which could
cause surprises.
…ore generating an entirely new name. Fix a bug where a generated name could collide with an identifier name. Add tests.
@fdxmw
Copy link
Member Author

fdxmw commented Jul 24, 2025

I've successfully tested this change by exporting pyrtlnet to Verilog with this change, and simulating the generated Verilog with Verilator.
I'm planning to merge this tomorrow, so let me know soon if you have comments or questions.

fdxmw added 3 commits July 24, 2025 16:36
… instead

of `mem_{memid}` names. Also add more comments to the generated Verilog:

- Indicate if a memory is a MemBlock or a RomBlock
- Show the un-sanitized name when it differs from the sanitized name
…e end of previous line, remove an unnecessary begin/end block, put "end else begin" on the same line.
…e and removes an unnecessary blank line from the generated output.
@fdxmw fdxmw merged commit 98d2b5c into UCSBarchlab:development Jul 25, 2025
5 checks passed
@fdxmw fdxmw deleted the verilog branch July 25, 2025 21:30
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.

1 participant