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

Working on porting bootcamp to chiseltest #121

Closed
wants to merge 14 commits into from
Closed

Conversation

chick
Copy link
Contributor

@chick chick commented Oct 27, 2020

IN PROGRESS CONVERSION
Questionable import of scalatest in load-ivy.sc
but it seems to work at moment.
Worksheets done: 2.1, 2.2, 2.6

Questionable import of scalatest in load-ivy.sc
but it seems to work at moment.
Worksheets done: 2.1, 2.2, 2.6
2.3 and 2.4
@jackkoenig jackkoenig requested a review from ducky64 November 2, 2020 20:03
Copy link
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions inline in the comments, but looks good as a direct translation so far!

It might be useful to clear the cells before committing to GitHub, the output cells are bloating the diff.

"SUCCESS!!\n"
]
}
],
"source": [
"// Scala Code: Calling Driver to instantiate Passthrough + PeekPokeTester and execute the test.\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment can go away now because of test magic

"If all `expect` statements are true, then our boilerplate code will return pass.\n",
"\n",
">Note that the `poke` and `expect` use chisel hardware literal notation. Both operations expect literals of the correct type.\n",
"If `poke`ing a `UInt()` you must supply a `UInt` literal (example: `c.io.in.poke(10.U)`, likewise if the input is a `Bool()` the `poke` would expect either `true.B` or `false.B`.\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also be useful to provide a negative example for each, eg for UInt, "this is distinct from a Scala number, eg 10, which will not work and type-error"

" c.io.in.poke(1.U) // Set our input to value 1\n",
" c.io.out.expect(1.U) // Assert that the output correctly has 1\n",
" c.io.in.poke(2.U) // Set our input to value 2\n",
" c.io.out.expect(2.U) // Assert that the output correctly has 2\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an example anywhere of an expect that fails? Because the current system doesn't return a test success / failure value, it might be useful to demonstrate what happens (exception), and maybe also have a side note on integration w/ ScalaTest

" for (i <- 0 until cycles) {\n",
" val in_a = Random.nextInt(16)\n",
" val in_b = Random.nextInt(16)\n",
" c.io.in_a.poke(in_a.U)\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a good candidate for abstracting this poke-and-expect operation into a test helper function.

" import scala.util.Random\n",
" val data = Random.nextInt(65536)\n",
" poke(c.io.fifo_data, data)\n",
" c.io.fifo_data.poke(data.U)\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly use Decoupled operations here, eg .enqueue and friends?

" poke(c.io.in_b, in_b)\n",
" poke(c.io.in_c, in_c)\n",
" expect(c.io.out, in_a*in_b+in_c)\n",
" c.io.in_a.poke(in_a.U)\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially a place to introduce a test helper function

@chick
Copy link
Contributor Author

chick commented Dec 1, 2020

Closing, this PR has been replaced by #122, #123, #124

@chick chick closed this Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants