chisel icon indicating copy to clipboard operation
chisel copied to clipboard

ChiselSim peek/poke/expect for Enums, Records, Vecs, and generic Data

Open kammoh opened this issue 7 months ago • 3 comments

This PR changes the "testable" implicit classes in PeekPokeAPI to allow for a unified peek(), poke(), and expect API for all data types, including Records, Vecs, Enums, and nested aggregates. This unified API would be very helpful when developing higher-level test abstractions, such as enqueue/deque over clocked Decoupled interfaces.

This is a step towards adding some useful features and ergonomic APIs similar to those provided by chiseltest (#4209 included a rough PoC for integrating some of these features).

The code does not look very pretty, but I could not figure out a cleaner way to make it work. Also, there is some repeated code.

I'd greatly appreciate comments and ideas for an alternative approach or other improvements.

Note: The previous (existing) implementation of expect[T]() seems incorrect. In addition to mistakingly using the same type name, T for the different method type parameter, the comparison uses object equality (!=).

def expect[T](
      expected:     T,
      encode:       (Simulation.Value) => T,
      buildMessage: (T, T) => String,
      sourceInfo:   SourceInfo
    ): Unit = {
...
        if (observed != expected) {

For the implemented Element values, the observed value is tuned into a Chisel object using the encode() method. Since the objects are not the same, the comparison would always fail, even when the value is expected. Seems the previous tests did not cover this, but using the tests in this PR, I get false failures with the previous expect + encode:

Observed value: 'UInt<33>(3521158774)'
Expected value: 'UInt<33>(3521158774)'

Contributor Checklist

  • [x] Did you add Scaladoc to every public function/method?
  • [x] Did you add at least one test demonstrating the PR?
  • [x] Did you delete any extraneous printlns/debugging code?
  • [x] Did you specify the type of improvement?
  • [ ] Did you add appropriate documentation in docs/src?
  • [x] Did you request a desired merge strategy?
  • [ ] Did you add text to be included in the Release Notes for this change?

Type of Improvement

Feature

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).

Release Notes

Reviewer Checklist (only modified by reviewer)

  • [ ] Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • [ ] Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
  • [ ] Did you review?
  • [ ] Did you check whether all relevant Contributor checkboxes have been checked?
  • [ ] Did you do one of the following when ready to merge:
    • [ ] Squash: You/ the contributor Enable auto-merge (squash) and clean up the commit message.
    • [ ] Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

kammoh avatar Mar 23 '25 21:03 kammoh