Improvements to dumpVCD

[DigitalBrains1]

We have a function dumpVCD that will produce a waveform of taps set through traceSignal. However, it currently has the first active clock edge right at the beginning, at t = 0 s, meaning you can't see the initial state of the signal. I'd like to propose an improvement to dumpVCD that will give more flexibility and also makes it possible to see the initial value if desired.

This is currently my best idea, but this is Discussions, let's see if we can make it even nicer and what the merits and the flaws are of my approach.

dumpVCD can deal with multiple clock domains in a single VCD file; however, this is currently broken: the output is simply wrong, all waveforms (or all waveforms but one) are offset and the offsets don't match, so it's really all over the place. My proposed solution works for both single domains and multiple domains without loss of generality. I will start with a single domain, but I will move on to multiple domains after that.

However, this discussion is purely about the intended current design and its replacement, I deliberately do not discuss obvious bugs in the current design any further; those can just go in Issues and would only muddy this discussion.

The design to generate VCD's as well as HDL simulation is the following:

VCD.hs:

module Main where

import Clash.Explicit.Prelude
import Clash.Explicit.Testbench
import System.Environment (getArgs)

import qualified Data.Text.IO as T
import qualified Prelude as P

topEntity ::
  Clock System ->
  Signal System (Unsigned 8)
