chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.24k stars 1.13k forks source link

Cannot compile ExampleTop as the top-level module #359

Closed ben-k closed 8 years ago

ben-k commented 8 years ago

This is essentially reopening #308, albeit with a different proposed solution. We'd like to split out the generation of the verilog testharness with that of the actual DUT to enable gate-level simulation. We like the suggestion by @zhemao about how to do this:

You should define a separate testbench that makes the dut a BlackBox. Then the post-synth sim scripts could run the generator with MODEL=BlackBoxedTestHarness and MODEL=Top to generate the two verilog files.

However, it seems that the new top-level organization does not allow this strategy. Setting MODEL=ExampleTop yields the following error:

java.lang.ClassCastException: rocketchip.ExampleTop cannot be cast to chisel3.core.Module

Perhaps LazyModule does not like to be the top-level Chisel object. Any suggestions on how to compile to verilog that has ExampleTop as its topcell?

zhemao commented 8 years ago

Yeah, that's because it's a LazyModule and not a Module. There are two ways we could do this. The easy way to be just wrap the ExampleTop lazy module in another Module. So something like.

class ExampleTopWrapper(p: Parameters) extends Module {
    val top = LazyModule(new ExampleTop(p)).module
    val io = top.io.cloneType
    io <> top.io
}

The harder but more general way would be to allow the generator to directly instantiate the module implementation of a LazyModule.

ben-k commented 8 years ago

OK, the "easy way" more or less works. (It's a bit goofy in that it leaves a needless level of hierarchy in the verilog, but beggars can't be choosers, I suppose.)

One other question before I close this - what's the motivation for the new $(long_name) naming scheme that removes $(MODEL) from the generated file names? The problem here is that changing $(MODEL) doesn't change the targets, so the Makefile dependencies are all screwy.

hcook commented 8 years ago

The point is that each PROJECT should be providing its own Top, TestHarness(es), and Generator(s) according to what files it needs to generate and what backend build flow its interfacing with. If you'd like to define longName to include the name of the top module, you could write a Generator that does so (different Generators can be invoked using sbt's run-main command).

Then the post-synth sim scripts could run the generator with MODEL=BlackBoxedTestHarness and MODEL=Top to generate the two verilog files.

Why do you need two separate verilog files? If you generate a .v that includes both a test harness and the DUT modules, why can't you use that same .v file with the backend simulation tools? Don't you get to pick which verilog module is treated as the top by each tool? That's what MODEL should be for, not changing frontend targets.

zhemao commented 8 years ago

The problem is that when you go to run the test harness, it will have two different implementations of the dut, the one generated by FIRRTL and the post-synthesis implementation from VCS.

zhemao commented 8 years ago

That's just a problem we have to solve on our end. It isn't actually affected by this naming thing. We will need a different verilog wrapper for the post-synthesis simulation anyway.

ben-k commented 8 years ago

Actually, I think that you would want to use the same testbench for RTL and gate-level simulation. Adding special cases for post-synthesis operation seems like an excellent way for bugs to creep in.

We have gone ahead and created our own Top, TestHarnesses, and Generator as you suggested, and we've changed longName in the generator to suit our needs. We're still a little hung up on the testharness naming issue - it seems safest to just re-use TestDriver.v to avoid code duplication, but that file has no provision for differently-named testharnesses.

Nonetheless, I'm going to go ahead and close this issue. It sounds like a consistent system has been established on rocket-chip master; as long as that's the case (and there are not changes moving forward), then we can work around any remaining issues.