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

When is it time to switch to ChiselTest #297

Closed
schoeberl opened this issue May 4, 2021 · 9 comments
Closed

When is it time to switch to ChiselTest #297

schoeberl opened this issue May 4, 2021 · 9 comments

Comments

@schoeberl
Copy link
Member

I am about to revise the Chisel book for a third edition (ETA before summer). In the second edition, I had PeekPokeTester only. Currently, both are described, which I find confusing for a beginner book. I am wondering if I should completely switch to ChiselTest. Opinions?

What are the plans for version 1.x? Maybe 1.6?

@ekiwi
Copy link
Collaborator

ekiwi commented May 4, 2021

am wondering if I should completely switch to ChiselTest.

I believe that ChiselTest already has some nice features, like overwriting the VCD from the prior test run, so that you can keep your waveform viewer open, that make it a good idea to use. If you stick to the simplest form of test with peek, poke, expect and step that should be relatively safe for people to use. There is a performance degradation that we have observed when comparing chiseltest and iotesters, but for the size of tests that you have in your book it should not matter. I would hold of on introducing the fork construct and anything related to multi-clock support as I believe that is a little more in flux.

What are the plans for version 1.x? Maybe 1.6?

I would be fairly pessimistic about that. Personally I do not want to do the work that would be required in order to ensure a stable API unless I am specifically paid to do so. So I would say that we are probably only going to see a 1.x once a company pays someone to maintain chiseltest as is the case with chisel and firrtl.

@schoeberl
Copy link
Member Author

Thanks for motivating me to move to ChiselTest. Actually, I introduce ScalaTest to run PeekPokeTester in order to run all tests with sbt test. This is quite a stretch and will come more naturally with ChiselTest.

I am aware of the performance degradation due to the multithreaded execution where coroutines would be enough (not yet available in JVM land). You are right that this does not matter for small examples. But, but... When testing larger designs, such a processor in co-simulation, this might kick in. Anyway, I guess I trust in the development of ChiselTest to catch up here.

fork should be a very nice feature. So was badly missing in iotesters, as this is implicit in VHDL or Verilog testbenches. Multi-clock, I guess is not an issue for my little book. I stick to kindergarden digital design: just one clock domain ;-)

OK, you see there is some company getting involved to advance ChiselTest? But without testing Chisel is dying, right?

@ekiwi
Copy link
Collaborator

ekiwi commented May 4, 2021

When testing larger designs, such a processor in co-simulation, this might kick in.

That is true. The reason I am warning you of fork is that we haven't yet found a way to maintain the fork API and improve the performance since, as you pointed out, there is no native co-routine support on the JVM or in Scala. I could see a situation where there will be a specific import like chiseltest.threaded._ if you want to use fork and take the performance hit and maybe a different import if you want to stay single threaded. Not sure how to do this in a (mostly) backwards compatible way yet though.

OK, you see there is some company getting involved to advance ChiselTest? But without testing Chisel is dying, right?

I think the main problem here is that SiFive does not currently use ChiselTest afaik. Rocket chip has its own verilator runner and it seems like it is going to stick with it. We need a company to actually start using chiseltest enough for them to want to sponsor a maintainer. I know that this sounds a bit like a chicken and egg problem, but I believe that for graduate students the incentives are not aligned for becoming a reliable maintainer. Like I am pushing some of chiseltest forward right now, but my research might change and then I might completely stop working on chiseltest in a couple of months.

@schoeberl
Copy link
Member Author

OK, but if we compare PeekPokeTest and ChiselTest: there is no development in PeekPokeTest, right? If we see any future in Chisel testing this should be (for now) ChiselTest, right?

How many users are out there using PeekPokeTest and would not migrate to ChiselTest because of performance issues?

@ducky64
Copy link
Member

ducky64 commented May 4, 2021

So for the performance it's mildly complicated:

  • Thread switching overhead when using fork is fundamenally unavoidable, at least given the lack of coroutine support in Scala and while maintaining the current API. See Improving threading performance #59 for discussions of alternative APIs - though they're not clean.
  • If you don't use fork, theoretically it's possible to have minimal performance impact - it shouldn't need to context switch every simulated time step. The scheduler currently does unnecessary context switching, but I do want to conduct a quick experiment to optimize that out and see how well that practically addresses performance issues. Perhaps combined with a sleep (eg, wait-for-signal-high or -low API), and perhaps combined with a fix for Tests hang: possible race condition #285.

@ducky64
Copy link
Member

ducky64 commented May 4, 2021

But overall usage-wise, my guess would be that it's more people don't want to rewrite code if they don't absolutely have to. We have for years (and still do) maintain a compatibility layer in base Chisel to accommodate rocket-chip.

@schoeberl
Copy link
Member Author

How much PeekPokeTest or ChiselTest is used in rocket-chip? Again to my original question: is PeekPokeTest just a dead end and any future design should use ChiselTest, or not? That is what I would like to describe in my next version of the book. A document for new designs (with minimal legacy) ;-)

@ducky64
Copy link
Member

ducky64 commented May 4, 2021

Does rocket-chip use either? I think rocket-chip predates both testing frameworks?

Another complication: there's also the super-bare-basic testing framework in the chisel3 unit tests. Those shouldn't be externally used (but they probably are somewhere, somehow). Those are (mostly) pure RTL semantics, as opposed to the imperative model in PeekPokeTester and ChiselTest.

@schoeberl
Copy link
Member Author

How should internal Chisel3 unit tests influence the testing strategies of (external) Chisel designs? Those should be just local unit tests, right?

@ekiwi ekiwi closed this as completed Nov 2, 2021
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

3 participants