Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tester unification #551

Closed
ducky64 opened this issue Mar 15, 2017 · 26 comments
Closed

Tester unification #551

ducky64 opened this issue Mar 15, 2017 · 26 comments

Comments

@ducky64
Copy link
Contributor

ducky64 commented Mar 15, 2017

Currently, we have three tester interfaces:

  • Circuit ("synthesizable") tester, the only tester included in chisel3 (everything else lives in a separate repo). The testdriver, like the DUT, is synthesizable hardware and is written in Chisel.
  • Peek-poke tester, where Scala drives the device-under-test through peek, poke, and check commands. Obviously non-synthesizable, since it's possible to peek values, do some compute in Scala-land, and poke the results back.
  • Advanced tester, like the peek-poke tester, but provides parallel actions with a limited amount of interfaces (like Decoupled).

Of course, all these testers have their problems and advantages (some fundamental, some engineering):

  • Neither peek-poke nor advanced tester are available in the chisel3 repository. You have to include a separate dependency. Minor usability nit, though I think many like Python's approach of "batteries included".
  • Circuit tester is great for lightweight or concurrent tests, but sequencing events in time usually requires the test writer to manually write a state machine. Not fun.
  • Circuit tester is synthesizable, meaning that full testbenches can be deployed on FPGA (and generally are self-contained). Apparently some people actually do this.
  • Circuit tester doesn't support any backends other than Verilator, and Verilator has a high compilation cost. The chisel3 standard tests usually take several minutes to run, probably a good percentage of that time is compiling design through Verilator.
  • It's difficult to specify concurrency (see Better tester concurrency specifications? #547) in the peek-poke tester, and advanced tester only allows limited concurrency models. The circuit tester has concurrency native from its circuit nature.

The goal here is to unify all the different testers into a single tester included in Chisel3.

