Correcetly simulate passed time before first clock edge in verilator

The FFI code used a default of 50 nanoseconds as time passed before the first clock edge. This resulted in an unobservable reset pulse when used with `resetGenN d1` because verilator's simulation time is off by a factor 1000. Now derive the time before the clock edge as a single period of vexrisc clock domain.
clash-lang · Jan 10, 2025 · e85979a · e85979a
1 parent 5123912
commit e85979a
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 21 deletions.
diff --git a/cabal.project b/cabal.project
@@ -17,36 +17,37 @@ package clash-prelude
 -- state by running `cabal update` twice and looking at the index state it
 -- displays to you (as the second update will be a no-op)
 index-state: 2023-09-28T08:48:26Z
+-- index-state: 2025-01-10T11:37:42Z
 
 -- Needed to simulate dynamic clocks.
 source-repository-package
   type: git
   location: https://github.com/clash-lang/clash-compiler.git
-  tag: a26bb9ef3cb57f24daec3da30db9fb54af91297a
+  tag: 6f9b7300da535bae01da67eeac99dd82f2cac965
   subdir: clash-prelude
 
 source-repository-package
   type: git
   location: https://github.com/clash-lang/clash-compiler.git
-  tag: a26bb9ef3cb57f24daec3da30db9fb54af91297a
+  tag: 6f9b7300da535bae01da67eeac99dd82f2cac965
   subdir: clash-ghc
 
 source-repository-package
   type: git
   location: https://github.com/clash-lang/clash-compiler.git
-  tag: a26bb9ef3cb57f24daec3da30db9fb54af91297a
+  tag: 6f9b7300da535bae01da67eeac99dd82f2cac965
   subdir: clash-lib
 
 source-repository-package
   type: git
   location: https://github.com/clash-lang/clash-compiler.git
-  tag: a26bb9ef3cb57f24daec3da30db9fb54af91297a
+  tag: 6f9b7300da535bae01da67eeac99dd82f2cac965
   subdir: clash-cores
 
 source-repository-package
   type: git
   location: https://github.com/clash-lang/clash-compiler.git
-  tag: a26bb9ef3cb57f24daec3da30db9fb54af91297a
+  tag: 6f9b7300da535bae01da67eeac99dd82f2cac965
   subdir: clash-prelude-hedgehog
 
 source-repository-package

diff --git a/clash-vexriscv-sim/clash-vexriscv-sim.cabal b/clash-vexriscv-sim/clash-vexriscv-sim.cabal
@@ -71,7 +71,7 @@ common common-options
       -- clash-prelude will set suitable version bounds for the plugins
   build-depends:
     base >= 4.14 && < 4.19,
-    clash-prelude >= 1.6 && < 1.10,
+    clash-prelude >= 1.9.0,
     containers >= 0.6 && < 0.8,
     ghc-typelits-natnormalise,
     ghc-typelits-extra,

diff --git a/clash-vexriscv/clash-vexriscv.cabal b/clash-vexriscv/clash-vexriscv.cabal
@@ -95,7 +95,7 @@ common common-options
       -- clash-prelude will set suitable version bounds for the plugins
   build-depends:
     base >= 4.14 && < 4.19,
-    clash-prelude >= 1.6 && < 1.10,
+    clash-prelude >= 1.9.0,
     containers >= 0.6 && < 0.8,
     ghc-typelits-natnormalise,
     ghc-typelits-extra,
@@ -119,7 +119,7 @@ library
   build-depends:
     base,
     bytestring >= 0.10 && < 0.13,
-    clash-prelude,
+    clash-prelude >= 1.9.0,
     clash-protocols,
     containers,
     directory >= 1.3 && < 1.4,
@@ -147,7 +147,7 @@ test-suite unittests
   build-depends:
     HUnit,
     base,
-    clash-prelude-hedgehog,
+    clash-prelude-hedgehog  >= 1.9.0,
     clash-vexriscv,
     bytestring,
     hedgehog >= 1.0 && < 1.5,

diff --git a/clash-vexriscv/src/VexRiscv.hs b/clash-vexriscv/src/VexRiscv.hs
@@ -185,7 +185,8 @@ vexRiscv dumpVcd clk rst cpuInput jtagInput =
 
 
 vexRiscv#
-  :: KnownDomain dom
+  :: forall dom .
+  KnownDomain dom
   => DumpVcd
   -> String
   -> Clock dom
@@ -250,7 +251,10 @@ vexRiscv# dumpVcd !_sourcePath clk rst0
   jtag_TCK0
   jtag_TMS0
   jtag_TDI0 = unsafePerformIO $ do
-    (v, initStage1, initStage2, stepRising, stepFalling, _shutDown) <- vexCPU dumpVcd
+    -- PeriodToHz imposes a minimum period of 1 Hz, but the KnownDomain constraint does
+    -- not supply this information.
+    let domPeriodFs = hzToFs (natToNum @(PeriodToHz (Max 1 (DomainPeriod dom))))
+    (v, initStage1, initStage2, stepRising, stepFalling, _shutDown) <- vexCPU dumpVcd domPeriodFs
 
     -- Make sure all the inputs are defined
     let
@@ -520,6 +524,7 @@ vexRiscv# dumpVcd !_sourcePath clk rst0
 -- the internal CPU state
 vexCPU ::
   DumpVcd ->
