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

Bump Verilator to v5.024 #223

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

RRozak
Copy link
Contributor

@RRozak RRozak commented Aug 12, 2024

It bumps Verilator to the version 5.024.
It can't be bumped to the newest, because it is currently unsupported by cocotb: cocotb/cocotb#3896.
I removed the assignment in the declaration of result variable to prevent from seg fault. That assignment is redundant, because the value is assigned in each branch of if and case.
The problem that cause seg fault is fixed in the newest Verilator: verilator/verilator#5365.

@@ -365,15 +367,15 @@ import el2_pkg::*;
buf_data_wr_en = buf_wr_en;

cmd_done = (ahb_hresp_q | (ahb_hready_q & (ahb_htrans_q[1:0] != 2'b0) &

Choose a reason for hiding this comment

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

[verible-verilog-format] reported by reviewdog 🐶

Suggested change
cmd_done = (ahb_hresp_q | (ahb_hready_q & (ahb_htrans_q[1:0] != 2'b0) &
cmd_done = (ahb_hresp_q | (ahb_hready_q & (ahb_htrans_q[1:0] != 2'b0) &

@@ -266,6 +266,8 @@ import el2_pkg::*;

// FIFO state machine
always_comb begin
// Temporary variable created to fix SIDEEFFECT warnings
logic [2:0] get_nextbyte_ptr_result = get_nxtbyte_ptr(buf_cmd_byte_ptrQ[2:0],buf_byteen[7:0],1'b1);

Choose a reason for hiding this comment

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

⚠️ [verible-verilog-lint] reported by reviewdog 🐶
Line length exceeds max: 100; is: 105 [Style: line-length] [line-length]

@@ -365,15 +367,15 @@
buf_data_wr_en = buf_wr_en;

cmd_done = (ahb_hresp_q | (ahb_hready_q & (ahb_htrans_q[1:0] != 2'b0) &
((buf_cmd_byte_ptrQ == 3'b111) | (buf_byteen[get_nxtbyte_ptr(buf_cmd_byte_ptrQ[2:0],buf_byteen[7:0],1'b1)] == 1'b0))));
((buf_cmd_byte_ptrQ == 3'b111) | (buf_byteen[get_nextbyte_ptr_result] == 1'b0))));

Choose a reason for hiding this comment

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

⚠️ [verible-verilog-lint] reported by reviewdog 🐶
Line length exceeds max: 100; is: 115 [Style: line-length] [line-length]

bypass_en = buf_state_en & buf_write_in & (buf_nxtstate == CMD_WR); // Only bypass for writes for the time being
ahb_htrans[1:0] = {2{(~(cmd_done | cmd_doneQ) | bypass_en)}} & 2'b10;
slave_valid_pre = buf_state_en & (buf_nxtstate != DONE_WR);

trxn_done = ahb_hready_q & ahb_hwrite_q & (ahb_htrans_q[1:0] != 2'b0);
buf_cmd_byte_ptr_en = trxn_done | bypass_en;
buf_cmd_byte_ptr = bypass_en ? get_nxtbyte_ptr(3'b0,buf_byteen_in[7:0],1'b0) :
trxn_done ? get_nxtbyte_ptr(buf_cmd_byte_ptrQ[2:0],buf_byteen[7:0],1'b1) : buf_cmd_byte_ptrQ;
trxn_done ? get_nextbyte_ptr_result : buf_cmd_byte_ptrQ;

Choose a reason for hiding this comment

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

⚠️ [verible-verilog-lint] reported by reviewdog 🐶
Line length exceeds max: 100; is: 105 [Style: line-length] [line-length]

@RRozak RRozak force-pushed the rrozak/62517-bump-verilator branch from ff8fc00 to f2d82b6 Compare August 14, 2024 11:05
@RRozak RRozak changed the title WIP: Bump Verilator and use one version in all tests Bump Verilator to v5.024 Aug 14, 2024
@RRozak RRozak marked this pull request as ready for review August 14, 2024 11:28
Copy link

Links to coverage and verification reports for this PR (#223) are available at https://chipsalliance.github.io/Cores-VeeR-EL2/

Internal-tag: [#62517]
Signed-off-by: Ryszard Rozak <[email protected]>
Internal-tag: [#62517]
Signed-off-by: Ryszard Rozak <[email protected]>
Internal-tag: [#62517]
Signed-off-by: Ryszard Rozak <[email protected]>
@RRozak RRozak marked this pull request as draft September 3, 2024 13:40
Copy link

github-actions bot commented Sep 3, 2024

Links to coverage and verification reports for this PR (#223) are available at https://chipsalliance.github.io/Cores-VeeR-EL2/

Copy link

Links to coverage and verification reports for this PR (#223) are available at https://chipsalliance.github.io/Cores-VeeR-EL2/

@kiryk kiryk marked this pull request as ready for review September 19, 2024 10:01
Copy link
Collaborator

@tmichalak tmichalak left a comment

Choose a reason for hiding this comment

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

LGTM

@tmichalak tmichalak merged commit 770ade3 into chipsalliance:main Sep 19, 2024
516 checks passed
@tgorochowik tgorochowik deleted the rrozak/62517-bump-verilator branch October 15, 2024 07:56
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