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

Chisel Notes: First Hardware discrepancy #130

Open
johnzl-777 opened this issue Apr 20, 2020 · 5 comments
Open

Chisel Notes: First Hardware discrepancy #130

johnzl-777 opened this issue Apr 20, 2020 · 5 comments
Milestone

Comments

@johnzl-777
Copy link
Contributor

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
Copy link
Contributor

powerjg commented Apr 20, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor

powerjg commented Apr 20, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor

powerjg commented Apr 21, 2020

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!

@powerjg powerjg added this to the ECS154 WQ '21 milestone Dec 9, 2020
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

2 participants