support enqueue/dequeue of IrrevocableIO #90
Conversation
Signed-off-by: shuyun <shuyunjia@outlook.com>
There was a problem hiding this comment.
This looks mostly fine. Some questions / potential improvement suggestions inline.
Some licensing stuff might need to be figured out before merging, either getting a formal CLA in place or perhaps just your agreement to dual-license your contribution under Apache 2 and BSD 3-clause.
| @@ -173,5 +173,7 @@ package object tester { | |||
|
|
|||
| implicit def decoupledToDriver[T <: Data](x: DecoupledIO[T]) = new DecoupledDriver(x) | |||
|
|
|||
| implicit def irrevocableToDriver[T <: Data](x: IrrevocableIO[T]) = new DecoupledDriver(x) | |||
There was a problem hiding this comment.
Will an implicit conversion from ReadyValidIIO instead of DecoupledIO work instead of requiring an implicit conversion for each individually?
There was a problem hiding this comment.
oh yes, it is much better to convert from ReadyValidIO.
Fixed in 49d2857.
| when(io.in.fire()){ full := true.B }.elsewhen(io.out.fire()){ full := false.B } | ||
| io.in.ready := !full | ||
| io.out.valid := full | ||
| io.out.bits := RegNext(io.in.bits) |
There was a problem hiding this comment.
Would it make more sense to just instantiate a queue here, instead of essentially rolling your own one-element queue which doesn't appear to be fully correct (the data bits updates every cycle, regardless of io.in.ready)?
There was a problem hiding this comment.
The original purpose of instantiate a different queue is to distinguish between DecoupledIO and IrrevocableIO. Since there are additional requirements on signaling of valid and ready. I looked into the queue object and it satisfy the requirement.
Fixed in 49d2857
| c.io.in.enqueue(9.U) | ||
| c.io.out.expectDequeue((9.U)) | ||
| parallel( | ||
| c.io.in.enqueueSeq(Seq(5.U, 2.U)), |
There was a problem hiding this comment.
Would it make sense to add the 9.U above to the enqueueSeq? Or is the above meant to specifically test cycle-by-cycle behavior?
There was a problem hiding this comment.
It is just some redundant cycly-by-cycle test. Removed in 49d2857
Signed-off-by: shuyun <shuyunjia@outlook.com>
|
And, I agree to dual-license my contribution under Apache 2 and BSD 3-clause. |
* support enqueue/dequeue of IrrevocableIO Signed-off-by: shuyun <shuyunjia@outlook.com> * fix per commit Signed-off-by: shuyun <shuyunjia@outlook.com> (cherry picked from commit 6767faa)
* Implement ChiselUtestTester for utest (#76) (cherry picked from commit da4f226) * Use latest utest; fix issue utest#211 (#77) * Use latest utest; fix issue utest#211 * Fix compatibility problem for 2.11 and 2.12 (cherry picked from commit 652a248) * Add valid io driver (#63) * Adds an ValidDriver interface similar to Decoupled * Cleanup imports in ValidQueueTest * Adds an ValidDriver interface similar to Decoupled Adds tests for this construct * Round 2 of ValidDriver support - change name to validSourceKey - remove enqueue, only supports enqueueNow - change how enqueueNow works internally - Simplify testing of Valid - make DUT have delay and typeGen - eliminate ValidQueue, just use Pipe instead - add example that shows how to enqueue multiple bundles at the same cycle * Valid interface updates - Add comment on valid's expectPeek saying it does not advance time - got rid of errant comment mentioning stall - used dequeueSeq for pass through elements test - renamed test with expectInvalid and added loops - added parens to getSourceClock calls * - removed wrongly changed entry in .gitignore - removed duplicated code in ValidQueueTest (cherry picked from commit 72cb844) * Fix toolchain so treadle supports boringutils (#79) * Fix toolchain so treadle supports boringutils - Add the low firrtl compiler to TreadleExecutive - Send hint to treadle so it knows compiled form - Fix backward compatibility with Driver - Check compiler type to help figure out the backend - Backward compatibility only supported for treadle and verilator - Add boringutils test - Backward compatibilty requires compile workaround - Can we just eliminate Driver compatibility mode here??? - Fixed CopyVpiFiles - looks like this might fail if early file was there but a late one wasn't - this code came from chisel-testers - Fix top level test names - behavior of "Testers2 with Vcs" - behavior of "Testers2 with Verilator" - Shortened VCS to just see if a simple circuit will build and simulate. - in long run we still need mechanism to run an arbitrary test with arbitrary backend * - Made associations between compiler and backend in compat mode explicit - In BoreTest - removed unused code. - In MixedVec - Renamed to VecPokeExpectTest - tested all addresses - removed bundle (cherry picked from commit dd6d083) * Fix external links in README (#84) (cherry picked from commit b6ea7e9) * Merge pull request #85 from ucb-bar/peekBundle Support Bundle literal peek (cherry picked from commit ea5e81f) * support enqueue/dequeue of IrrevocableIO (#90) * support enqueue/dequeue of IrrevocableIO Signed-off-by: shuyun <shuyunjia@outlook.com> * fix per commit Signed-off-by: shuyun <shuyunjia@outlook.com> (cherry picked from commit 6767faa) * Fix missing Scaladoc parenthesis (#92) Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com> (cherry picked from commit 7e4ef7f) * Fix README links (#109) (cherry picked from commit 2b6cae0) * Re-add -DwriteVcd=1 feature (#110) (cherry picked from commit 2f1cbae) * gitignore: add Visual Studio Code folders (#100) (cherry picked from commit d9d2a7a) * Bump minor version Co-authored-by: Sequencer <liujiuyang1994@gmail.com> Co-authored-by: Chick Markley <chick@qrhino.com> Co-authored-by: edwardcwang <edwardcwang@users.noreply.github.com> Co-authored-by: Richard Lin <elpato25@gmail.com> Co-authored-by: ShuyunJia <shuyunjia@outlook.com> Co-authored-by: Schuyler Eldridge <schuyler.eldridge@gmail.com> Co-authored-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Signed-off-by: shuyun shuyunjia@outlook.com
issue related: #89
This commit is to: