[Proposal] Add optional timeout to (en|de)queue

[nhynes]

Setting a maximum number of cycles would prevent infinite loops when testing buggy circuits.

[ducky64]

I actually think it might make more sense to have a default global timeout with some kind of idle detection (like no inputs changed for some number of cycles - maybe 1000? or 10,000?). The idea is to avoid whack-a-mole debugging (by spraying timeouts in en|dequeue operations, and then leaving them in there like stray debugging prints) and avoiding needing to both pass through a timeout to every single blocking operation and needing library functions to handle a timeout parameter. Thoughts?

The default timeout cycle count would need to be change-able through some kind of as-to-be-determined option system (are there tests which would have >1000 idle cycles?), and it's also conceivable that some library functions may cause livelock (so potential gotcha for library developers and users - mainly developers). So not foolproof, but would allow infrastructure to handle what would otherwise be a lot of boilerplate.

[nhynes]

more sense to have a default global timeout

That was my initial thought, but I wondered whether some users might have real-time constraints and it seems that queueing is the only place where the test might be unbounded. In the most common scenario, though, you're right that the overall test would be less maintainable. Maybe the correct proposal would be to have a global (per-clock) timeout and then possibly add a few lines of code to handle enq/deq timeouts.

For the implementation, it'll probably be simplest to do something like

implicit class testableClock(x: Clock) {
  private var maxCycles = 1000
  private var elapsedCycles = 0

  def setMaxCycles(cycles: Int) = maxCycles = cycles
  
  def step(cycles: Int = 1): Unit = {
    elapsedCycles += cycles
    if(elapsedCycles > maxCycles) {
      Context().env.testerFail(s"Max cycles exceeded for clock: $x")
    }
    Context().backend.step(x, cycles)
  }
}

and, for the queue timeouts

def enqueue(data: T, timeout: Option[Int] = None): Unit = timescope {
  x.valid.poke(true.B)
  var steps = 0
  while (x.ready.peek().litToBoolean == false && timeout.map(steps < _).getOrElse(true)) {
    clk.step(1)
    steps += 1
  }
  x.bits.poke(data)
  clk.step(1)
}

The local timeouts aren't so important because a motivated user could just re-roll *queue directly.

[ducky64]

I'm not sure implicit conversions will work, since wouldn't a new object be created at each conversion point and internal state (like elapsed and max cycles) lost?

I think timeout by idle cycles makes more sense than a simple max cycles, since I could see a case for long(ish) tests, but less of a case for long periods where the testdriver doesn't write into the circuit.

I do like the idea of calling setMaxCycles on the clock object itself, this is probably cleaner than adding a slew of flags at testdriver invocation. It might also make sense for the max cycles to be mutable, since there may be segments of test where things are expected to take longer. Though this also brings up, does it make sense for timeout cycles to be scoped?

As for what cause a potentially unbounded test, I could see anything in the general class of waiting for event. For some designs (especially system-level designs?) though, this may be primarily Decoupled-based.

What do you mean by real-time constraints?

[nhynes]

timeout by idle cycles

I like that better actually. I'm not quite seeing anything in treadle that would expose whether the circuit state changed, but I also didn't look too deeply.

real-time constraints

Like if I have a specification that states that output.valid is asserted x cycles after input.valid. The Xilinx DMA IP Spec Table 2-2 is an example.

#11 shows an example of how a simple global per-clock limit would work.

[ducky64]

I think a timeout would be a different mechanism than LTL constraints (which I think are the basis for some SVA properties). LTL would seem to be the right mechanism for expressing something like "make sure X happens Y time after Z", and generally allows constraints to apply across time. I think @jackkoenig worked on LTL-style assertions in base Chisel with CACL, though I'm not sure about the current status of that project. It might also be possible to implement LTL-style assertions with threads in testers2, though those would need to be written into every single test as opposed to being a circuit property associated with the RTL.

My thoughts on timeouts vs. LTL constraints are that LTL constraints encode correctness properties, whereas timeouts are supposed to prevent the need for the user to kill sbt because the test deadlocks. One might fit some of the needs of the other, but I don't think the use cases map over generally.

As for idle cycles, I'm thinking more of testdriver inputs. I don't think it makes sense to reset the timeout if internal registers change, since the presence of internal state machines would be kind of separate from whether the testbench is prone to deadlock. But testers2 could notice whenever a new value has been poked into a wire, and use that to reset the timeout.

The main questions there would be whether repeated pokes of the same value trigger the timeout reset, and if it looks at all pokes or ignores intermediate pokes between register updates (clock cycles) or within timesteps.