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
--savable generates invalid c++ for some packed arrays #1465
Comments
Original Redmine Comment First thanks for making a patch and improving the tests. The patch seems reasonably close, just a few little things.
Thanks! |
Original Redmine Comment I see yes, I never bothered to look at the definition of @isWide@ etc, I was just mirroring the code in @emitVarDecl@ to ensure the decision would match. Certainly what I wrote looks equal to @baiscp->isWide()@. I'm note sure the @string@ part is going to work; I believe we don't know at this point if @basicp@ is a @BasicDType@ or a @PackArrayDType@ (this was one of the mistakes the old code failed to consider). I suppose we could attempt to cast to a @BasicDType@ and check the @Keyword@ if that succeeds. I'm not sure if a packed array of strings is possible, but that would not work if so. As I mentioned the patch I submitted works in the test case (which includes a @string@ variable) so I'm not sure if it's necessary to check explicitly. As for the assignments in @VN_CAST@, are you happy with use of the comma operator to take care of this or would you prefer the more verbose but potentially clearer breaking into multiple statements outside the @for@? |
Original Redmine Comment Strings can't be packed arrays, so we're probably ok there. If you're ambitious try adding an array of strings to the test, but might cause other breakage. Yes, please go for multiple statements in the VN_CAST. |
Original Redmine Comment Changes made, please find the new patch attached. |
Original Redmine Comment Thanks much. Pushed to git towards 4.015. |
Original Redmine Comment In 4.016. |
Author Name: Alex Chadwick (@Chadderz121)
Original Redmine Issue: 1465 from https://www.veripool.org
Original Assignee: Alex Chadwick (@Chadderz121)
When trying to use @--savable@ I noticed compiler errors in the generated C++ code for the save/restore routines. This seems to be the result of improper code being generated for some types of variable; notably multidimensional packed arrays. I'm not familiar with verilator's internals, but I've attached a patch which I believe should work in all cases. I have tried to mirror the code that declares types to generate appropriate save/restore routines accordingly. The only thing I'm not clear on is whether the code I've written will work properly for opaque types like strings, but it does seem to in practice. I've also extended the regression test @t_savable@ to include multidimensional packed arrays.
Thanks in advance,
Alex
The text was updated successfully, but these errors were encountered: