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

Support RawModule / multi-clock modules #14

Open
edwardcwang opened this issue Jan 29, 2019 · 5 comments
Open

Support RawModule / multi-clock modules #14

edwardcwang opened this issue Jan 29, 2019 · 5 comments
Labels
enhancement New feature or request feature request

Comments

@edwardcwang
Copy link
Member

edwardcwang commented Jan 29, 2019

Example of multi-clock circuits: https://github.com/edwardcwang/chisel-multiclock-demo

It would make testing these kinds of circuits more DRY and less tedious than manually creating a wrapper Module.

Ideas

  • testers1 semantics (poke-clock and wait-on-clock semantics)
  • clock wrapper generator
@edwardcwang edwardcwang added the enhancement New feature or request label Jan 29, 2019
@ducky64
Copy link
Member

ducky64 commented Jan 30, 2019

I have a few ideas, but let's recap the current testers2 architecture:

  • Testers2 currently only works on modules with an implicit clock. This clock is driven by the testdriver, with the smallest time step being one cycle of the implicit clock.
  • Internal (derived in RTL) clocks are handled by the simulator, so you can do things like digital clock dividers which will work as expected. See ClockCrossingTest and ClockDividerTest (which also steps on a derived clock)
  • Clocks can also be poked, which will also work as expected since the details are handled by the simulator. See AsyncClockTest.
  • The clock.step(...) operation blocks until a rising edge is detected on the target clock, by polling once per time step (currently one cycle of the implicit clock). This works fine for derived clocks (eg digital clock dividers, since the implicit clock is an integer multiple), but not for poked clocks. This is the case in AsyncClockTest: the DUT clock is manipulated by pokes and is NOT the implicit clock, but the implicit clock is used to pass time (which should be equivalent to a no-op in this particular design).
    • We can (but currently don't) do sanity checks on this by disallowing stepping on Input clocks and Output clocks that are combinationally dependent on any input.

I think the current two ideas all have trade-offs and depends on use cases.

Clock Generator Wrapper

Turns out this might not be feasible given MultiIOModules, because I don't think it's possible to generate a Module that has the same top-level-IO as another Module (the one it wraps).

The idea would have been to encapsulate the non-implicit-clock-module DUT in an implicit-clock-module wrapper, pass through the non-clock IOs, and generate digital clock dividers for the clock IOs from the wrapper's implicit clock. The derived clocks could be specified simply as a map of the Clock object to the period multiple of the implicit clock.

More thought would be required about optionally specifying phase and default reset behavior.

This also only works when clocks can be regular and have static frequency relationships that don't change. So great if this reflects your real system, but could make writing tests for async circuits (eg testing various cases of AsyncQueue) more difficult. But those cases maybe also need non-ideal circuits where combinational logic takes nonzero time to propagate?

Clock Generator with Realtime Semantics

Largely the same proposal (from a user point of view) as above, except the clock generation is moved into the testdriver infrastructure. So instead of the current situation where testers2 infrastructure generates only the implicit clock, it can optionally take a map of Clock to Int / Float frequency and generate the rising and falling edges as needed. Similar limitations as above, it's just a different implementation of a similar API.

Improving Poke-on-Clock

As noted in the intro, in the current architecture poking clocks works as expected, though stepping (waiting for rising edge) on a clock poked in another thread will NOT work. This attempts to unify those.

Instead of (necessarily - still might be useful for optimization purposes though) having the implicit generated in infrastructure, clock generation could just be another thread that pokes clocks. There would be two ways that threads blocked on a step can be triggered:

  1. Poking a rising edge on a clock immediately pauses the current thread and runs all the threads blocking on that clock. In this case, it's not possible to have true simultaneous rising edges.
  2. Add a realtime construct with some way to advance realtime. A thread does not pause until it advances realtime (or waits on a clock edge, or joins on another thread).

In both cases, the default behavior would still be to provide a default clock for implicit-clocked-modules, but not for RawModule. In the realtime case, there would be some default realtime clock frequency for the implicit clock, though this mainly affects waveform dump since combinational logic is instantaneous. Mixing realtime and clock step semantics may also lead to poor style code.

In this model, I'd be concerned about the complexity and antipatterns possible with the power of an arbitrary event scheduler model - for example, what if you poke a clock rising edge from one thread, and then poke a clock rising edge from the triggered thread? Some of the cross-thread-interactions limitations could generalize here, particularly if the clocking thread is in its own thread (as opposed to the root thread - so it would have no child threads which can override the clock value).

One way proposal 2 could look would be:

test(new MyRawModule()) { c =>
  fork {
    while (True) {
      c.clock1.poke(false.B)
      c.clock2.poke(false.B)
      step(1.0)  // ns
      c.clock1.poke(true.B)
      step(1.0)  // ns, clock1's threads triggered here
      c.clock1.poke(false.B)
      c.clock2.poke(true.B)
      step(1.0)  // ns, clock2's threads triggered here
      c.clock1.poke(true.B)
      step(1.0)  // ns
    }
  }
  // test code here
}

or different clocks in different threads:

test(new MyRawModule()) { c =>
  fork {
    while (True) {
      c.clock1.poke(false.B)
      step(1.0)  // ns
      c.clock1.poke(true.B)
      step(1.0)  // ns, clock1's threads triggered here
    }
  }.fork {
    while (True) {
      c.clock2.poke(false.B)
      step(1.5)  // ns
      c.clock2.poke(true.B)
      step(1.5)  // ns, clock2's threads triggered here
    }
  }
  // test code here
}

in addition to the poking the clock in the main thread as in the AsyncClockTest.

@ducky64
Copy link
Member

ducky64 commented Feb 23, 2019

This was briefly discussed at the meeting today.

While there is a use for the fixed frequency case, it is limiting, so we mainly consider ways we could arbitrarily poke clocks.

Clock Dependence Linearization

Under this proposal, all pokes to a clock must happen in a thread that runs before any thread that is stepping on (blocked on) that clock. Because thread execution order largely follows lexical ordering (assuming function calls are flattened), this means that clock pokes must be lexically before any clock steps. Practically, I expect the most common pattern to be spawning a thread as the clock generator before any other testdriver action.

Other details:

  • Clock step / poke dependencies are checked at runtime, and if failing will throw an exception like thread ordering dependent operations.
  • Clock pokes count as combinational operations on signals downstream of that clock, (to be) obtained with FIRRTL clock analysis. So this may trigger cross-thread read-after-write (thread order dependent operation) exceptions. This (virtual) combinational path is broken if a thread is unblocked on that clock, so it is legit to have a thread poke a clock, and a sibling clock that steps on that clock operate on downstream signals.
    • It's unclear if this will limit the tests that can be written with multiclock designs. I'm going to take a "expose a limited and safe-ish API and we can expand that when use cases come up" approach.
  • Otherwise, clock pokes only subject to the usual poke ownership rules. The above are the main restrictions.
  • There would need to be a realtime semantics, so the testdriver thread can advance realtime. It is still generally recommended for test code to wait on clock edges using clock.step when possible - this should be closer to test intent.
  • The single-clock case (testing a Module with implicit clock) continues to provide a default clock generator without user intervention

Other Possibilities

Another option discussed is only being able to poke clocks in a special region that runs before the main testdriver region, but this is probably too restrictive.

@sequencer
Copy link
Collaborator

Hello, @ducky64, if I wanna implement a RawModule tester without wrapper, what's the best way?
I'm trying to port diplomatic module to tester2, But they are in RawModule.

@ducky64
Copy link
Member

ducky64 commented Feb 20, 2020

So the reason we need a full Module is for the clock and reset line, in particular the clock line which is currently treated as special. RawModule doesn't provide a clock, which breaks one of the fundamental abstractions of the tester infrastructure driving the clock.

The above are some possibilities about how we can make the Module clock not-special, by providing ways for user code to drive clocks. This gets complicated because clocks can trigger other logic. You're welcome to try to reason through the above and see if there's a good solution, but I imagine that this would be a lot of time building infrastructure.

Is it impossible to wrap a diplomatic module in a Module with the implicit clock and reset? It is a bit ugly, but we also really do need the clock...

@sequencer
Copy link
Collaborator

Yes, I'm writing a wrapper, by extraction all Edge paramters, reconstruct a Top MultiIOModule, yes, it's a liitle ugly but seems works now. After finishing this, I will give a try to port MultiIOModule to RawModule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request
Projects
None yet
Development

No branches or pull requests

4 participants