Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Scala type peek poke #480

Merged
merged 13 commits into from
Jan 19, 2022
Merged

Scala type peek poke #480

merged 13 commits into from
Jan 19, 2022

Conversation

ekiwi
Copy link
Collaborator

@ekiwi ekiwi commented Jan 10, 2022 •
edited
Loading

  • clean up peek/poke/expect classes:
    • remove unused parameter,
    • use lazy eval for expect message (similar to scala assert)
    • clean up IntelliJ warnings
  • provide scala type peek/poke/expect access for:
    • Bool: Boolean, BigInt (as input only!)
    • UInt: BigInt
    • SInt: BigInt
    • Interval: BigDecimal, Double
    • FixedPoint: BigDecimal, Double
  • check input size (since we loosen the type restrictions, we are adding a runtime check to ensure values fit into type)
    • Bool, SInt, UInt

Note: for Interval and FixedPoint I decided to just make sure that we get all the features from the old PeekPokeTester API.

@ekiwi ekiwi force-pushed the scala-type-peek-poke branch 3 times, most recently from ac2ecc0 to ee76a7d Compare January 10, 2022 23:20
This was referenced Jan 10, 2022
Closed
Closed
@ekiwi ekiwi force-pushed the scala-type-peek-poke branch 2 times, most recently from 36adedf to 21512ef Compare January 17, 2022 19:18
@ekiwi
wip: Interval peek/poke
35fa513 
missing: respect epsilon for expect
@ekiwi ekiwi marked this pull request as ready for review January 18, 2022 22:46
@ekiwi
Copy link
Collaborator Author

ekiwi commented Jan 18, 2022

@carlosedp and @schoeberl, I would appreciate your feedback. Would this PR solve your issues wrt. using native Scala types to interact with the DUT?

@carlosedp
Copy link
Contributor

Pretty amazing @ekiwi! I just think we could have the peekBigInt as peekInt like Jack suggested in my PR.

Other than that, the ability to use native and Chisel types for poke and peek/expect is great!
Thanks a lot.

@ekiwi
Copy link
Collaborator Author

ekiwi commented Jan 19, 2022

I just think we could have the peekBigInt as peekInt like Jack suggested in my PR.

Changed it.

Thanks!

@schoeberl
Copy link
Member

+1 Very good! Looking forward to having this available. Sad that it didn't make it for 3.5.0

@ekiwi
Copy link
Collaborator Author

ekiwi commented Jan 19, 2022

+1 Very good! Looking forward to having this available. Sad that it didn't make it for 3.5.0

We can try to backport it.

@ekiwi ekiwi added this to the 0.5.x milestone Jan 19, 2022
@ekiwi ekiwi requested a review from jackkoenig January 19, 2022 20:55
@ekiwi ekiwi added the Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. label Jan 19, 2022
@jackkoenig
Copy link
Collaborator

We can have this in 3.5.1/0.5.1!

@mergify mergify bot merged commit 7a21ed6 into ucb-bar:master Jan 19, 2022
mergify bot pushed a commit that referenced this pull request Jan 19, 2022
@ekiwi
Scala type peek poke (#480)
94ec0fc 
* peek/poke: lazy message + remove un-used "stale" arg

* package: clean up

* add new private Utils object

* rename private expect to "expectInternal" in order to fix faul locator

* provide scala type access to UInt, SInt and Bool

* add peek/poke/expect tests, make sure values fit

* wip: Interval peek/poke

missing: respect epsilon for expect

* test: add fixed point tests for chiseltest

* provide FixedPoint methods

* internal: implement expect check outside of backend

* fix some tests

* peekBigInt -> peekIn

* add trailing new lines

(cherry picked from commit 7a21ed6)
@mergify mergify bot mentioned this pull request Jan 19, 2022
Merged
@mergify mergify bot added the Backported This PR has been backported to marked stable branch label Jan 19, 2022
@carlosedp
Copy link
Contributor

Awesome! Thanks all :)

@schoeberl schoeberl mentioned this pull request Jan 19, 2022
Closed
@schoeberl
Copy link
Member

I do not really understand what backporting means. I thought this means that it will show up in the future 0.4.X version. It will not change 0.5.0, right?

On 0.5.1 and book/teaching timing. I can add this into the book now with the master SNAPSHOT. However, my class starts on February 3rd, and then all code and examples should be in place. Will 0.5.1 be out until then?

@ekiwi
Copy link
Collaborator Author

ekiwi commented Jan 19, 2022

I do not really understand what backporting means

In this case it will eventually be part of 0.5.1

On 0.5.1 and book/teaching timing.

I would not count on that. @jackkoenig knows more though.

ekiwi added a commit that referenced this pull request Jan 19, 2022
@ekiwi
Scala type peek poke (#480)
2569402 
* peek/poke: lazy message + remove un-used "stale" arg

* package: clean up

* add new private Utils object

* rename private expect to "expectInternal" in order to fix faul locator

* provide scala type access to UInt, SInt and Bool

* add peek/poke/expect tests, make sure values fit

* wip: Interval peek/poke

missing: respect epsilon for expect

* test: add fixed point tests for chiseltest

* provide FixedPoint methods

* internal: implement expect check outside of backend

* fix some tests

* peekBigInt -> peekIn

* add trailing new lines

(cherry picked from commit 7a21ed6)
mergify bot added a commit that referenced this pull request Jan 19, 2022
@mergify @ekiwi
Scala type peek poke (#480) (#495)
560599e 
* peek/poke: lazy message + remove un-used "stale" arg

* package: clean up

* add new private Utils object

* rename private expect to "expectInternal" in order to fix faul locator

* provide scala type access to UInt, SInt and Bool

* add peek/poke/expect tests, make sure values fit

* wip: Interval peek/poke

missing: respect epsilon for expect

* test: add fixed point tests for chiseltest

* provide FixedPoint methods

* internal: implement expect check outside of backend

* fix some tests

* peekBigInt -> peekIn

* add trailing new lines

(cherry picked from commit 7a21ed6)

Co-authored-by: Kevin Laeufer <laeufer@cs.berkeley.edu>
@carlosedp
Copy link
Contributor

I've migrated all tests in my core to the new API using Scala native values and it all worked perfectly.

Commit: carlosedp/chiselv@221cab2

@carlosedp carlosedp mentioned this pull request Mar 21, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jackkoenig jackkoenig Awaiting requested review from jackkoenig

Assignees
No one assigned
Labels
Backported This PR has been backported to marked stable branch Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Milestone
0.5.x
Development

Successfully merging this pull request may close these issues.
4 participants
@ekiwi @carlosedp @schoeberl @jackkoenig