From 0cda2e887bcf02626642af72bfd126ef22f34e10 Mon Sep 17 00:00:00 2001 From: Martijn Bastiaan Date: Fri, 10 Feb 2023 12:38:57 +0100 Subject: [PATCH] Remove domain ordering from 'trueDualPortBlockRam' Previous commits have removed the clock domain ordering sensitivity from the TDP BRAM model, but didn't remove the wrapper, nor the hack prepending `ClockB` to the result of `clockTicks`. Closes #2352 --- .gitignore | 3 + clash-prelude/src/Clash/Explicit/BlockRam.hs | 139 ++++++------------ .../Clash/Tests/AsyncFIFOSynchronizer.hs | 24 +-- 3 files changed, 60 insertions(+), 106 deletions(-) diff --git a/.gitignore b/.gitignore index 8162eea560..9b3a393a7d 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,6 @@ docs/env # rewrite histories *.dat + +# build_clash_dev.sh output +clash-dev.result diff --git a/clash-prelude/src/Clash/Explicit/BlockRam.hs b/clash-prelude/src/Clash/Explicit/BlockRam.hs index 79c3d16410..305b3d4a8f 100644 --- a/clash-prelude/src/Clash/Explicit/BlockRam.hs +++ b/clash-prelude/src/Clash/Explicit/BlockRam.hs @@ -445,7 +445,6 @@ import GHC.Arr import qualified Data.Sequence as Seq import Data.Sequence (Seq) import Data.String.Interpolate(__i) -import Data.Tuple (swap) import GHC.Generics (Generic) import GHC.Stack (HasCallStack, withFrozenCallStack) import GHC.TypeLits (KnownNat, type (^), type (<=)) @@ -458,9 +457,8 @@ import Clash.Explicit.Signal (KnownDomain, Enable, register, fromEnab import Clash.Signal.Internal (Clock(..), Reset, Signal (..), ClockAB (..), invertReset, (.&&.), mux, clockTicks) -import Clash.Promoted.Nat (SNat(..), snatToNum, natToNum) +import Clash.Promoted.Nat (SNat(..), natToNum) import Clash.Signal.Bundle (unbundle, bundle) -import Clash.Signal.Internal.Ambiguous (clockPeriod) import Clash.Sized.Unsigned (Unsigned) import Clash.Sized.Index (Index) import Clash.Sized.Vector (Vec, replicate, iterateI) @@ -1251,49 +1249,6 @@ trueDualPortBlockRamWrapper clkA enA weA addrA datA clkB enB weB addrB datB = trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB {-# NOINLINE trueDualPortBlockRamWrapper #-} --- | Primitive of 'trueDualPortBlockRam'. -trueDualPortBlockRam#, trueDualPortBlockRamWrapper :: - forall nAddrs domA domB a . - ( HasCallStack - , KnownNat nAddrs - , KnownDomain domA - , KnownDomain domB - , NFDataX a - ) - => Clock domA - -- ^ Clock for port A - -> Signal domA Bool - -- ^ Enable for port A - -> Signal domA Bool - -- ^ Write enable for port A - -> Signal domA (Index nAddrs) - -- ^ Address to read from or write to on port A - -> Signal domA a - -- ^ Data in for port A; ignored when /write enable/ is @False@ - - -> Clock domB - -- ^ Clock for port B - -> Signal domB Bool - -- ^ Enable for port B - -> Signal domB Bool - -- ^ Write enable for port B - -> Signal domB (Index nAddrs) - -- ^ Address to read from or write to on port B - -> Signal domB a - -- ^ Data in for port B; ignored when /write enable/ is @False@ - - -> (Signal domA a, Signal domB a) - -- ^ Outputs data on /next/ cycle. If write enable is @True@, the data written - -- will be echoed. If write enable is @False@, the read data is returned. If - -- port enable is @False@, it is /undefined/. -trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB - | snatToNum @Int (clockPeriod @domA) < snatToNum @Int (clockPeriod @domB) - = swap (trueDualPortBlockRamModel labelB clkB enB weB addrB datB labelA clkA enA weA addrA datA) - | otherwise - = trueDualPortBlockRamModel labelA clkA enA weA addrA datA labelB clkB enB weB addrB datB - where - labelA = "Port A" - labelB = "Port B" {-# NOINLINE trueDualPortBlockRam# #-} {-# ANN trueDualPortBlockRam# hasBlackBox #-} {-# ANN trueDualPortBlockRam# ( @@ -1407,8 +1362,8 @@ trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB // Shared memory logic [~SIZE[~TYP[#{datA}]]-1:0] ~GENSYM[mem][#{symMem}] [~LIT[#{knownNatAddrs}]-1:0]; - ~SIGD[~GENSYM[data_slow][#{symDoutA}]][#{datA}]; - ~SIGD[~GENSYM[data_fast][#{symDoutB}]][#{datB}]; + ~SIGD[~GENSYM[a_dout][#{symDoutA}]][#{datA}]; + ~SIGD[~GENSYM[b_dout][#{symDoutB}]][#{datB}]; // Port A always @(~IF~ACTIVEEDGE[Rising][#{knownDomainA}]~THENposedge~ELSEnegedge~FI ~ARG[#{clockA}]) begin @@ -1473,8 +1428,8 @@ trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB // Shared memory reg [~SIZE[~TYP[#{datA}]]-1:0] ~GENSYM[mem][#{symMem}] [~LIT[#{knownNatAddrs}]-1:0]; - reg ~SIGD[~GENSYM[data_slow][#{symDoutA}]][#{datA}]; - reg ~SIGD[~GENSYM[data_fast][#{symDoutB}]][#{datB}]; + reg ~SIGD[~GENSYM[a_dout][#{symDoutA}]][#{datA}]; + reg ~SIGD[~GENSYM[b_dout][#{symDoutB}]][#{datB}]; // Port A always @(~IF~ACTIVEEDGE[Rising][#{knownDomainA}]~THENposedge~ELSEnegedge~FI ~ARG[#{clockA}]) begin @@ -1503,53 +1458,52 @@ trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB // end trueDualPortBlockRam |]) #-} - --- | Haskell model for the primitive 'trueDualPortBlockRam#'. +-- | Haskell model/primitive for 'trueDualPortBlockRam'. -- --- Warning: this model only works if @domFast@'s clock is faster than (or equal --- to) @domSlow@'s clock. -trueDualPortBlockRamModel :: - forall nAddrs domFast domSlow a . +trueDualPortBlockRam#, trueDualPortBlockRamWrapper :: + forall nAddrs domB domA a . ( HasCallStack , KnownNat nAddrs - , KnownDomain domSlow - , KnownDomain domFast + , KnownDomain domA + , KnownDomain domB , NFDataX a ) => - String -> - Clock domSlow -> - Signal domSlow Bool -> - Signal domSlow Bool -> - Signal domSlow (Index nAddrs) -> - Signal domSlow a -> - - String -> - Clock domFast -> - Signal domFast Bool -> - Signal domFast Bool -> - Signal domFast (Index nAddrs) -> - Signal domFast a -> - - (Signal domSlow a, Signal domFast a) -trueDualPortBlockRamModel labelA clkA enA weA addrA datA labelB clkB enB weB addrB datB = + Clock domA -> + -- | Enable + Signal domA Bool -> + -- | Write enable + Signal domA Bool -> + -- | Address + Signal domA (Index nAddrs) -> + -- | Write data + Signal domA a -> + + Clock domB -> + -- | Enable + Signal domB Bool -> + -- | Write enable + Signal domB Bool -> + -- | Address + Signal domB (Index nAddrs) -> + -- | Write data + Signal domB a -> + + (Signal domA a, Signal domB a) +trueDualPortBlockRam# clkA enA weA addrA datA clkB enB weB addrB datB = ( startA :- outA , startB :- outB ) where (outA, outB) = go (Seq.fromFunction (natToNum @nAddrs) initElement) - -- ensure 'go' hits 'goFast' first for 1 cycle, then execute 'goBoth' - -- once, followed by the regular cadence of either 'ceil(tA / tB)' or - -- 'floor(tA / tB)' cycles for the fast clock followed by 1 cycle of the - -- slow clock - (ClockB : clockTicks clkA clkB) + (clockTicks clkA clkB) (bundle (enA, weA, fromIntegral <$> addrA, datA)) (bundle (enB, weB, fromIntegral <$> addrB, datB)) startA startB - startA = deepErrorX $ "trueDualPortBlockRam: " <> labelA <> ": First value undefined" - startB = deepErrorX $ "trueDualPortBlockRam: " <> labelB <> ": First value undefined" + startA = deepErrorX $ "trueDualPortBlockRam: Port A: First value undefined" + startB = deepErrorX $ "trueDualPortBlockRam: Port B: First value undefined" initElement :: Int -> a initElement n = @@ -1620,16 +1574,16 @@ trueDualPortBlockRamModel labelA clkA enA weA addrA datA labelB clkB enB weB add go :: Seq a -> [ClockAB] -> - Signal domSlow (Bool, Bool, Int, a) -> - Signal domFast (Bool, Bool, Int, a) -> + Signal domA (Bool, Bool, Int, a) -> + Signal domB (Bool, Bool, Int, a) -> a -> a -> - (Signal domSlow a, Signal domFast a) + (Signal domA a, Signal domB a) go _ [] _ _ = - error "trueDualPortBlockRamModel.go: `ticks` should have been an infinite list" + error "trueDualPortBlockRamModel#.go: `ticks` should have been an infinite list" go ram0 (tick:ticks) as0 bs0 = case tick of - ClockA -> goSlow - ClockB -> goFast + ClockA -> goA + ClockB -> goB ClockAB -> goBoth where (enA_, weA_, addrA_, datA_) :- as1 = as0 @@ -1669,25 +1623,22 @@ trueDualPortBlockRamModel labelA clkA enA weA addrA datA labelB clkB enB weB add outB2 = if enB_ then outB1 else prevB (as2,bs2) = go ram2 ticks as1 bs1 outA2 outB2 - -- 1 iteration here, as this is the slow clock. - goSlow _ prevB | enA_ = out0 `seqX` (out0 :- as2, bs2) + goA _ prevB | enA_ = out0 `seqX` (out0 :- as2, bs2) where (wrote, !ram1) = writeRam weA_ addrA_ datA_ ram0 out0 = fromMaybe (ram1 `Seq.index` addrA_) wrote (as2, bs2) = go ram1 ticks as1 bs0 out0 prevB - goSlow prevA prevB = (prevA :- as2, bs2) + goA prevA prevB = (prevA :- as2, bs2) where (as2,bs2) = go ram0 ticks as1 bs0 prevA prevB - -- 1 or more iterations here, as this is the fast clock. First iteration - -- happens here. - goFast prevA _ | enB_ = out0 `seqX` (as2, out0 :- bs2) + goB prevA _ | enB_ = out0 `seqX` (as2, out0 :- bs2) where (wrote, !ram1) = writeRam weB_ addrB_ datB_ ram0 out0 = fromMaybe (ram1 `Seq.index` addrB_) wrote (as2, bs2) = go ram1 ticks as0 bs1 prevA out0 - goFast prevA prevB = (as2, prevB :- bs2) + goB prevA prevB = (as2, prevB :- bs2) where (as2,bs2) = go ram0 ticks as0 bs1 prevA prevB diff --git a/clash-prelude/tests/Clash/Tests/AsyncFIFOSynchronizer.hs b/clash-prelude/tests/Clash/Tests/AsyncFIFOSynchronizer.hs index d26bbf26f5..5d5d74d0ca 100644 --- a/clash-prelude/tests/Clash/Tests/AsyncFIFOSynchronizer.hs +++ b/clash-prelude/tests/Clash/Tests/AsyncFIFOSynchronizer.hs @@ -279,7 +279,7 @@ test2R1 = testR sync2R1 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -289,7 +289,7 @@ test2R1 = testR sync2R1 , (True,(Just 3,False)) , (True,(Just 4,False)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 5,True)) , (False,(Just 5,True)) , (False,(Just 5,False)) , (False,(Just 5,False)) @@ -364,7 +364,7 @@ test2R2 = testR sync2R2 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -374,7 +374,7 @@ test2R2 = testR sync2R2 , (True,(Just 2,False)) , (True,(Just 3,False)) , (True,(Just 4,False)) - , (False,(Nothing,True)) + , (False,(Just 5,True)) , (False,(Just 5,True)) , (False,(Just 5,False)) , (False,(Just 5,False)) @@ -454,7 +454,7 @@ test2R3 = testR sync2R3 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -464,7 +464,7 @@ test2R3 = testR sync2R3 , (True,(Just 3,False)) , (True,(Just 4,False)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 5,True)) , (False,(Just 5,True)) , (False,(Just 5,False)) , (False,(Just 5,False)) @@ -542,7 +542,7 @@ test2R4 = testR sync2R4 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -552,7 +552,7 @@ test2R4 = testR sync2R4 , (True,(Just 2,False)) , (True,(Just 3,False)) , (True,(Just 4,False)) - , (False,(Nothing,True)) + , (False,(Just 5,True)) , (False,(Just 5,True)) , (False,(Just 5,False)) , (False,(Just 5,False)) @@ -643,7 +643,7 @@ test4R5 = testR sync4R5 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -778,7 +778,7 @@ test4R6 = testR sync4R6 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (False,(Just 1,False)) , (False,(Just 1,False)) @@ -786,7 +786,7 @@ test4R6 = testR sync4R6 , (True,(Just 2,False)) , (True,(Just 3,False)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 4,True)) , (False,(Just 4,True)) , (False,(Just 4,False)) , (False,(Just 4,False)) @@ -911,7 +911,7 @@ test6R7 = testR sync6R7 , (False,(Nothing,True)) , (False,(Nothing,True)) , (False,(Nothing,True)) - , (False,(Nothing,True)) + , (False,(Just 1,True)) , (False,(Just 1,True)) , (True,(Just 1,False)) , (False,(Just 2,True))