Reasoning:

  • support all the different backends (mainly for performance reasons: chisel3 tests may run faster through the FIRRterpreter)
  • unify the test specification model (which may be a hybrid approach, as detailed below)
  • include it in base chisel3 (tests aren't optional for engineers serious about hardware design).

Some hybrid approaches have been suggested:

  • If synthesizability isn't useful, then it's possible to write the circuit tester programming model in the peek-poke tester framework. Only the step command needs to be added to the peek-poke tester.
  • It is also possible to translate series of step, check, and pokes to synthesizable hardware, having Chisel generate the state machine. There would be no poke support, since the testbench would be standalone and not have a Scala testdriver.
    • Several attempts have been made before with systems like these, for whatever reason they have never gained traction.
    • This may make testvector literal specification a non-issue, since all test constructs live in Chisel-land and can seamlessly use Chisel literals and future features like Bundle literals.

@jackkoenig @jackbackrack @sdtwigg @dt27182 @chick @azidar @aswaterman

@donggyukim
Copy link
Contributor

As a heavy user & developer of peekpoke testers 1~3 years ago, I strongly disagree with this suggestion unless the following issues are resolved:

  • The main reason I stopped using peekpoke testers is it's extremely hard to use in the development mode. Once you have a submodule of chisel-testers in your project, you need to local-publish chisel, firrtl, and firrrtl-interpreters altogether. Don't tell me I need to local-publish them with this change. This sounds like adding additional dependencies on chisel, which will worsen Workspace Issues: Compile errors with firrtl master #499.
  • For the above reason, I switched all peek-poke testers to hw testers to cut out the dependencies.(e.g. No more chisel testers ucb-bar/riscv-mini#2) Never tell me I should change testers again with this change.

@ducky64
Copy link
Contributor Author

ducky64 commented Mar 15, 2017

So there are two separate goals for this issue:

  1. Determine the underlying programming model (or a hybrid approach). Synthesizable circuit-based, Scala-driven peek-poke, synthesized check-poke, etc...
  2. Determine the implementation strategy, include packaging strategy. I would advocate for everything to be included in chisel3 instead of the current chisel-testers and dependency hell we have right now.

So having a peek-poke programming model isn't mutually exclusive with not having dependency hell, and we should make sure to discuss the pros and cons of each issue separately.

@jackkoenig
Copy link
Contributor

You're right that there are two separate goals, but I think @donggyukim is right in that fixing the packaging strategy should be super high priority because the current system doesn't really work. Unifying the repos won't fix the problem because this problem affects anyone who wishes to create a repo with their own Chisel utils or custom Firrtl passes (eg. https://github.com/ucb-bar/barstools).

To the best of my knowledge, sbt does not have an easy way of doing development with local versions (as we have discussed in #499). I recently found a couple of StackOverflow posts that might be helpful:

@shunshou
Copy link
Contributor

I'd be kind of upset if you entirely remove the peek-poke setup. It's super easy to get started with and mimics how some people would write (non-synthesizable) test benches.

And I agree that the packaging strategy right now is a huge nightmare. Every time I want to update something, I hate to go through 5 different dependencies and all publish-local and hope that I'm on compatible versions.

I spent a good hour last night just updating firrtl and everything else and hoping that no dependency broke. (and then I ran into the problem of barstools still not actually compiling...).

@dt27182
Copy link
Contributor

dt27182 commented Mar 17, 2017

+1 on the sbt packaging issues

It seems like there are 2 options for getting everything that we what (easy concurrent test drivers and easy sequencing of events in time).

One option is to stick with the scala based software test model and implement some sort of event driven scheduling system. This option provides the full power of scala in writing the driver logic, but writing a event scheduler seems like a lot of work and a step back to verilog ...

The other options is to synthesize peek (check?), poke, step into Chisel state machines. This option provides good backwards compatibility with the existing hardware based tests, so people don't have to rewrite the tests yet again, which is a pretty big pro. However, there is less power in how to sequence the peek, poke, steps and there might be issues with running very large test vectors with this system since everything has to be synthesized into hardware.

I think I prefer the synthesized peek, poke, steps approach. Very large software driven tests probably don't need to concurrent drivers since they are whole system tests, so they can probably be written against the verilator produced C++. Also, don't we already have the SteppedHWIOTester? Is nobody using that anymore?

@ducky64
Copy link
Contributor Author

ducky64 commented Mar 17, 2017

@shunshou when you want to write non-synthesizable testbenches, is it because you want to be able to control things in timesteps, or because you need to do some compute in Scala? The first could be solved in a synthesizable test specification model by having a framework to generate a state machine from check / poke / step commands, while the second is pretty much incompatible with a synthesizable framework.

One issue with synthesizable testbenches would be the delay model to use, since a common programming model is waiting until some condition (like signal valid) is true. A solution might be just wait(expr), which steps until expr === true.B.

It also seems like everyone here hates the packaging setup. What does everyone want to see there? A mega-repo of chisel3, FIRRTL, FIRRterpreter, and chisel-testers? Or some other split of those repositories?

It seems the SteppedHWIOTester is still there, but not much work has happened on it. Which I think is a good reason we should unifying testers...

@dt27182
Copy link
Contributor

dt27182 commented Mar 17, 2017

Mega repo with git submodules seems like a logical solution. What was the reason for moving away from that model before? I know rocket had some issues with it. What where they?

@jackkoenig
Copy link
Contributor

jackkoenig commented Mar 17, 2017

@dt27182 The project repo with git submodules is still the goal, the problem comes from dependencies between these submodules and sbt's lack of support for specifying that you want to use a local build instead of the library dependency. I'm trying to figure out how to work around this (see the StackOverflow links above), but I'm pretty sure it's impossible to do 100% with sbt, so we can look into using some sort of setup script or possibly environment variables to workaround.

@shunshou
Copy link
Contributor

@ducky64 for both reasons. Synthesizable tb makes sense, but only after I do sufficient debug with peek/poke. It's like I personally prefer to peek and stare at stuff before I convert everything into expects.

@dt27182
Copy link
Contributor

dt27182 commented Mar 17, 2017

@jackkoenig
Sbt does support using local builds via sbt subprojects right? (I think this is what we were using before) Is it just that sbt doesn't allow a way to switch between subprojects and library dependencies?

And what is the reason we want to publish the subparts as sbt libraries?

@ducky64
Copy link
Contributor Author

ducky64 commented Mar 17, 2017

@shunshou do you think the peek-for-debug could be replaced with a cycle-dependent printf?

@jackkoenig
Copy link
Contributor

@ducky64 a single mega repo is a one time solution that doesn't fix the fundamental problem. Anyone who wants to create a chisel library that uses an sbt library dependency in the common case but can also be used in development mode will run into the same problem.

@dt27182 That is correct, and I think you've hit the nail on the head.

Should we be publishing the subparts all separately? We currently have a 1-to-1 mapping of git repos to sbt libraries. Perhaps it would be better to publish everything together again. The repos can stay separate and instead the chisel-release repo publishes them as a single library.

@jackkoenig
Copy link
Contributor

Publishing together still doesn't solve the problem of separate chisel libraries though. I have an idea that might work:

Libraries like barstools have two sbt projects in them: barstools and barstools-dev. barstools has a library dependency on chisel3 and whatever else. barstools-dev is kind of a raw project that is intended to be used as a git submodule and have a parent project handle it's dependencies. Does this make sense? I'm kind of just spitballing here.

@shunshou
Copy link
Contributor

@ducky64 does the printf thing work w/ Verilator?

@jackkoenig
Copy link
Contributor

@shunshou Yes it does! And checkout the chisel3 wiki page about printing (if you don't know about Chisel's custom string interpolator for printf).

@jackkoenig
Copy link
Contributor

Basically, you can do stuff like

val (cycle, _) = Counter(true.B, 10)
val myReg = Reg(UInt(32.W))
printf(p"At cycle $cycle: myReg = $myReg\n")

@shunshou
Copy link
Contributor

@jackkoenig so my issue there is -- at least for PeekPokeTester, I can't peek/poke internal signals with Verilator. Idk how using an inline printf resolves this issue. Please explain?

@jackkoenig
Copy link
Contributor

Ah yeah it doesn't fix that

@shunshou
Copy link
Contributor

But you're saying that if I printf IO, I can do it even with Verilator?

Printf stuff makes sense with firrtlInterpreter, since you have access to internal nodes, but at least with PeekPoke, I can bring the internal signals out and hook it up to Verilator.

If I don't have the PeekPoke connection to Verilator, idk how that helps me any.

@ucbjrl
Copy link
Contributor

ucbjrl commented Mar 17, 2017

build.sbt can be configured to support either subproject or libraryDependencies for a project (it does use Scala after all) but:

  • it's not pretty,
  • it requires a triggering mechanism (how do you know which flavor to build)

We should make packaging a separate issue (from tester unification).

@ducky64
Copy link
Contributor Author

ducky64 commented Mar 22, 2017

Ok, the summary so far seems to be (and please correct me if I'm wrong):

  • The packaging issue sucks, bad. Will be discussed separately in Fix Packaging For Development Use #556, but whatever new tester we make should be in chisel3 (main repository) to avoid dependency hell.
  • It seems no one has a solid use case for peek that printf can't address (internal signals aside). We might want to find some way for tester-level printf to work with internal signals.
  • I've seen no one really advocating for Scala-driven (peek-poke) test flow, so this seems like an unimportant use case. Especially since tester concurrency would require basically reimplementing Verilog.

And from the initial post we have:

  • Some people actually want to synthesize tests to FPGA.
  • The step-by-step testvector specification model is useful.

So the solutions seems to be:

  • Synthesizable testbench
  • Figure out printf on internal nodes / with Verilator
  • check/poke/step commands generated into a hardware state machine. This could be a general Chisel utility rather than a tester-specific construct.
    • Possibly additional timing constructs, like waitFor. We'll need to figure out a good unifying model so that this is sufficiently general.
  • Concurrency model to be determined. Possibly synthesized fork/join concurrency and/or synthesized LTL constructs. This would be on top of any concurrency constructs provided by base Chisel HCL.

Does this sound like a good solution that addresses everyone's use case?

@ducky64
Copy link
Contributor Author

ducky64 commented Apr 5, 2017

It now seems that there are people who want to advocate for the Scala-driven testers (I think mostly for hardware models written in Scala?), which vastly complicates things now...

We're going to have a meeting next week about this. If you have thoughts and example use cases, please make them known on this thread. If you have a serious stake in this are are interested in attending, let us know...

@dt27182
Copy link
Contributor

dt27182 commented Apr 6, 2017

Interested in attending

@ducky64
Copy link
Contributor Author

ducky64 commented Apr 14, 2017

Ok, results from today's meeting:

Internal testing model

The fundamental testing model is circuit-based, with the DUT and test driver(s) being modules that are hooked up in a circuit simulator. The whole thing may or may not be synthesizable.

Rationale:

  • This defines a common tester API (between the DUT and tester environment).
  • Multiple test driver modules allow test drivers to be composed and split (i.e. instantiate different peripherals).
  • While we will need an event simulator to sequence circuit activity, this model may allow us to use Verilog.

Tester User APIs

We don't expect to be able to unify the tester API while supporting all possible use cases. Additionally, the above testing model allows a lot of flexibility in the implementation of each test driver module (such as calls to Scala code in black box test drivers).

Instead, we will focus (for now) on developing a testing API that will be the "default" for Chisel. These are the requirements we came up with:

  • Allow people to accomplish common testing tasks
  • Must have a sequential test specification API (i.e. poke, check, step), since this is a common way to write tests. This can synthesize into circuits.
  • This will probably be synthesizable overall.
  • Some kind of concurrency model would be nice, details TBD
  • Must have documentation, including limitations and recommendations for use cases past those limits.
  • Test literals specified as Chisel lits using literal extraction (rather than Scala types). This allows the use of near-future features (like Bundle literals and better enums) without a clunky parallel API.

Concurrency Model

Concurrency was discussed and is overall a hard problem. The model we were thinking of was a parallel ("threading") model, but with checks to prevent people from shooting themselves in the foot. The simplest is a dynamic check, which ensures within a cycle, peek results are not changed by poke results from another "thread" (and will assert out otherwise).

More advanced checks based on static (combinational dependence) analysis may be possible.

Misc

Misc notes that may be helpful:

  • There is some overlap (perhaps interop?) with MIDAS, a testing framework also being developed here.
  • Main argument for software testers was the ease of writing things in software and ability to use libraries. One specific use case was the ability to read out a circuit value and perform a fuzzy check (like partial Bundle check or FixedPoint check with tolerance).
  • SVA (SystemVerilog Assertions) is also interesting, basically LTL on circuits. Many can be synthesized, but some are difficult. At one point, this was a Chisel project.
  • Scala will ultimately likely be the top-level test driver (as opposed to C++, like for Verilator). Communications with C++ should be done using a high performance mechanism like JNI, since sockets are very slow. Bad performance was one of the reasons rocket has its own test infrastructure.

As always, thoughts and feedback are welcome!

@chick
Copy link
Contributor

chick commented May 8, 2017

I'd like to add build directory management must be overhauled. There are two competing use case

  1. When developing a circuit, a local, unchanging build directory makes inspection of files, e.g. firrtl output, annotations, etc makes life better
  2. Libraries and chisel repos, have many tests, often with distinct tests run against the same chisel circuit which leads to collisions when scalatest parallelizes its tests.
    • In this case test need to be accessible but named in some way to avoid collision
    • It would be really nice if there was some friendly cleanup of these directories and files because they can become an annoying waste of disk space
  3. Consider using scalatest beforeAndAfter and beforeAndAfterEach to manage this

@ducky64
Copy link
Contributor Author

ducky64 commented Dec 6, 2017

Superseded by #725.

@ducky64 ducky64 closed this as completed Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants