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

tests: fix blockrom.v driver conflict #4783

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

widlarizer
Copy link
Collaborator

Prior to this PR, integer j had two drivers: a constant, and the result of the xorshift chain. This can be verified by replacing the contents of tests/arch/ice40/memories.ys with the following pass sequence. The driver conflict created a loop which later somehow would get resolved, but it killed opt_expr topological ordering and inflated make test runtime. This PR reduces the duration of make test -j24 on my machine by a factor of >2.5.

design -reset; read_verilog -defer ../common/blockrom.v
chparam -set ADDRESS_WIDTH 2 -set DATA_WIDTH 2 sync_rom
hierarchy -top sync_rom
proc -noopt
check

@@ -10,8 +10,9 @@ module sync_rom #(parameter DATA_WIDTH=8, ADDRESS_WIDTH=10)
reg [WORD:0] data_out_r;
reg [WORD:0] memory [0:DEPTH];

integer i,j = 64'hF4B1CA8127865242;
Copy link
Member

Choose a reason for hiding this comment

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

integer is a variable, so this should be static initialization, not a constant driver. I think Yosys interprets this wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tracking separately as #4792

@widlarizer widlarizer force-pushed the emil/blockrom-driver-conflict branch from dd9144c to c26966e Compare December 2, 2024 15:56
@widlarizer widlarizer merged commit 5233636 into main Dec 3, 2024
35 checks passed
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.

2 participants