pulp-platform / snitch_cluster

An energy-efficient RISC-V floating-point compute cluster.
https://pulp-platform.github.io/snitch_cluster/
Apache License 2.0
48 stars 46 forks source link

target: Add `VLT_NUM_THREADS` to configure threads used by verilator #148

Closed zero9178 closed 1 month ago

zero9178 commented 3 months ago

Using more threads in the simulation may lead to a speedup in simulation time, especially when the program running in the simulator makes use of multiple cores. This PR therefore adds the VLT_NUM_THREADS option to make to allow users to set the thread number used in the simulator. For backwards compatibility, and since the number of threads is environment and code dependent, the default value of 1 was kept.

zero9178 commented 3 months ago

I am seeing issues with the simulation where it seemingly gets stuck/deadlocks for non-obvious reasons. While the PR itself is okay its probably not a good idea to actually use any value but 1 for the time being.

colluca commented 3 months ago

I am seeing issues with the simulation where it seemingly gets stuck/deadlocks for non-obvious reasons. While the PR itself is okay its probably not a good idea to actually use any value but 1 for the time being.

See also issue https://github.com/pulp-platform/snitch_cluster/issues/116.

@and-ivanov may know something more about this. Tagging him in case he might have further comments.

colluca commented 2 months ago

Dear @zero9178,

I opened a new PR upgrading the supported Verilator version to 5.006 (https://github.com/pulp-platform/snitch_cluster/pull/158). I tentatively added your multi-threading contributions from this PR. If you would like to try it, perhaps it fixes the issues you mentioned above. If this is the case we can close this PR in favour of https://github.com/pulp-platform/snitch_cluster/pull/158 and merge it.

Best, Luca

zero9178 commented 2 months ago

Dear @zero9178,

I opened a new PR upgrading the supported Verilator version to 5.006 (#158). I tentatively added your multi-threading contributions from this PR. If you would like to try it, perhaps it fixes the issues you mentioned above. If this is the case we can close this PR in favour of #158 and merge it.

Best, Luca

That's amazing! Let me run the verilator model from your PR through our stack and I'll get back to you whether it works (I'll respond on that PR). If yes then I think we can close this. If the multithreading part does not work then I'd either leave this open and/or make this an issue.

colluca commented 1 month ago

Finally merged in https://github.com/pulp-platform/snitch_cluster/pull/158. Thanks again for the help @zero9178!