ram: remove explicit power_pin/ground_pin args from 2r1w test tcl files#10832
Conversation
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds two new compulsory integration tests, make_7x7_2r1w_nangate45 and make_8x8_2r1w_sky130, to the RAM test suite. This includes updating the Bazel BUILD file, golden outputs, and TCL test scripts, where explicit power and ground pin arguments are removed from the generate_ram command. The reviewer pointed out a copy-paste inconsistency in make_8x8_2r1w_sky130.tcl where the behavioral file name is incorrectly defined using a different test configuration (make_8x8_1rw1r_behavioral.v), and recommended updating it to prevent maintenance confusion.
| @@ -16,8 +16,6 @@ generate_ram \ | |||
| -r_ports 2 \ | |||
| -w_ports 1 \ | |||
| -storage_cell sky130_fd_sc_hd__dfxtp_1 \ | |||
There was a problem hiding this comment.
In make_8x8_2r1w_sky130.tcl, the behavioral file name defined on line 10 (outside the diff) is set to make_8x8_1rw1r_behavioral.v:
set behavioral_file [make_result_file make_8x8_1rw1r_behavioral.v]However, this test is for make_8x8_2r1w_sky130, and the golden file compared on line 36 is make_8x8_2r1w_sky130_behavioral.vok. This copy-paste inconsistency makes the test confusing to maintain. It is highly recommended to update line 10 to use make_8x8_2r1w_sky130_behavioral.v instead.
rovinski
left a comment
rovinski
left a comment
There was a problem hiding this comment.
Fix the one Gemini comment and then it's good
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
3 participants
Summary
Remove
-power_pin/-ground_pinflags from 2r1w test tcl files since #10580 adds support for auto detecting these pinsType of Change
Impact
Update tcl files since no manual power pin names needed to be specified
Verification
./etc/Build.sh).Related Issues
#9392
Updates separately per feedback on #10784
@rovinski