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

First active edge of clock in testbench not half of clock period #2001

Closed
KoenRaben opened this issue Nov 17, 2021 · 11 comments · Fixed by #2007
Closed

First active edge of clock in testbench not half of clock period #2001

KoenRaben opened this issue Nov 17, 2021 · 11 comments · Fixed by #2007

Comments

@KoenRaben
Copy link

When generating a verilog testbench and a clock signal with a period of 20 ns the time to the first rising edge of the clock is set to 3 ns instead of the expected 10 ns. I have included the relevant part of the verilog code below. This results in problems with a post-synthesis simulation, as there is too little setup time before the first clock edge. Preferably this time to the first rising edge would be half of the clock period, or adjustable through a setting.

Verilog snippet:
// tbClockGen begin
// pragma translate_off
reg clk_0;
// 1 = 0.1ps
localparam half_period = (200000 / 2);
always begin
// Delay of 1 mitigates race conditions (steveicarus/iverilog#160)
#1 clk_0 = 0 ;
#30000 forever begin //<------------- Where does this number come from?
if (~ (~ c$result_rec)) begin
$finish;
end
clk_0 = ~ clk_0;
#half_period;
clk_0 = ~ clk_0;
#half_period;
end
end

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Nov 17, 2021

In a multi-clock design, all clocks currently have their first active edge at the same time. So simply setting the first active edge to half the period of the respective clock would break that expectation, we need to do something clever :-).

[edit]
But your solution of making it configurable would allow to maintain that relation between clocks.
[/edit]

@rowanG077
Copy link
Member

rowanG077 commented Nov 17, 2021

Wouldn't it be as easy to set the first active edge of every clock at half period of that respective clock? Or do we depend on every clock starting at the same time for proper simulation results?

@christiaanb
Copy link
Member

christiaanb commented Nov 17, 2021

Our synchronization model:

veryUnsafeSynchronizer t1 t2
-- this case is just an optimization for when the periods are the same
| t1 == t2 = same
| otherwise = go 0
where
same :: Signal dom1 a -> Signal dom2 a
same (s :- ss) = s :- same ss
go :: Int -> Signal dom1 a -> Signal dom2 a
go relativeTime (a :- s)
| relativeTime <= 0 = a :- go (relativeTime + t2) (a :- s)
| otherwise = go (relativeTime - t1) s

assumes that the first active edges of all clocks coincide. So the clockgen code and that model are linked: change one, then you need to change the other.

We will be adding phase rotation information to clock domains, which would also necessitate a change to the synchronization model. Hopefully we can then tackle this issue as well.

Then if we want clocks that do have first active edges that coincide we can do something like https://hackage.haskell.org/package/clash-prelude-1.4.6/docs/Clash-Clocks.html allowing us to know which clock has the longest period.

@DigitalBrains1
Copy link
Member

If our simulation in Haskell produces values without throwing exceptions, we generally require those values to be the same as when the design is run in HDL simulation. Our multi-clock primitives assume the first active edge coincides for all domains, so we require that for HDL simulation as well.

By the way, why /half/ the clock period? Why not a full period?

@christiaanb
Copy link
Member

christiaanb commented Nov 17, 2021

It needs to be a full period in order to meet setup time constraints.

@christiaanb
Copy link
Member

A more immediate solution is to collect information about domains before the netlist stage. Then we can instantiate the clockGen with a startdelay of the domain with the longest clock period.

@alex-mckenna
Copy link
Contributor

We already have information about domains from GenerateBindings, or do you mean something else / more?

@christiaanb
Copy link
Member

No, that's it. I didn't realize we already collected info. What do we use it for? I always assumed we just parsed the KnownDomain dom arguments.

@alex-mckenna
Copy link
Contributor

IIRC I added it a while back to start on an idea @martijnbastiaan had about making KnownDomain less necessary in user code. But then I didn't have time to pursue it so I left it at collecting the info

@alex-mckenna
Copy link
Contributor

Looking around I think we pass these collected domain configurations to mkManifest. The domain configuration map is passed around though generateHDL and friends

@rowanG077
Copy link
Member

I think you are referring to #978

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 a pull request may close this issue.

5 participants