topEntity clk =
  let o = delay clk enableGen 10 $ o + 1
   in traceSignal "o" o
{-# OPAQUE topEntity #-}

testBench ::
  Signal System (Unsigned 8)
testBench = topEntity clk
 where
  cnt = register clk noReset enableGen (0 :: Index 10) $ satSucc SatBound <$> cnt
  clk = tbClockGen $ (< maxBound) <$> cnt
{-# OPAQUE testBench #-}

main :: IO ()
main = do
  offset <- (read . P.head) <$> getArgs
  vcd <- dumpVCD (offset, 5) (topEntity clockGen) ["o"]
  either error (T.writeFile "sig.vcd") vcd

topEntity is a flip-flop without reset; register with a noReset argument would be equivalent to delay. The test bench is there to allow us to simulate in HDL as well for comparison, but we generate VCD's with main where the offset is a command line argument.

dumpVCD is invoked as follows: dumpVCD (offset, cycles) signal traceNames. signal is the signal to simulate to hopefully get the traceSignals evaluated; the output value of the signal isn't in the VCD, it's just computed to get GHC to evaluate all the traceSignals we're interested in. offset then is at which sample of the signal to start outputting data to the VCD; it is expressed in the period of the signal sampled. cycles is for how many cycles we want to simulate. traceNames is a technicality; it should specify all the traces we're interested in, so we keep simulating until we have all the named traces, which might not "show up" immediately due to lazy evaluation.

If we use Clash.Explicit's sampleN on this entity we get:

clashi> E.sampleN 10 $ topEntity clockGen
[10,11,12,13,14,15,16,17,18,19]

Let's see what Modelsim makes of this:

Explanation: the initial value of the flip-flop is 10. For HDL simulation, we start all clocks at a particular time well past 0. That time is the moment that one period of the slowest clock has passed; but it has a minimum of 100 ns. This construction is there for timing analysis: if you simulate a post-timing netlist, this will respect setup time even for the slowest clock if you assume the signals become stable at t = 0 s. And the minimum of 100 ns is discussed in issue #2455. Because the 100 ns is probably more than the periods of the clocks, in practice, it's a nicely stable value, making mental arithmetic about clock cycle numbers easier.

If we compare the Modelsim output to sampleN, we see that sampleN starts with the initial value as well.

Now let's make a VCD file, with offset = 0 (main is invoked as ./VCD 0). This is equivalent to:

dumpVCD (0, 5) (topEntity ...

Let's look at it in Surfer:

Huh... Now what is happening here? Time starts at 0 ps, but at that time, the signal immediately transitions to 11. We said we wanted offset 0 and 5 cycles, but if you asked me how many cycles there are my first answer would be 3. What is probably happening with that number of 5 cycles is this: initial value is first one, cycle with value 14 is the fifth, which is why we see the signal changing at the end: dumpVCD has rendered the 14, and in the waveform it is changing into 14, but we stop the time as well and never get a chance to properly see. I did not verify this analysis of the number 5 here, but I am certain what happens with offset 0, which is the crux of this discussion.

Our dumpVCD decided that time starts with the clock, and that at time t = 0 s there should be the first active clock edge. In VCD, negative time does not exist (the VCD standard is intimately coupled to the Verilog simulation model). Initial values in a VCD file are associated with a time stamp; my interpretation is that this also sets the start time of the simulation. So what we output into the VCD file are these two facts: the initial value of the signal at time t = 0 s is 10. At time t = 0 s, this value changes into 11. Frankly, I'm not even sure this is a valid VCD file, with this repeating time stamp. But both GTKWave and Surfer are fine with it, so maybe that point is academical.

The VCD file does specify the initial value of 10. But it specifies that at the exact same time, it changes to 11.

This difference with HDL simulation and with the output of, e.g., sampleN, makes the already non-trivial task of interpreting the waveform correctly really confusing.

Now let's try with a non-zero offset. Let's invoke dumpVCD (1,5) .... and look at it in Surfer:

Now time starts even later! 0 ps is at the second active clock edge! Apparently time starts when we start with the values we're interested in. Note that it is fine to start time in a VCD at a non-zero time, this is purely a choice, not a necessity. If asked what I would think it would look like based on my invocation, I would have expected this:

(this is a hand-edited VCD file, not output from dumpVCD)

Also, apparently the 5 is relative to the initial value, not to offset. It basically specifies an interval of the form (start, end), but from the documentation (number of samples), I would have expected 5 samples in the output, not an end at cycle 5.

When thinking about how to fix this, I think there is a very important point to consider. I really don't think we can go around changing the output without somehow calling this out very explicitly to the user! We don't draw the clock itself in the waveform, so the only point of reference a user who uses dumpVCD has is either the time or are the values of the signal itself, but they might not actually know those things already: that's why they're looking at the waveform! If a clock had been drawn, they'd be able to use that as a reference (much as the system itself does :-).

So we want two things:

1. Move the start of the clock ticks away from t = 0 (obviously in the positive direction)
2. Somehow get the user to notice we changed stuff

My proposal is this:

We change the function

dumpVCD
  :: NFDataX a
  => (Int, Int)
  -- ^ (offset, number of samples)
  -> Signal dom a
  -- ^ (One of) the outputs of the circuit containing the traces
  -> [String]
  -- ^ The names of the traces you definitely want to be dumped in the VCD file
  -> IO (Either String Text.Text)
dumpVCD (offset, cycles) signal traceNames = ...

to

dumpVCD
  :: NFDataX a
  => Int
  -- ^ Clock delay (in ps)
  -> Int
  -- ^ Start of sampling (in cycles)
  -> Int
  -- ^ Number of samples to output
  -> Signal dom a
  -- ^ (One of) the outputs of the circuit containing the traces
  -> [String]
  -- ^ The names of the traces you definitely want to be dumped in the VCD file
  -> IO (Either String Text.Text)
dumpVCD delay start cycles signal traceNames = ...

I make the clock delay explicit. A user has to specify the delay from 0. We don't have to guess what would be best for the user; they know that. Since the design might very well have multiple domains, it is specified as physical time, not a number of cycles, which would be a meaningless quantity in the other clock domains that are also rendered.

Because I changed the API for dumpVCD, old code with dumpVCD no longer type checks. When their compilation errors out, a user is confronted with an incorrect use of dumpVCD and will need to consult the documentation. There, they find they are now supposed to specify when the clock starts. We didn't just change the API, we also force them to think about a value. It's not just a mechanistic "oh the arguments have slightly changed", very cursory glance over the doc, and change the invocation to match. I think many developers when faced with an API change don't actually focus much on the new documentation; they're just trying to get their old code to compile and will just give the arguments a quick glance to see what they need to pass in. In other words, I think it's important to force them to think first, lest they go too quickly.

As a minor point, I want to interpret the cycles signal as a number of samples to put in the VCD, so I basically curry the argument with offset and number of cycles as they seem pretty unrelated now. When they were a left and right edge, I'd see that as a pair of numbers, but an offset and a number of cycles, while still both relative quantities with the same unit, feel much more distinct. But I'm also fine with keeping the tuple as it was if others think this is more intuitive or useful.

And when I say I want 5 cycles, I want to see 5 numbers spaced at the clock period.

As a convenience, we could add another function, maybe dumpVCD1, that has an implicit value for the delay. Either the period of the domain of the sampled signal or a fixed 100 ns would work for me. I prefer the latter, as it means we end up usually looking the same as HDL simulation, unless the design contains a clock domain with a period less than 10 MHz.

And I want the time in the VCD file to reflect simulation time of the design, so t = delay ps is always when the clocks start a-ticking. So we don't move the time reference when we change offset. Hence, offset isn't a great name anymore either, and I rename it to start and adjust the documentation.

With that, we'd get the following:

dumpVCD 100_000 0 5 (topEntity clockGen) ["o"]

(or

dumpVCD1 0 5 (topEntity clockGen) ["o"]

) gives us:

dumpVCD1 1 5 (topEntity clockGen) ["o"]

gives us:

We dropped the initial value from consideration because we specified an offset of 1. Continue on like this for higher values.

Let's throw in another clock domain for good measure:

Hmmm.... I'm not completely happy with Surfer drawing a transition for o2 at the right edge at 150 ns, because I did not specify a transition in the VCD file. The next active clock edge would be at ~153 ns and is not in the VCD file. For comparison, this is GTKWave, scrolled to the right as far as it will go:

It doesn't draw a transition for o1 either, but we can interpret this as that time has stopped just before the transition of o1. Whereas with Surfer, I feel like o2 does have a transition, which it does not.

So, let's discuss. I hope I managed to properly convey the current situation with all its intricacies!

[DigitalBrains1]

Ah. Note that the HDL behaviour of delaying by the period of the slowest clock in the design can't be transferred perfectly to dumpVCD. It only knows the domains of taps that end up in the trace through traceSignal, but there might be other clock domains in the design that are slower than any domain where actual traceSignals are used. Otherwise, I would have definitely said "make it the same as HDL simulation", but this is impossible.

[martijnbastiaan]

Hah, so that was going to be my comment. For HDL we indeed use ~LONGESTPERIOD (or whatever the exact name is). Maybe I missed it in your explanation above, but what would be against doing this in simulation too? I know we can't gather all the clocks in simulation, but we could do it for the ones that end up in the VCD.

On the API (non-)change: I really doubt anyone is using the VCDs as a machine format. I would feel comfortable just changing it -- though of course in a major release with a corresponding changelog entry.

(Also: maybe we should start plotting the clocks!)

[DigitalBrains1]

what would be against doing this in simulation too?

I think mainly this: suppose a user sees something in their VCD they don't really understand, at t = 1,234 ns. They decide to add another signal to the output, which happens to live in a clock domain they hadn't traced yet. The whole simulation might shift in time, and now they have to figure out where that interesting event at 1,234 ns went to. Although that will only occur if the added domain is now the slowest domain and it has a frequency less than 10 MHz.

I would feel comfortable just changing it

If a user is accustomed to having the first clock edge at t = 0 s and we change the output so that edge is now at a later point in time, that user might get quite confused by the new output since they don't know we changed the output unless they happened to go through the Changelog carefully. Worse than being confused, they might misinterpret the output altogether. If we always rendered a clock signal, they can see the beginning of the output changing and will pick up on it. But we're potentially just changing a waveform that went 1 - 2 - 3 etcetera to a waveform that goes 0 - 1 - 2 - 3. If I was used to my waveform not showing the initial value and suddenly being confronted with a waveform that has an unexpected value tacked to the front of it, I wouldn't think the output format had changed. I'd think I'd done something wrong in my design like accidentally adding an extra flip-flop at the end or keeping my reset asserted longer than I thought I did, or enabling a flip-flop before I meant to, etcetera.

[martijnbastiaan]

Right, a similar thing happens when you add a clock in your design due to ~LONGESTPERIOD. I had never considered that.

It's a good point. It didn't occur to me because I never look at timestamps, except for getting a very rough idea of where I am in the plot.

[marijn-qbaylogic]

I wonder what the primary purpose of offset is. Is it to simply reduce the number of values that end up in the VCD file? It will rarely cause less values to be evaluated, so that's not it. I would think looking at it in a waveform viewer where it is offset by the actual time would get rather annoying, though I also see the drawbacks of having a "redefined" timeline.

I have made a version of dumpVCD that takes (start,stop) clkDelay as inputs, and fixes the weird number of clockcycles behaviour by adding a timestamp without value changes at t=stop. However, with the current definition of the (start,stop) slicing, this means that:

• start and clkDelay are basically mutually exclusive - why would you want to start the clocks later and then cut off that extra time?
• if you want to run the actual simulation for some amount of time, you need to add the clkDelay to stop too.

But just now I thought: why not simply take negative time as the start offset? I.e. if you simulate with (-1,10) it will show you the 1 time unit before the first clock edge, and continue running for 10 time units after that, for a total simulation length of 11. That feels fairly intuitive to me.

[marijn-qbaylogic]

An alternative way to define it is that start is relative to the start of the clocks

Basically, yeah. Both start and stop would be, and you would need no clock start delay parameter.

This means that if you start at 0, you don't see the initial value - just like it is right now. If you specify a negative value, the VCD traces start at some point before the first clock edge, and so you will see the initial values. A positive value will just cut off more form the start. You are effectively setting the "viewport" of your signals, and in my mind that's fairly elegant.

it might mean a user might need to do a little calculation

You say you will need to do calculations, but those can be put into functions. "Give me 50 cycles of the 100MHz domain" using some function clockCycles @Dom100, for example. I also think it'd be least confusing if all time parameters used the same time unit, so the default isn't some mix between ps and cycles. I do agree that "cycles in the sampled domain" is a better unit than GCD cycles.

Oh, and as for adding the timestamp to the end: it is at the very least completely valid VCD (nothing says a timestamp needs something to happen) and it works in Shockwaves. I haven't tested GTKWave, but I suspect it does too. The standard specifies vcdclose in the extended VCD file format to set the end time of the simulation, but I am pretty sure this is not supported by Surfer. It should be supported by other waveform viewers, though in the worst case they might not draw the final cycle, which in my opinion is not the end of the world if both Surfer and GTKWave are fine.

[marijn-qbaylogic]

Actually, what do you expect the VCD to look like if the cutoff point is somewhere where all signals are stable? Would you want dumpVCD to just repeat the current values? That seems much weirder to me than adding a timestamp without any changes at the end.

[marijn-qbaylogic]

https://sigrok.org/wiki/File_format:Vcd this source places one timestamp at the end too, and looking at the source code of pyvcd, you can call writer.close(timestamp) to close the VCD file and add timestamp to the very end.

So it is something other people are doing. I think it's fine.

[DigitalBrains1]

A more standards-conformant thing to emit would be a timestamp followed by a $dumpall, but we can also emit a bare timestamp and wait for a bug report to come in. However, people might not realise it when their viewer doesn't support the bare timestamp, and just think we're ending at a weird place.

[DigitalBrains1]

https://sigrok.org/wiki/File_format:Vcd this source places one timestamp at the end too, and looking at the source code of pyvcd, you can call writer.close(timestamp) to close the VCD file and add timestamp to the very end.

So it is something other people are doing. I think it's fine.

I forgot to say: this is very good to know, knowing that others do this as well! Thanks for sharing.

[marijn-qbaylogic]

On plotting clock signals:

I don't know who said it first, but I agree plotting clock signals should be optional. Implementing them as normal signals is... problematic - mostly because the clock signal has double the clock period (which makes clock signals for domains with an odd period impossible and also affects the domain period GCD). I have made a version where the list of clocks is just another argument - it's just a list of signal name-clock period pairs that you can easily get using a function, like [clockWave @DomA "domA", clockWave @DomB "domB"].

I know that Surfer then gives you the option to mark the signal as a clock signal, which adds some vertical lines on positive clock edges.

However, I personally don't think the simple clock wave helps much visually when working with multiple clock domains, so I was also working on a Shockwaves version that simply alternates colors between clock cycles (and has a separate color for the initial values). I was thinking about expanding it to include reset and enable, and basically just make it a complete domain tracing signal.

[martijnbastiaan]

I don't know who said it first, but I agree plotting clock signals should be optional. Implementing them as normal signals is... problematic - mostly because the clock signal has double the clock period (which makes clock signals for domains with an odd period impossible and also affects the domain period GCD).

Check out:

clash-compiler/clash-lib/prims/verilog/Clash_Signal_Internal.primitives.yaml

Lines 134 to 158 in 1b27229

	template: |-
	// tbDynamicClockGen begin
	// pragma translate_off
	reg ~TYPO ~GENSYM[clk][0];
	time ~GENSYM[half_period][1];
	always begin
	// Delay of 1 mitigates race conditions (https://github.com/steveicarus/iverilog/issues/160)
	#1 ~SYM[0] = ~IF~ACTIVEEDGE[Rising][0]~THEN 0 ~ELSE 1 ~FI;
	#~LONGESTPERIOD0 forever begin
	~IF~ISACTIVEENABLE[2]~THEN
	if (~ ~ARG[2]) begin
	`ifdef VERILATOR
	$c("std::exit(0);");
	`endif
	$finish(0);
	end
	~ELSE~FI
	// 1 = 0.1ps
	~SYM[1] = (~VAR[periods][1] / 2);
	~SYM[0] = ~ ~SYM[0];
	#(~SYM[1] * 0.01);
	~SYM[0] = ~ ~SYM[0];
	#(~SYM[1] * 0.01);
	end

I.e., you can get around this by (p*10) / 2 (i.e., use a resolution of 0.1ps instead of 1ps).

[marijn-qbaylogic]

There hasn't been a lot of activity in a while here, so here's my current ideas for a rewrite of dumpVCD:

• Have parameters for the clock start, output start, output stop/duration, and a boolean for whether the start of the simulation should be shifted to 0.
• Have an argument for a list of clock signals to generate. To make this possible, internal timings in dumpVCD are converted to fs 64 bit numbers.
• Time parameters take some special type SimTime dom which matches the domain of the signal provided. Integer literals are converted to cycles in that clock domain, but it is also possible to specify time in ps/ns/etc or as clock cycles in another domain. Internally, these are all just ps and can be added and compared. This allows for, for example, a start time that is max(1 cycle,100ns) very easily.
• timescale is determined using the GCD of all signal periods and the time parameters, but otherwise we just do away with the GCDs alltogether. Values don't need to actually be scaled to the GCD.
• The VCD looks like #<start> $dumpvars <values before start> $end <changes> #<stop> with possibly a $dumpall afterwards
• dumpVCD doesn't really need all this - instead, the advanced features are available through one or more other functions (like advancedDumpVCD or dumpVCDWithClocks - to be determined)

[marijn-qbaylogic]

I've created a branch with my proposed changes to dumpVCD (experimental-rewrite-dumpvcd). I'd love to hear what can be improved.

[DigitalBrains1]

So taking a bit of code from experimental-rewrite-dumpvcd:

data Time (dom :: Domain) = Time {timeInPs :: Int64}

instance KnownDomain dom => Num (Time dom) where
  [...]
  fromInteger = Time . (* clockPeriod' @dom) . fromInteger

-- | Time in picoseconds
timePs :: forall dom. Int64 -> Time dom
timePs = Time
-- | Time in nanoseconds
timeNs :: forall dom. Int64 -> Time dom
timeNs = Time . (*1000)
[...]

-- | Time in clock cycles of the specified domain, i.e. `clkCycles @DomB 5`
clkCycles :: forall dom1 dom2. KnownDomain dom1 => Int64 -> Time dom2
clkCycles = Time . (* clockPeriod' @dom1)

dumpVCD
  :: NFDataX a
  => KnownDomain dom
  => Time dom
  -- ^ Start
  -> Time dom
  -- ^ Duration
  -> Signal dom a
  -- ^ (One of) the outputs of the circuit containing the traces
  -> [String]
  -- ^ The names of the traces you definitely want to be dumped in the VCD file
  -> IO (Either String Text.Text)

This dom in Time (dom :: Domain) isn't sitting completely right with me. The only reason Time is polymorphic is to shove the dom from dumpVCD into Num (Time dom). This feels fabricated to me. While I understand that in, e.g., ¹

x :: Signal System Int

dumpVCD (clockCycles @System 5) (clockCycles @System 42) x ["x"]

it's quite a bit more verbose to have to write clockCycles @System as opposed to

dumpVCD 5 42 x ["x"]

but I'm unconvinced that this is a good reason to decide to make dom a part of the Time type. It feels improper to me.

You could still write dumpVCD 5 42 if you just say that dumpVCD always takes clock cycles, and for any other method of specifying time you use advancedDumpVCD. I'm not saying this is a necessary thing, it's just one of the options.

¹ Note that my clockCycles is a different function than your clkCycles in the code above.

[DigitalBrains1]

The amount of text in this discussion was enough to potentially scare away would-be contributors. Marijn already recapped some things, and the rest could be easily summed up in this message, so I've hidden a slew of messages which are no longer necessary.

Marijn proposed the ability to specify times passed to a VCD generating function in different ways:

• In clock cycles of the domain sampled by the dumpVCD function
• As a proper time (ps and other units)
• As clock cycles in a specific domain

He suggested making the Clash domain part of the type representing times, and tying this to the domain of the traced signal in dumpVCD. That way, you can have expressions like startDelay = max (timeNs 100) (clockCycles 1) and clockCycles would be inferred to be in the domain of the sampled signal. He suggested using a Num instance for the time argument, which means plain numbers would be interpreted as clock cycles of the domain sampled.

I suggested we make multiple functions, e.g. dumpVCD and advancedDumpVCD, where dumpVCD is kept simple with a fixed unit and the other allows more configurability.

[marijn-qbaylogic]

Alternative which I'm currently leaning towards: just using picoseconds for time, but creating patterns (like TimePS, TimeNS) for that type that allow time to be specified with units. This way, there is no Time type convoluting the namespace. It's also safer than say, multiple constructors, since TimePS 1000 will just match TimeNS 1.

[rowanG077]

Quoting @lmbollen:

I've taken a look at the discussion and collected some thoughts. They contain both relatively straightforward remarks as well as what could be categorized as wishful thinking / too much of an overhaul:

General dumpVcd API

• If we change dumpVCD to be more configurable in what it evaluates / things it stores in the VCD (also a clock signal?) I also want an idiotproof wrapper that does not require the user to think about the arguments, as simple as possible.
  Peter already mentioned dumpVCD and advancedDumpVcd (though I'd prefer dumpVcd and dumpVcdWithconfig, but alas).

• I would like writeVCD, a function that immediately writes the vcd to a supplied path instead of making the user writing it themselves (having to depend on text themselves and importing the correct writeFile)

• I don't necessarily see much benefit in specifying a start offset in dumpVCD (we also don't do it in our sample functions). To me it only pollutes the API.

• It makes sense to me to either (derive a time range from the number of cycles and the domain) or allow the user to specify the duration in a time unit (ns, us, ms, s, etc.).

• "To make this possible, internal timings in dumpVCD are converted to fs 64 bit numbers.", Is the performance hit of Natural too large? (Or Integer if we consider it to be possible to ever have to encounter negative values)

• If we are going to allow the user to specify the duration in time, I propose to have a single way to represent time in clash-prelude. We already have type synonyms for this, but it makes sense to me to also have one way of specifying type on a term level. We should then also consider if we want to apply that to all functions that talk about time (also sample and friends?). Though this can be done incrementally.

General dumpVcd behavior

• We need clear consistency between the the samples in the VCD, their timestamps and the arguments supplied to dumpVCD (and potential friends?), Peter has thoroughly shown that that is currently not the case..

• It makes sense to me to (optionally?) evaluate all traced signals for the whole simulation window instead of relying on lazy evaluation to add it to the vcd when it is evaluated.

• This may be more of a shockwaves thing, but it would be nice if we could color traces in surfer based on their clock domain.

Tracing

• To allow the user to better differentiate between reset states and a running circuit we should be able to trace the reset signal (or it should be traced by default?). Marijn also mentioned something related.

• In an ideal world the user would not have to manually select which Signal's to trace and we'd get hierarchical traces of all signals that have an assigned name. (wishful thinking very much, I know). Could we investigate the possibility of automatically adding signals.

Implementation

• I can imagine that a lot of problems can be solved (or features added?) if we simply extend Signal / build "low level" tracing infrastructure into Signal / Clash. I just want to note that if this could be beneficial it makes a lot of sense to do so.

Please let me know if anything is unclear

[lmbollen]

As an extra remark, I think that the current feature where dumpVCD (or rather it was a function called waitForSignals I think) searches outside the specified range until it has found all the given names is more of a burden than anything else. It becomes annoying to differentiate between a mistyped signal name and a long running simulation.

If we stay with this approach of evaluating past the range to find signals we should at least bound it. (e.g evaluate up to 1.10x the given range)

[lmbollen]

Maybe it would also be possible to keep a "progress bar"? For example just printing an overwriting percentage or status to the terminal would already be helpful, or we could track the signals that have been found and the ones that have not been found.

[marijn-qbaylogic]

Ok so for dumpVCD, we'll probably have a simple and an advanced version. The complete version has:

• a start time in ps
• a duration or stop time in ps
• the clock start time in ps
• whether or not to shift the start of the simulation to t=0
• what clock waveforms to generate (this could technically be done elsewhere through trace-like functions)

What functionality do we want in the simple version? Do we need a start? Do we need full time control, or is cycles enough? I suppose we probably don't need clock start or shift. Do we want clock waveforms?

And what would be a logical way of supplying these arguments? I personally think (start,stop) is fine but (start,duration) is a bit weird. We could also have a configuration object that you can then modify using record syntax, to emulate optional parameters. Would that be worth it here? I'd also like to look at the order of the arguments - I kind of get why the list of signals to trace is the last argument, and not the signal, but what if we start adding a list of clocks to the mix?

[marijn-qbaylogic]

Feature idea: pack the signals under their domain, instead of just logic. I also wrote code so signals like "module.input" would be put in logic > module > input (in a module subscope) but I'm not entirely sure how these should be combined. Just have the domain as the root, always?

[marijn-qbaylogic]

Feature request (from @lmbollen ): if no list of signals to wait for is provided, instead of erroring, evaluate the provided signal in the slice range and use any signals that pop up there.

[DigitalBrains1]

Here's a concrete suggestion for the new dumpVCD API. Or actually, two suggestions.

First off, our naming conventions are not rigorously defined, but in general we say max two capitals in a row. So it's actually dumpVcd, not dumpVCD. I never noticed before because I'm used to the status quo.

Secondly, let's define some time handling.

lmbollen:

I propose to have a single way to represent time in clash-prelude

These utilities should be broadly applicable, not just for dumpVcd.

Let's define:

newtype Time = TimeFS Integer
  deriving (Eq, Ord)

instance Show Time where ...

pattern TimePS t <- (getTimePsPat -> Just t) where
  TimePS t = TimeFS (t * 1000)

-- This is an internal function
getTimePsPat = ...

-- Time in picoseconds, round to nearest, ties away from zero
timeInPS :: Time -> Integer
timeInPS t = ...

-- ... and so on for TimeNS, TimeUS, TimeMS, TimeS, TimeM and TimeH

-- The duration of n clock cycles in domain dom
-- Negative is fine to use
clockCycles ::
  forall dom .
  KnownDomain dom =>
  Integer ->
  Time

These patterns mean you can actually pattern-match on TimeNS t and it would match if it is an exact number of nanoseconds. No rounding! Pattern-matching TimeNS t on TimeFS 12_000_000 would bind t to 12, but TimeFS 12_000_001 would fail to match the pattern. I don't really see an application for pattern-matching like this, but if we're going to define extra constructors we kinda do want a bidirectional pattern synonym. Using the timeInPS function would round to whole picoseconds even for inexact durations.

I would like to be able to add and subtract Time but I'm not a fan of fromInteger. Ugh, Num, why are you the way you are? Should fromInteger just bottom?

I propose we define dumpVcd and friends as:

-- Internal implementation, no need to expose constructor. Keep the mystery.
type ClockWave = (String, Period)

clockWave ::
  KnownDomain dom =>
  -- | The name of the VCD variable with the clock wave
  String ->
  ClockWave

dumpVcd ::
  NFDataX a =>
  -- | Duration
  Time ->
  -- | Clock waves to render
  [ClockWave] ->
  -- | Names of the VCD variables you definitely want to be dumped in the VCD file
  [String] ->
  -- | (One of) the outputs of the circuit containing the traces
  Signal dom a ->
  IO (Either String Text)

and additionally define functions like:

writeVcd ::
  FilePath ->
  ... ->
  IO ()

-- This one is just a convenience function for `unsafeFromReset` and `traceSignal` combined
traceReset

-- And just `fromEnable`
traceEnable

The clock for dumpVcd always starts at 100 ns, regardless of clock domains. So you would see the initial value in the waveform in the interval 0 ns to 100 ns and then the wave transitions to the first output value.

Note that dumpVcd (clockCycles @System 5) would show a waveform ending at (100 + 5×10) = 150 ns and so would show 5 full clock cycles, and the actual VCD file would end with the sixth sample which cannot really be seen in the waveform as it is right at the end. In case there is no change at 150 ns, the VCD file would end with a bare timestamp, which is also emitted by sigrok, so we're not inventing out-of-spec things solo at least.

Currently, the search for all signals we know we want traced is never-ending. I propose we limit it to the duration of the VCD trace itself, and output both "Searched 10%..." (20%, 30%, ...), "Found trace <name>" and "Found all traces" to stdout while this process is going on.

Both marijn-qbaylogic and lmbollen have expressed doubts about the usefulness of not starting the simulation at the start. If we drop that ability, I actually feel like this dumpVcd / writeVcd already provides all useful features.

But if we decide that we do want more features, I propose we define something like

data TimestampOrDuration = Timestamp Time | Duration Time
  deriving (Show)

data VcdConfig
  = VcdConfig
  { vcdStart :: Time
  -- ^ Start the dump at this time
  , vcdClockStart :: Time
  -- ^ Start the clocks at this time
  , vcdShiftZero :: Bool
  -- ^ Shift `vcdStart` to time zero in the VCD?
  , vcdStatusMsgs :: Bool
  -- ^ Emit status messages to stdout?
  }
  deriving (Show)

instance Default VcdConfig where
  def =
    VcdConfig
      { vcdStart = TimeFS 0
      , vcdClockStart = TimeNS 100
      , vcdShiftZero = False
      , statusMsgs = True
      }

dumpVcdWith ::
  NFDataX a =>
  VcdConfig ->
  -- | Stop the dump at this time or after this much time since `vcdStart`
  TimestampOrDuration ->
  -- | Clock waves to render
  [ClockWave] ->
  -- | Names of the traces you definitely want to be dumped in the VCD file
  [String] ->
  -- | (One of) the outputs of the circuit containing the traces
  Signal dom a ->
  IO (Either String Text)

writeVcdWith ::
  FilePath ->
  VcdConfig ->
  ...

and dumpVcd would end up being:

dumpVcd t = dumpVcdWithConfig def (vcdClockStart def + Duration t)

Finally, my second proposal. I feel we use the "design for qualified imports" naming scheme too little in clash-prelude. Furthermore, our terminology does not align with VCD terminology, I'd much rather use established names so people who know VCD files understand what we say. How about we design for qualified imports so people do

import qualified Clash.Signal.Vcd as Vcd

(yes, new name!)

and we define

module Clash.Signal.Vcd where

clockWave

data TimestampOrDuration

data Config
  = Config
  { start :: Time
  , clockStart :: Time
  , shiftZero :: Bool
  , statusMsgs :: Bool
  }

-- This is dumpVcd
dumpText

-- This is writeVcd
dumpFile

-- This is dumpVcdWith
dumpTextWith

-- This is writeVcdWith
dumpFileWith

-- This is traceSignal
dumpVar

-- This is traceVecSignal
dumpVecVar

-- This is traceReset
dumpReset

-- This is traceEnable
dumpEnable

Note that the time functions don't belong in either Clash.Signal.Trace nor the proposed Clash.Signal.Vcd, I'm not sure where we define them but people import them from Clash.Prelude.

I've run out of time writing and rereading this, I hope I put in all my major thoughts. I won't have an opportunity to continue on this for several days, so happy extended weekend!

[edit 2026-04-06]
I realised I had inadvertently named functions dumpVcdWithConfig instead of the established pattern of dumpVcdWith (used at least in clash-cores), so I changed the names of the functions to drop the Config suffix.

Also, I realised that if you define dumpVcd in terms of dumpVcdWith, you need to add the vcdClockStart to the Duration, so I amended that. This also shows how Num is very nice to have on Time even though I don't actually want fromInteger.
[/edit]

[DigitalBrains1]

There's also our current functions dumpReplayable and replay. The only commonality they share with the VCD stuff is that they also use traceSignal to select a signal to observe. I'm not quite sure what to do with these in a different naming scheme. The functions have a purpose, as explained in the PR that introduced them. They deserve to be kept.

[marijn-qbaylogic]

A few remarks:

• I thought we were against using Time as a name, because it's very likely to cause namespace collisions?
  While we can say that you should just import it as qualified, I think the constructors (or well, patterns) like TimePS should be exported by the prelude, but doing that without exporting the type feels weird to me. I don't necessarily have a suggestion or strong opinions, but it's something that was brought up that I don't see here anymore.
• Do we want to make the simulation and file write functions one function? I thought it'd be like writeVcd "filename" $ simulateVcd ... or so. I personally dislike the idea that there is a "matrix" of functions combining the different simulation and different output functions.
• If we have a special function for writing the VCD... we have a VCD type now, so why don't we just return that object directly and let the file write function handle the conversion? I don't know if that's a good idea, but in my mind, having a simulation command that returns a VCD object and another function to write that object to a file makes sense.
• I do think that for some reasons like file sizes it might be good to set the start time. If you are using shockwaves LUTs it could certainly be very useful.
• Is it just me, or does it make sense to run a simulation with {start=-100ns, clock start=0, stop=duration, shift=True}? The output isn't any different - this would only be a change to the default config and stop you from having to add the 100ns to the duration.
• Do we want time to work in femtoseconds? I figured we'd be using picoseconds since that seems to be the standard throughout Clash. I don't think we actually need fs outside of dumpVcd, and really only half picoseconds for making the clock waves? If we allow fs in Time, we have to add support for that too everywhere, or add a bunch of checks to make sure everything is a multiple of 1ps.
• If we restrict time to ps, we could have fromInteger default to that - it'd make the changes more compatible with the rest of clash, though it might be unintuitive in the case of dumpVCD.
• If we have a "simple" dumpVCD, can we just keep to clock cycles as an input? I doubt that'd be better, but having to write (clockCycles @dom x) for the most common case feels ugly. I also thought of maybe using a hidden clock for the time parameter so you don't have to explicitly supply the domain of the toplevel signal again.

[marijn-qbaylogic]

At the very least, providing the time parameter with a hidden clock can't hurt, can it?
(HiddenClock => Time) also works for anything that provides a time value without hidden clock, and allows us or users to define such a function.

[marijn-qbaylogic]

For Time: allowing multiplication through mixed type mul would be nice.

[DigitalBrains1]

I will answer more later, but let's pick this one out now

At the very least, providing the time parameter with a hidden clock can't hurt, can it?

I assume you mean something like this:

Periods.hs

{-# LANGUAGE AllowAmbiguousTypes #-}
module Periods where

import Clash.Prelude

createDomain vSystem{vName = "DomA", vPeriod = 42}

getPeriod ::
  forall dom .
  KnownDomain dom =>
  (HiddenClock dom => Natural) ->
  Natural
getPeriod f = withClock (clockGen @dom) f

periodFor ::
  forall dom .
  KnownDomain dom =>
  Natural
periodFor = natToNatural @(DomainPeriod dom)

periodI ::
  forall dom .
  HiddenClock dom =>
  Natural
periodI = periodForClock hasClock
 where
  periodForClock ::
    Clock dom ->
    Natural
  periodForClock _ = periodFor @dom

which gives us:

$ clashi-1.8 -Wall Periods.hs
[...]
>>> getPeriod @DomA (periodFor @System)
10000
>>> getPeriod @DomA periodI
42

This of course doesn't need HiddenClock per se, on the face of it, it seems better to create a new Hidden variation than pervert HiddenClock this way. It is basically a HiddenDomain, where the term could be a Proxy.

But it works because of the fact that expecting and filling in a hidden parameter does not require that the argument actually has a hidden parameter. So we can just fill in periodFor despite it not having a hidden parameter, have it type-check and correctly obtain the period for System, but at the same time allow us to use periodI which will get its hidden parameter from its caller.

I need to think about how confusing this is to new users, especially since we want to shift focus for new users on not using HiddenClockResetEnable in the first place.

[marijn-qbaylogic]

I want to open a bit of discussion to the signal names too.
Currently all signals are bundled together under the logic module, making everything a big mess.
I proposed we use . as separator to create a hierarchy. This means you could add module.input as a signal name and get a signal input in logic > module > input. It also makes namespacing easier.

However, it would also be very useful to group signals by domain. That way, signals of a domain are automatically grouped together, and namespace collisions are also avoided.

Combining the two, however, is a bit tricky. What would be the best solution? I think that maybe it'd just be better to do something like "$ in the signal name is replaced by the domain name", so you can just write $.clock for example. Other suggestions were automatically prepending the domain if the signal name has a leading period.

Changing the signal names makes waiting for certain signals to become defined a little more difficult. We don't want to have it so that you write $.sig as the signal name, and then have to wait for System.sig, so I propose we store the original and modified strings in a set and check the requested signal names against that set. This way you can still wait for $.sig but also System.sig, should the same signal be traced in two domains.

Note that we'd also change vector tracing - instead of creating signals like myvec_0, myvec_1, etc. we'd create them as myvec.0, myvec.1 etc and have them grouped in a module automatically. Also, the original name myvec could get stored, so you could wait for myvec instead of having to wait for myvec.0, for example.

[marijn-qbaylogic]

But numbers sort differently from strings, since 10>2 but "10"<"2", so you have to treat them differently.
Storing "a.b.c" as a list of Eithers could work.

But I probably shouldn't have brought up such implementation details in the first place. Let's keep it to the behavior we want for now.