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

Async API #1724

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Async API #1724

wants to merge 4 commits into from

Conversation

sequencer
Copy link
Member

Contributor Checklist

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

Type of Improvement

  • new feature/API

API Impact

new API addition in chisel3.util.experimental.crossing

Desired Merge Strategy

  • Rebase: You will rebase the PR onto master and it will be merged with a merge commit.

Release Notes

Add Async API

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (3.2.x, 3.3.x, 3.4.0, 3.5.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

@ekiwi
Copy link
Contributor

ekiwi commented Jan 12, 2021

This is really cool! I recently wanted an AsyncQueue for one of my designs.

Is this the exact implementation used in RocketChip?

Are there any unittests you could copy?

@ekiwi
Copy link
Contributor

ekiwi commented Jan 12, 2021

@sequencer
Copy link
Member Author

This one is a rewrite based on my own understanding to rocket-chip AsyncQueue architecture.
I think the original micro-arch is from:
http://www.sunburst-design.com/papers/CummingsSNUG2002SJ_FIFO1.pdf
http://www.sunburst-design.com/papers/CummingsSNUG2002SJ_FIFO2.pdf

@sequencer
Copy link
Member Author

I think before we land a CDC and SVA in FIRRTL, I have no idea on how to test this(and so did in RC).
I think the road map can be.
--this PR--

  1. Adding full architecture documentation.
  2. review to architecture.

--next PR--

  1. FIRRTL clock-domain analysis API
  2. Chisel clock-domain annotate API
  3. auto SDC generation in FIRRTL

--next PR--

  1. SVA emitting in SV backend.
  2. add SVA codes to AsyncQueue.
  3. user can use their favorite commercial tool(VCS) to verify this AsyncQueue.

--future PR--

  1. SMT+SVA checking.

@ekiwi
Copy link
Contributor

ekiwi commented Jan 12, 2021

I think before we land a CDC and SVA in FIRRTL, I have no idea on how to test this(and so did in RC).

Chiseltest seems to have multi-clock support. So while you cannot exhaustively verify it at the moment, you could try to write some concrete unittests that ensure basic functionality.

@sequencer
Copy link
Member Author

you could try to write some concrete unittests that ensure basic functionality.

Yes I'm going to do it this week to do some basic functionality verification.

Chiseltest seems to have multi-clock support.

IIRC, ChiselTest doesn't support delta-cycle simulation, need confirm from @ducky64

@ekiwi
Copy link
Contributor

ekiwi commented Jan 12, 2021

As an idea on how to verify the AsyncQueue: You could try to add an assertion that at most on bit in the gray code encoded counter changes at a time.

@ducky64
Copy link
Contributor

ducky64 commented Jan 12, 2021

What do you mean precisely by delta-cycle simulation? ChiselTest propagates combinational operations immediately (well, actually on read), but there isn't a dedicated concept of a delta cycle.

Multiclock on ChiselTest is limited, but things might work if everything is derived (integer divisor) of a main clock.

@sequencer sequencer force-pushed the async_queue branch 2 times, most recently from 806e4ac to 2ea8b23 Compare January 13, 2021 08:49
@jiegec
Copy link

jiegec commented Mar 8, 2022

Any progress? I am currently copying the whole rocket chip utils package to my own project, but I hope this can be builtin.

@sequencer
Copy link
Member Author

sequencer commented Mar 8, 2022

I'd like to continue working on this.
The main issue is how to verify this?
Maybe @ekiwi can provide some idea, will his formal infra support multi clock domain?

@ekiwi
Copy link
Contributor

ekiwi commented Mar 8, 2022

Maybe @ekiwi can provide some idea, will his formal infra support multi clock domain?

Sorry, but I am currently busy and to get the multi-clock support to a usable state will take 2-3 weeks of full focus.
ucb-bar/chiseltest#482

jackkoenig pushed a commit that referenced this pull request Feb 28, 2023
* Define 'same clock' in a syntactic sense

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

None yet

4 participants