+  Femtoseconds ->
   IO
     ( Ptr VexRiscv
     , Ptr VexRiscv -> NON_COMB_INPUT -> IO OUTPUT           -- initStage1
@@ -528,7 +533,7 @@ vexCPU ::
     , Ptr VexRiscv -> Word64 -> COMB_INPUT -> IO ()         -- falling
     , Ptr VexRiscv -> IO ()
     )
-vexCPU dumpVcd = do
+vexCPU dumpVcd (fromIntegral . unFemtoseconds -> domPeriodFs :: Word64)  = do
   v <- vexrInit
   vcd <- case dumpVcd of
     NoDumpVcd -> pure nullPtr
@@ -541,7 +546,7 @@ vexCPU dumpVcd = do
     initStage1 vPtr nonCombInput =
       alloca $ \nonCombInputFFI -> alloca $ \outputFFI -> do
         poke nonCombInputFFI nonCombInput
-        vexrInitStage1 vcd vPtr nonCombInputFFI outputFFI
+        vexrInitStage1 vcd domPeriodFs vPtr nonCombInputFFI outputFFI
         peek outputFFI
 
     {-# NOINLINE initStage2 #-}

diff --git a/clash-vexriscv/src/VexRiscv/FFI.hsc b/clash-vexriscv/src/VexRiscv/FFI.hsc
@@ -27,7 +27,7 @@ foreign import ccall unsafe "vexr_init" vexrInit :: IO (Ptr VexRiscv)
 foreign import ccall unsafe "vexr_init_vcd" vexrInitVcd :: Ptr VexRiscv -> CString -> IO (Ptr VerilatedVcdC)
 foreign import ccall unsafe "vexr_shutdown" vexrShutdown :: Ptr VexRiscv -> IO ()
 
-foreign import ccall unsafe "vexr_init_stage1" vexrInitStage1 :: Ptr VerilatedVcdC -> Ptr VexRiscv -> Ptr NON_COMB_INPUT -> Ptr OUTPUT -> IO ()
+foreign import ccall unsafe "vexr_init_stage1" vexrInitStage1 :: Ptr VerilatedVcdC -> Word64 -> Ptr VexRiscv -> Ptr NON_COMB_INPUT -> Ptr OUTPUT -> IO ()
 foreign import ccall unsafe "vexr_init_stage2" vexrInitStage2 :: Ptr VexRiscv -> Ptr COMB_INPUT -> IO ()
 foreign import ccall unsafe "vexr_step_rising_edge" vexrStepRisingEdge :: Ptr VerilatedVcdC -> Ptr VexRiscv -> Word64 -> Ptr NON_COMB_INPUT -> Ptr OUTPUT -> IO ()
 foreign import ccall unsafe "vexr_step_falling_edge" vexrStepFallingEdge :: Ptr VerilatedVcdC -> Ptr VexRiscv -> Word64 -> Ptr COMB_INPUT -> IO ()

diff --git a/clash-vexriscv/src/ffi/impl.cpp b/clash-vexriscv/src/ffi/impl.cpp
@@ -35,8 +35,9 @@ VVexRiscv *vexr_init();
 VerilatedVcdC *vexr_init_vcd(VVexRiscv *top, const char *path);
 void vexr_shutdown(VVexRiscv *top);
 
-void vexr_init_stage1(VerilatedVcdC *vcd, VVexRiscv *top,
-                      const NON_COMB_INPUT *input, OUTPUT *output);
+void vexr_init_stage1(VerilatedVcdC *vcd, uint64_t dom_period_fs,
+                      VVexRiscv *top, const NON_COMB_INPUT *input,
+                      OUTPUT *output);
 void vexr_init_stage2(VVexRiscv *top, const COMB_INPUT *input);
 void vexr_step_rising_edge(VerilatedVcdC *vcd, VVexRiscv *top,
                            uint64_t time_add, const NON_COMB_INPUT *input,
@@ -119,8 +120,9 @@ void set_outputs(VVexRiscv *top, OUTPUT *output) {
   output->jtag_TDO = top->jtag_tdo;
 }
 
-void vexr_init_stage1(VerilatedVcdC *vcd, VVexRiscv *top,
-                      const NON_COMB_INPUT *input, OUTPUT *output) {
+void vexr_init_stage1(VerilatedVcdC *vcd, uint64_t dom_period_fs,
+                      VVexRiscv *top, const NON_COMB_INPUT *input,
+                      OUTPUT *output) {
   // Set all inputs that cannot combinationaly depend on outputs. I.e., all
   // inputs except the Wishbone buses.
   set_non_comb_inputs(top, input);
@@ -132,9 +134,10 @@ void vexr_init_stage1(VerilatedVcdC *vcd, VVexRiscv *top,
   }
   set_outputs(top, output);
 
-  // Advance time by 50 nanoseconds. This is an arbitrary value. Ideally, we
-  // would do something similar to Clash's template tag "~LONGESTPERIOD".
-  contextp->timeInc(50000);
+  // Advance time by the period of the vexrisc clock in femtoseconds.
+  // Because verilator does not support femtosecond resolution, the simulation
+  // time is off by a factor of 1000.
+  contextp->timeInc(dom_period_fs);
 }
 
 void vexr_init_stage2(VVexRiscv *top, const COMB_INPUT *input) {