jlpteaching / dinocpu

A teaching-focused RISC-V CPU design used at UC Davis
BSD 3-Clause "New" or "Revised" License
143 stars 38 forks source link

Chisel Notes: First Hardware discrepancy #130

Open johnzl-777 opened 4 years ago

johnzl-777 commented 4 years ago

The current first-hardware.md guide assumes that simple.scala doesn't exist and that it can be created.

However, the file does exist and contains a number of major differences from the guide.

Furthermore, simple.scala contains the following test case code:

class SimpleSystemUnitTester(c: SimpleSystem) extends PeekPokeTester(c) {
  step(10)
}

object simple {
  def main(args: Array[String]): Unit = {
    Driver( () => new SimpleSystem ) { c => new SimpleSystemUnitTester(c) }
  }
}

I'd be willing to update the guide for this new situation, especially since I'm working towards adding MuxCase examples (you can check out the muxcase-documentation branch on my current progress).

However, I'm not quite certain what the best way is to approach such an update

powerjg commented 4 years ago

First of all, thanks, John!

I'm not sure I understand what you're asking, though. Do you need help understanding how to write Chisel tests or something else?

If your question is how to do testing in Chisel, I suggest checking out their documentation: https://www.chisel-lang.org/. However, there are two versions of testers. The first one, "Testers" is the old version (which is currently used in DINO CPU). However, they have a new way to test (ChiselTest/testers2) that looks like it's much better.

I would normally suggest using the new version. However, I'm not sure if the benefits are worth having inconsistency in the documentation or migrating everything.

johnzl-777 commented 4 years ago

Whoops! Apologies, I seem to have missed my main concern(s) in haste.

I'd like to add a MuxCase example to that first hardware page (I've already done it for the cheat sheet) but I guess I had a couple concerns with the existing state of "first hardware.md":

  1. Would it be desirable to keep its instructions saying that you can make simple.scala from scratch or should we alert users that simple.scala already exists and that they should pretend it doesn't (make a simple2.scala?)
  2. The simple.scala contents don't seem to line up with the full resulting code in "simple hardware". Should an update be made to make it so that the guide and code line up?
  3. The simple.scala also has a test case section that is not mentioned at all in the markdown file. Should that be added as well? Or somehow crammed into instTests?

All of these differences make it somewhat tricky for me to figure out how to add a nice MuxCase example.

Any clarification would be greatly appreciated!

powerjg commented 4 years ago

Whoops! Apologies, I seem to have missed my main concern(s) in haste.

I'd like to add a MuxCase example to that first hardware page (I've already done it for the cheat sheet) but I guess I had a couple concerns with the existing state of "first hardware.md":

  1. Would it be desirable to keep its instructions saying that you can make simple.scala from scratch or should we alert users that simple.scala already exists and that they should pretend it doesn't (make a simple2.scala?)

We should definitely alert users that it already exists in the dinocpu repo. They can either make another file (with new names), just look at the code in simple.scala and follow along, or delete simple.scala and start from scratch.

  1. The simple.scala contents don't seem to line up with the full resulting code in "simple hardware". Should an update be made to make it so that the guide and code line up?

Sure! TBH, I don't remember exactly what I was thinking at the time.

  1. The simple.scala also has a test case section that is not mentioned at all in the markdown file. Should that be added as well? Or somehow crammed into instTests?

I'm not sure exactly what you're referring to here. Could you provide links?

All of these differences make it somewhat tricky for me to figure out how to add a nice MuxCase example.

Any clarification would be greatly appreciated!

I think a separate MuxCase example (maybe building on top of simple) would be best. The key idea with the SimpleSystem is how your chisel code translates into hardware (to try to bridge the gap between logisim and chisel). Adding more things there could muddy this point. However, I think adding more examples (especially ones that look like what the CPU ends up looking like) are useful!

johnzl-777 commented 4 years ago

Looks like the test case code is accounted for in first-hardware.md! Nice.

Would the separate MuxCase example be a separate markdown file worth of instructions altogether that still cites the simple code according to your last statement?

If it was a separate markdown file I think it might be worth considering adding some other common constructs like those from the chisel cheat sheet from the Chisel project.

powerjg commented 4 years ago

Yeah, a separate markdown file sounds great! Adding other things is a wonderful idea as well.

We should chat sometime about credit for this... I only have so much extra credit I can give ;). Significantly improving the documentation this way is a huge contribution!