-
Notifications
You must be signed in to change notification settings - Fork 302
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
[ImportVerilog] Update to Slang 4 #6629
Conversation
@teqdruid This Slang version bump gets rid of the RTTI/EH issues. Last time I run this though I was having trouble with the older compiler versions present in the CI image. You mentioned you might be willing to look into updating them? I'm also not entirely sure how to get this to build with MSVC... maybe you have some insight 🤔 |
c3d2a14
to
eaeaa9b
Compare
hmm, depending on concepts, that's quite a jump. |
Yeah |
I'm finally working on this. Seems to be very sensitive to both clang and gcc versions (even though they have both "supported" c++20 for some time). We should add version checks to prevent random build failures. Still trying to determine the minimum versions. |
How do folks in general feel about modern C++ versions? As in, do we care if CIRCT requires a fairly modern compiler, with the benefit of using newer Slang versions, or do we care about CIRCT being easy to deploy on compilers that are older than a year, with the downside of older Slang versions and having to do some manual work to get exceptions out? |
I have no problem with optional parts of CIRCT requiring newer versions of compilers, so long as we check. I do, however, feel somewhat strongly that the non-optional stuff should compile on older compilers. A lot of people (including myself) still use Ubuntu 20.04, which has pretty ancient versions out-of-the-box. This is especially true if we ever want to upstream anything. Back porting the new no RTTI/EH features from slang seems like a bad idea to me. We'd be stuck on an older version without any new features! |
That makes sense. How would we go about ensuring that everything works on older compilers? Would we add a CI job that runs on some minimum version of Ubuntu that builds with the non-optional parts? It would be great if we had regular CI with all stuff enabled, such that it is properly checked; but that also requires us to have usual CI run on very new compilers, which just asks for the backwards compatibility to slowly rot away if we don't explicitly check for it. |
The CI images now run Ubuntu 20.04. I'm in the process of updating the integration image to the minimum versions (clang 16 and gcc 10, since clang uses g++'s stdlib) so we'll at least be able to test import verilog there. This is what I do for the ESI runtime -- it's only tested in the integration tests, both the short one which runs on PRs and the nightlies. In general, it's the only pipeline which tests everything. Do you want import verilog to run on the standard pipelines? |
Building with clang requires clang-16 and g++-10 (since clang uses g++'s stdlib). |
@fabianschuiki For GCC, g++-11 seems to get through slang, but fails with:
|
Actually, that error occurs even in the clang build... wonder how I got through it in my test builds. |
Let me rebase this onto master. Probably something broken since we've added |
I've confirmed that the slang_slang target builds with clang-16 and gcc-11. I've (tried to) enable slang builds in the CI workflow for gcc builds only. This'll probably still break the nightly matrix test (since it doesn't know about the compiler requirements) and gcc-11 isn't in the integration_test image. Will add it now to take care of the latter problem. |
I've pushed the new images into main: 017bf54 |
Awesome 🥳 Thanks a lot for the help! I'll see what I can do about the Slang build. There are a few weird things going on related to the packaged version of |
Bump the Slang dependency to version 4. This version supports builds with exceptions and RTTI fully disabled, which allows us to get rid of the ugly glue code that tried to bridge the gap between the RTTI/EH world of Slang and the non-RTTI/non-EH world of LLVM. Co-authored-by: John Demme <[email protected]>
69e1045
to
c25579d
Compare
Bump the Slang dependency to version 4. This version supports builds with exceptions and RTTI fully disabled, which allows us to get rid of the ugly glue code that tried to bridge the gap between the RTTI/EH world of Slang and the non-RTTI/non-EH world of LLVM.