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

Updated verilator waivers so the sim builds #91

Closed
wants to merge 1 commit into from

Conversation

HU90m
Copy link
Member

@HU90m HU90m commented Jan 31, 2024

No description provided.

Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes. I had been meaning to fix the Verilator lint file for a while. On which version of Verilator did you test this?

ibex_demo_system_core.core Outdated Show resolved Hide resolved
@nbdd0121
Copy link
Contributor

nbdd0121 commented Feb 2, 2024

I think the lint that you're trying to mute is a legit warning.

In ibex_register_file_fpga, if RdataMuxCheck is not enabled then the oh_raddr_a_err and oh_raddr_b_err wires are indeed not driven. Probably that should be a RTL fix instead of lint fix.

@HU90m
Copy link
Member Author

HU90m commented Feb 7, 2024

Thanks for making these changes. I had been meaning to fix the Verilator lint file for a while. On which version of Verilator did you test this?

Verilator 4.210 (The officially supported ibex version)

@HU90m
Copy link
Member Author

HU90m commented Feb 7, 2024

I think the lint that you're trying to mute is a legit warning.

In ibex_register_file_fpga, if RdataMuxCheck is not enabled then the oh_raddr_a_err and oh_raddr_b_err wires are indeed not driven. Probably that should be a RTL fix instead of lint fix.

I've moved the Verilator top over to using ibex_register_file_ff in #93, which it should have been using anyway and with which this waiver isn't needed.

Closing this PR in favour of #93

@HU90m HU90m closed this Feb 7, 2024
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.

5 participants