-
Notifications
You must be signed in to change notification settings - Fork 615
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Use c++14 to pass the verilator check #3876
Conversation
The committers listed above are authorized under a signed CLA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally seems fine to me. There shouldn't be an issue with requiring a compiler that supports C++14.
We haven't bumped the version of Tabby CAD OSS Suite in a while, so this likely hasn't cropped up. (We're using an older version of Verilator in CI.) We should probably do that, too, put that on latest, or add nightly/CD that tries to keep Tabby CAD up to date.
We should merge this, but a longer term fix is we should probably make the arguments version-specific so that we can support some older Linux distros and Verilator versions out-of-the-box. |
I also wouldn't call this "No release notes", we do now require a compiler with C++14 support which should work for most, but lots of users in the hardware industry are on surprisingly old Linux distros. |
@jackkoenig: Can we backport this and/or get this onto a non-snapshot release? I think this may be necessary for newer Verilator. I'm thinking this (chipsalliance/chisel-template#127) is related. |
(cherry picked from commit 6229155) # Conflicts: # svsim/src/main/scala/verilator/Backend.scala
(cherry picked from commit 6229155)
@seldridge sure, no reason not to release 6.2.0 (or 6.1.1?) with this. |
When I update to verilator v5.022, the svsim test will fail (someone has already raised an issue). I found out this was because verilator added a check for the c++14 standard.
Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
Change ChiselSim to requires a C++14 compiler.
Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.