Fix uninitialized signal error #129
Conversation
Inky-13
changed the title
Inky-13
changed the title
| @@ -30,6 +30,7 @@ probe is required. | |||
| * openocd (version 0.11.0 or above) | |||
| * screen | |||
| * srecord | |||
| * verilator (version 5.024-1 or above) | |||
There was a problem hiding this comment.
I've been using 5.024, is there a difference between that and 5.024-1?
There was a problem hiding this comment.
I believe no difference. However, I saw it being reference here as 5.024-1 unsure if this is an ubuntu thing?
There was a problem hiding this comment.
I would remove the -1 because it is not reported by Verilator itself and it doesn't exist as a tag: https://github.com/verilator/verilator/tags
| @@ -53,7 +53,7 @@ static bool has_hpm_counter(int index) { | |||
|
|
|||
| std::string ibex_pcount_string(bool csv) { | |||
| char separator = csv ? ',' : ':'; | |||
| std::string::size_type longest_name_length; | |||
| std::string::size_type longest_name_length = 0; | |||
There was a problem hiding this comment.
Interesting that this wasn't flagged before. What behavior did this cause?
There was a problem hiding this comment.
When trying to build the verilator simulation using:
fusesoc --cores-root=. run --target=sim --tool=verilator --setup --build lowrisc:ibex:demo_system
I got the following error:
ERROR: ../src/lowrisc_dv_verilator_ibex_pcounts_0/cpp/ibex_pcounts.cc: In function ‘std::string ibex_pcount_string(bool)’:
../src/lowrisc_dv_verilator_ibex_pcounts_0/cpp/ibex_pcounts.cc:80:41: warning: ‘longest_name_length’ may be used uninitialized [-Wmaybe-uninitialized]
80 | int padding = longest_name_length - ibex_counter_names[i].length();
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/lowrisc_dv_verilator_ibex_pcounts_0/cpp/ibex_pcounts.cc:56:26: note: ‘longest_name_length’ was declared here
56 | std::string::size_type longest_name_length;
| ^~~~~~~~~~~~~~~~~~~
The full error message when building with the wrong verilator version (5.020) as well is in issue #125
There was a problem hiding this comment.
This is a change to vendored code, which essentially lives in another repository. The original file lives here: https://github.com/lowRISC/ibex/blob/master/dv/verilator/pcount/cpp/ibex_pcounts.cc
Would you like to make a PR in the Ibex repo or shall I do that for you?
There was a problem hiding this comment.
I've made a PR to fix this: lowRISC/ibex#2227
|
Thank you so much for making these changes by the way. Very useful to get these fixes in! |
I had some issues when building the core for simulation using verilator (see #125) This was first related to me using an older version of verilator as well as an uninitialized warning I was getting from verilator.
This PR documents adds verilator as a dependency and specifies the version and also initializes the variable that was causing the issue.
Closes: #125