Verilog $readmemh is rather limited and reads one array value per white-space separated number. In order to read into a 32-bit or 64-bit array, the data must be formatted accordingly. I propose to modify the verilog target in order to add data width specification. I see several ways to go about this: * define verilog16, verilog32, and verilog64 targets, so that the target specifies the data width * default to the data width of the architecture * add options to object copy similar to srec-len. The first option is the most flexible, but I'm looking for the option that binutils maintainers would be willing to accept.
Hi Jamey, I would prefer the option with the least impact on the current sources. Ie the one which would introduce the least number of potential new bugs. To me this sounds like your third option - a new option to objcopy - since in theory the new code would be restricted to just that program, and not need to affect the BFD library itself. Does this make sense ? Cheers Nick
After talking about it with others who use the verilog output, a new option seems best because it is the most flexible. The data width of a ROM does not necessarily match the data width of the processor. I think the changes will be very small in this case. A new option in objcopy and a change to verilog.c to use the specified data width.
Created attachment 9170 [details] patch adding --verilog-data-width option to objcopy This patch adds --verilog-data-width option to objcopy. It includes several test cases.
This works fine, but the byte ordering caused by "for (i = VerilogDataWidth-1; i >= 0; i--)" is a bit unexpected, at least on OpenRISC. I'm not sure yet if we need to change the ordering only on BE targets, or if should be reversed on both LE and BE targets
Hi Jamey, > This patch adds --verilog-data-width option to objcopy. It includes several > test cases. The BE vs LE issue mentioned by Olof seems to be the most important issue, but there are a couple more comments that I would like to add: * Please do not use C++ style // comments. * You should add a line to the binutils/NEWS file mentioning this new feature. You should also add some text to binutils/doc/binutils.texi describing the new objcopy option and how to use it. Cheers Nick
Created attachment 9179 [details] revised patch
Hi Jamey, > revised patch Thanks for this - but you need to update the tests to cope with both types of endian target. Cheers Nick
Full ack from the OpenRISC side. Would be nice to get rid of the Windows (CR-LF) line endings as well in the verilog backend, but that can wait for another patch I guess
Adding a comment here to ask if someone could actually work on this bug. I reported it on the RISC-V tools github here. https://github.com/riscv/riscv-tools/issues/168#issuecomment-358165027 When I first reported it was assumed it would have been fixed and pushed upstream by now. Apparently we were wrong and now someone else also wants the same functionality and found my original report. Can someone please fix this? This functionality is useful for bare metal RISC-V Verilog processors.
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=37d0d09177dc02e0002ab8b90d9b7bc402af9240 commit 37d0d09177dc02e0002ab8b90d9b7bc402af9240 Author: Jamey Hicks <jamey.hicks@gmail.com> Date: Tue May 14 10:40:04 2019 +0100 Add new option to objcopy: --verilog-data-width. Use this option to set the size of byte bundles generated in verilog format files. PR 19921 binutils* objcopy.c: Add new option --verilog-data-width. Use it to set the value of VerilogDataWidth. * doc/binutils.texi: Document the new option. * testsuite/binutils-all/objcopy.exp: Run tests of new option. * testsuite/binutils-all/verilog-1.hex: New file. * testsuite/binutils-all/verilog-2.hex: New file. * testsuite/binutils-all/verilog-4.hex: New file. * testsuite/binutils-all/verilog-8.hex: New file. * NEWS: Mention the new feature. bfd * verilog.c: (VerilogDataWidth): New variable. (verilog_write_record): Emit bytes in VerilogDataWidth bundles.
(In reply to Donato Kava from comment #9) > Adding a comment here to ask if someone could actually work on this bug. Well nobody else seems to want to fix the testsuite problems, so I have gone ahead and done so myself. A little bit of code tidying as well, and the patch is now in. Cheers Nick
Created attachment 11773 [details] attachment-42343-0.html Thank you, Nick! On Tue, May 14, 2019 at 5:51 AM nickc at redhat dot com < sourceware-bugzilla@sourceware.org> wrote: > https://sourceware.org/bugzilla/show_bug.cgi?id=19921 > > Nick Clifton <nickc at redhat dot com> changed: > > What |Removed |Added > > ---------------------------------------------------------------------------- > Status|NEW |RESOLVED > Resolution|--- |FIXED > > --- Comment #11 from Nick Clifton <nickc at redhat dot com> --- > (In reply to Donato Kava from comment #9) > > Adding a comment here to ask if someone could actually work on this bug. > > Well nobody else seems to want to fix the testsuite problems, so I have > gone ahead and done so myself. A little bit of code tidying as well, > and the patch is now in. > > Cheers > Nick > > -- > You are receiving this mail because: > You reported the bug.