bespoke-silicon-group / bsg_manycore

Tile based architecture designed for computing efficiency, scalability and generality
Other
223 stars 58 forks source link

Verilator support #589

Closed dpetrisko closed 2 years ago

dpetrisko commented 2 years ago

This PR adds support for verilator compilation. Very naively, I just split the compilation makefile into Makefile.vcs and Makefile.verilator.

There are some random changes included which I'll clean up, but I wanted to get feedback on the preferred way of switching between the compilation tools. It doesn't have to be super easy to go back and forth. Presumably, VCS will continue to be the preferred BSG way with Verilator being used by external contributors. For instance, we could only allow either simv or simsc to be built at a given time, etc. Whatever makes this easy to reason about. To throw another wrench in, I plan to add Xcelium (Cadence) simulator support next, so whatever option we choose should scale to that as well.

(This builds on https://github.com/bespoke-silicon-group/bsg_manycore/tree/small-pod for the small manycore, which makes verilator compilation palatable)

dpetrisko commented 2 years ago

Note that this requires a pretty recent verilator that incorporates the patch for DPI clock generator support. The version on our group server is recent enough, that's what I've been developing with

drichmond commented 2 years ago

Note that this requires a pretty recent verilator that incorporates the patch for DPI clock generator support. The version on our group server is recent enough, that's what I've been developing with

What did the patch do? Is this to label the clock output of the clock generator as a clock?

dpetrisko commented 2 years ago

What did the patch do? Is this to label the clock output of the clock generator as a clock?

https://github.com/verilator/verilator/pull/3091

It basically would label as a clock but the scheduler didn't recognize that the clock signal could change at any time, since it wasn't in the global ordering. (Normally clocks are inputs which are assumed to be volatile)

drichmond commented 2 years ago

Cool

TODO: We should increment the verilator submodule in bladerunner to get this PR.

dpetrisko commented 2 years ago

Yeah bladerunner verilator platform is the next step PR

taylor-bsg commented 2 years ago

Generally the best practice for spurious assertions and resets is && (reset_i !== 0) which includes X and 1.

On Mon, Oct 4, 2021 at 4:49 PM Dan Petrisko @.***> wrote:

@.**** commented on this pull request.

In testbenches/common/v/spmd_testbench.v https://github.com/bespoke-silicon-group/bsg_manycore/pull/589#discussion_r721790808 :

@@ -190,5 +192,15 @@ module spmd_testbench(); ,.ctr_r_o(global_ctr) );

  • // Disable assertions until reset is lowered

Can remove if you want, but there are a bunch of spurious assertions that trigger before reset is lowered and this cleans that up

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/589#discussion_r721790808, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AEX3YGKYCWKN6YRI7DUFI4RBANCNFSM5FBR6BDQ .

dpetrisko commented 2 years ago

Generally the best practice for spurious assertions and resets is && (reset_i !== 0) which includes X and 1.

  1. Verilator doesn't support X
  2. Does not work for a pulsed reset which is 0 at the beginning (____|----|____) which is the case for many testbenches
dpetrisko commented 2 years ago

I'll add a 1b. which is that I've seen companies use 2-state variants of simulators for speed when they're not concerned about X-prop (power+performance validation). But admittedly, you'd probably disable assertions entirely in that case

taylor-bsg commented 2 years ago

I think the challenge is, this assumes you have uniform reset across the chip. I guess you could turn off assertions after you have asserted reset?

Dustin, maybe worth benchmarking manycore simulation with 0/1 for the paper?

M

On Mon, Oct 4, 2021 at 6:00 PM Dan Petrisko @.***> wrote:

I'll add a 1b. which is that I've seen companies use 2-state variants of simulators for speed when they're not concerned about X-prop (power+performance validation)

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/589#issuecomment-933972926, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5ABLH2UBGXGFR4DRNC3UFJE3NANCNFSM5FBR6BDQ .

dpetrisko commented 2 years ago

I think the challenge is, this assumes you have uniform reset across the chip. I guess you could turn off assertions after you have asserted reset?

Yeah that's true. AFAIK, most simulators support $assertoff(hier.path.to.reset.domain). So you could do it in stages as the chip comes out of reset for more complex situations. In the manycore case, there's only a single domain so we only care about propagated rather than heterogeneous reset. Therefore, enabling assertions when reset falls is sufficient assuming that reset=1 has propagated throughout the chip

drichmond commented 2 years ago

Dustin, maybe worth benchmarking manycore simulation with 0/1 for the paper?

If you know how to do this in VCS, that sounds great! I've looked several time and as far as I remember this feature was removed from VCS some time ago.

dpetrisko commented 2 years ago

Comments addressed, I think this is good to go unless others have feedback

dpetrisko commented 2 years ago

Any last comments?