Skip to content

Commit c8ad98c

Browse files
committed
clash-prelude: Fix holdReset glitch behaviour
for async resets add `registerSyncReset`
1 parent d0f65c4 commit c8ad98c

File tree

8 files changed

+240
-25
lines changed

8 files changed

+240
-25
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ADDED: `registerSyncReset` [#3115](https://github.com/clash-lang/clash-compiler/issues/3115)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FIXED: Fix `holdReset` glitch behaviour for asynchronous resets and wrong hold cycles for sync resets. [#3115](https://github.com/clash-lang/clash-compiler/issues/3115)

clash-prelude/src/Clash/Explicit/Reset.hs

Lines changed: 107 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{-|
2-
Copyright : (C) 2020-2023, QBayLogic B.V.,
2+
Copyright : (C) 2020-2026, QBayLogic B.V.,
33
2022-2023, Google LLC
44
License : BSD2 (see the file LICENSE)
55
Maintainer : QBayLogic B.V. <devops@qbaylogic.com>
@@ -23,6 +23,7 @@ module Clash.Explicit.Reset
2323
, resetGlitchFilter
2424
, resetGlitchFilterWithReset
2525
, unsafeResetGlitchFilter
26+
, registerSyncReset
2627
, holdReset
2728
, convertReset
2829
, noReset
@@ -399,20 +400,62 @@ resetGlitchFilter# SNat reg dffSync rstIn0 =
399400
SActiveHigh -> True
400401
SActiveLow -> False
401402

402-
-- | Hold reset for a number of cycles relative to an incoming reset signal.
403+
-- | Register a synchronous reset signal.
404+
--
405+
-- `registerSyncReset` delays an incoming reset by one clock cycle using a
406+
-- register. This can be useful to break combinational paths involving reset
407+
-- logic.
408+
--
409+
-- __NB__: This is not a synchronizer. Use `resetSynchronizer` to synchronize
410+
-- a reset.
411+
--
412+
-- Example:
413+
--
414+
-- >>> let sampleReset = sampleN 7 . unsafeToActiveHigh
415+
-- >>> let rst = unsafeFromActiveHigh (fromList [False, True, False, False, True, False])
416+
-- >>> sampleReset (registerSyncReset @XilinxSystem clockGen rst enableGen True)
417+
-- [True,False,True,False,False,True,False]
418+
--
419+
registerSyncReset
420+
:: forall dom
421+
. KnownDomain dom
422+
=> HasSynchronousReset dom
423+
=> Clock dom
424+
-> Reset dom
425+
-> Enable dom
426+
-> Bool
427+
-- ^ Initial assert value of the register if supported by the domain.
428+
-- If True the initial reset value is asserted.
429+
-- If False the initial reset value is de-asserted.
430+
-> Reset dom
431+
registerSyncReset clk (unsafeFromReset -> rst) en initialValue = unsafeToReset outRst
432+
where
433+
intialRst :: Bool
434+
intialRst =
435+
case resetPolarity @dom of
436+
SActiveHigh -> initialValue
437+
SActiveLow -> not initialValue
438+
outRst = delay clk en intialRst rst
439+
440+
-- | Hold/stretch reset for a number of cycles relative to an incoming reset
441+
-- signal.
442+
--
443+
-- __NB__: The output of this function is combinational for @n > 0@ on domains
444+
-- with a synchronous reset. Use `registerSyncReset` to add an output register if
445+
-- desired.
403446
--
404447
-- Example:
405448
--
406-
-- >>> let sampleWithReset = sampleN 8 . unsafeToActiveHigh
407-
-- >>> sampleWithReset (holdReset @System clockGen enableGen (SNat @2) (resetGenN (SNat @3)))
449+
-- >>> let sampleReset = sampleN 8 . unsafeToActiveHigh
450+
-- >>> sampleReset (holdReset @System clockGen enableGen (SNat @2) (resetGenN (SNat @3)))
408451
-- [True,True,True,True,True,False,False,False]
409452
--
410453
-- 'holdReset' holds the reset for an additional 2 clock cycles for a total
411454
-- of 5 clock cycles where the reset is asserted. 'holdReset' also works on
412455
-- intermediate assertions of the reset signal:
413456
--
414457
-- >>> let rst = fromList [True, False, False, False, True, False, False, False]
415-
-- >>> sampleWithReset (holdReset @System clockGen enableGen (SNat @2) (unsafeFromActiveHigh rst))
458+
-- >>> sampleReset (holdReset @System clockGen enableGen (SNat @2) (unsafeFromActiveHigh rst))
416459
-- [True,True,True,False,True,True,True,False]
417460
--
418461
holdReset
@@ -427,11 +470,66 @@ holdReset
427470
-> Reset dom
428471
-- ^ Reset to extend
429472
-> Reset dom
430-
holdReset clk en SNat rst =
431-
unsafeFromActiveHigh ((/=maxBound) <$> counter)
473+
holdReset clk en n rst = case resetKind @dom of
474+
SSynchronous -> holdResetSync clk en n rst
475+
SAsynchronous -> holdResetAsync clk en n rst
476+
477+
holdResetSync
478+
:: forall dom n
479+
. KnownDomain dom
480+
=> DomainResetKind dom ~ 'Synchronous
481+
=> Clock dom
482+
-> Enable dom
483+
-- ^ Global enable
484+
-> SNat n
485+
-- ^ Hold for /n/ cycles
486+
-> Reset dom
487+
-- ^ Reset to extend
488+
-> Reset dom
489+
holdResetSync clk en sn@SNat rst = rstOut
490+
where
491+
rstOut = case snatToInteger sn of
492+
0 -> rst
493+
1 -> orReset rst (unsafeToReset regReset)
494+
where
495+
isActiveHigh = case resetPolarity @dom of { SActiveHigh -> True; _ -> False }
496+
regReset = register clk rst en isActiveHigh (pure (not isActiveHigh))
497+
_ -> orReset rst (unsafeToReset rawRst)
498+
where
499+
counter :: Signal dom (Index (n + 1))
500+
counter = register clk rst en 0 (satSucc SatBound <$> counter)
501+
rawRst :: Signal dom Bool
502+
rawRst = case resetPolarity @dom of
503+
SActiveHigh -> (/=maxBound) <$> counter
504+
SActiveLow -> (==maxBound) <$> counter
505+
506+
holdResetAsync
507+
:: forall dom n
508+
. KnownDomain dom
509+
=> DomainResetKind dom ~ 'Asynchronous
510+
=> Clock dom
511+
-> Enable dom
512+
-- ^ Global enable
513+
-> SNat n
514+
-- ^ Hold for /n/ cycles
515+
-> Reset dom
516+
-- ^ Reset to extend
517+
-> Reset dom
518+
holdResetAsync clk en sn@SNat rst = rstOut
432519
where
433-
counter :: Signal dom (Index (n+1))
434-
counter = register clk rst en 0 (satSucc SatBound <$> counter)
520+
isActiveHigh = case resetPolarity @dom of { SActiveHigh -> True; _ -> False }
521+
rstOut = case toUNat sn of
522+
UZero -> rst
523+
USucc UZero -> unsafeToReset (register clk rst en isActiveHigh (pure (not isActiveHigh)))
524+
USucc (USucc _) -> unsafeToReset (register clk rst en isActiveHigh rawRst)
525+
where
526+
counter :: Signal dom (Index n)
527+
counter = register clk rst en 0 (satSucc SatBound <$> counter)
528+
rawRst :: Signal dom Bool
529+
rawRst = case resetPolarity @dom of
530+
SActiveHigh -> (/=maxBound) <$> counter
531+
SActiveLow -> (==maxBound) <$> counter
532+
435533

436534
-- | Convert between different types of reset, adding a synchronizer when
437535
-- the domains are not the same. See 'resetSynchronizer' for further details

clash-prelude/src/Clash/Signal.hs

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Copyright : (C) 2013-2016, University of Twente,
33
2016-2019, Myrtle Software Ltd,
44
2017 , Google Inc.,
5-
2021-2024, QBayLogic B.V.
5+
2021-2026, QBayLogic B.V.
66
License : BSD2 (see the file LICENSE)
77
Maintainer : QBayLogic B.V. <devops@qbaylogic.com>
88
@@ -156,6 +156,7 @@ module Clash.Signal
156156
, unsafeFromActiveLow
157157
, resetSynchronizer
158158
, resetGlitchFilter
159+
, registerSyncReset
159160
, holdReset
160161
-- * Enabling
161162
, Enable
@@ -1635,22 +1636,61 @@ testFor
16351636
-> Property
16361637
testFor n s = property (and (Clash.Signal.sampleN n s))
16371638

1638-
-- | Hold reset for a number of cycles relative to an implicit reset signal.
1639+
1640+
-- | Register a synchronous reset signal.
1641+
--
1642+
-- `registerSyncReset` delays an incoming reset by one clock cycle using a
1643+
-- register. This can be useful to break combinational paths involving reset
1644+
-- logic.
1645+
--
1646+
-- __NB__: This is not a synchronizer. Use `resetSynchronizer` to synchronize
1647+
-- a reset.
1648+
--
1649+
-- Example:
1650+
--
1651+
-- >>> registerSyncResetBool = unsafeToActiveHigh . (registerSyncReset @XilinxSystem)
1652+
-- >>> let rst = unsafeFromActiveHigh (fromList [False, True, False, False, True, False])
1653+
-- >>> sampleN 7 (exposeReset (registerSyncResetBool True) rst)
1654+
-- [True,False,True,False,False,True,False]
1655+
--
1656+
registerSyncReset
1657+
:: forall dom
1658+
. HiddenClockResetEnable dom
1659+
=> KnownDomain dom
1660+
=> DomainResetKind dom ~ 'Synchronous
1661+
=> Bool
1662+
-- ^ Initial assert value of the register if supported by the domain.
1663+
-- If True the initial reset value is asserted.
1664+
-- If False the initial reset value is de-asserted.
1665+
-> Reset dom
1666+
registerSyncReset initialValue = hideClockResetEnable E.registerSyncReset initialValue
1667+
1668+
-- | Hold/stretch reset for a number of cycles relative to an incoming reset
1669+
-- signal.
1670+
--
1671+
-- __NB__: The output of this function is combinational for @n > 1@ on domains
1672+
-- with a synchronous reset. Use `registerSyncReset` to add an output register if
1673+
-- desired.
16391674
--
16401675
-- Example:
16411676
--
1642-
-- >>> sampleN @System 8 (unsafeToActiveHigh (holdReset (SNat @2)))
1643-
-- [True,True,True,False,False,False,False,False]
1677+
-- >>> holdResetBool = unsafeToActiveHigh . (holdReset @System)
1678+
-- >>> sampleN 8 (exposeReset (holdResetBool (SNat @2)) (resetGenN (SNat @3)))
1679+
-- [True,True,True,True,True,False,False,False]
16441680
--
16451681
-- 'holdReset' holds the reset for an additional 2 clock cycles for a total
1646-
-- of 3 clock cycles where the reset is asserted.
1682+
-- of 5 clock cycles where the reset is asserted. 'holdReset' also works on
1683+
-- intermediate assertions of the reset signal:
1684+
--
1685+
-- >>> let rst = fromList [True, False, False, False, True, False, False, False]
1686+
-- >>> sampleN 8 (exposeReset (holdResetBool (SNat @2)) (unsafeFromActiveHigh rst))
1687+
-- [True,True,True,False,True,True,True,False]
16471688
--
16481689
holdReset
16491690
:: forall dom m
16501691
. HiddenClockResetEnable dom
16511692
=> SNat m
1652-
-- ^ Hold for /m/ cycles, counting from the moment the incoming reset
1653-
-- signal becomes deasserted.
1693+
-- ^ Hold for /m/ cycles
16541694
-> Reset dom
16551695
holdReset m =
16561696
hideClockResetEnable (\clk rst en -> E.holdReset clk en m rst)

tests/Main.hs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,8 @@ runClashTest = defaultMain
802802
, "testBench53"]}
803803
in runTest "RWMultiTop" _opts
804804
]
805+
, runTest "HoldResetAsync" def
806+
, runTest "HoldResetSync" def
805807
, runTest "ResetGen" def
806808
,
807809
-- TODO: we do not support memory files in Vivado
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{-# LANGUAGE CPP #-}
2+
3+
module HoldResetAsync where
4+
5+
import Clash.Explicit.Prelude
6+
import Clash.Explicit.Testbench
7+
8+
topEntity
9+
:: Clock XilinxSystem
10+
-> Signal XilinxSystem (Bool, Bool, Bool, Bool)
11+
topEntity clk = bundle (rBool, r0, r1, r2)
12+
where
13+
r = resetGenN (SNat @3)
14+
rBool = unsafeToActiveHigh r
15+
r0 = unsafeToActiveHigh (holdReset clk enableGen (SNat @0) r)
16+
r1 = unsafeToActiveHigh (holdReset clk enableGen (SNat @1) r)
17+
r2 = unsafeToActiveHigh (holdReset clk enableGen (SNat @2) r)
18+
{-# OPAQUE topEntity #-}
19+
20+
testBench :: Signal XilinxSystem Bool
21+
testBench = done
22+
where
23+
expectedOutput =
24+
outputVerifier'
25+
clk
26+
rst
27+
-- Note that outputVerifier' skips first sample
28+
( (True, True, True, True)
29+
:> (True, True, True, True)
30+
:> (False, False, True, True)
31+
:> (False, False, False, True)
32+
:> (False, False, False, False)
33+
:> Nil )
34+
35+
done = expectedOutput (topEntity clk)
36+
clk = tbClockGen (not <$> done)
37+
rst = resetGen
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{-# LANGUAGE CPP #-}
2+
3+
module HoldResetSync where
4+
5+
import Clash.Explicit.Prelude
6+
import Clash.Explicit.Testbench
7+
8+
topEntity
9+
:: Clock System
10+
-> Signal System (Bool, Bool, Bool, Bool)
11+
topEntity clk = bundle (rBool, r0, r1, r2)
12+
where
13+
r = resetGenN (SNat @3)
14+
rBool = unsafeToActiveHigh r
15+
r0 = unsafeToActiveHigh (holdReset clk enableGen (SNat @0) r)
16+
r1 = unsafeToActiveHigh (holdReset clk enableGen (SNat @1) r)
17+
r2 = unsafeToActiveHigh (holdReset clk enableGen (SNat @2) r)
18+
{-# OPAQUE topEntity #-}
19+
20+
testBench :: Signal System Bool
21+
testBench = done
22+
where
23+
expectedOutput =
24+
outputVerifier'
25+
clk
26+
rst
27+
-- Note that outputVerifier' skips first sample
28+
( (True, True, True, True)
29+
:> (True, True, True, True)
30+
:> (False, False, True, True)
31+
:> (False, False, False, True)
32+
:> (False, False, False, False)
33+
:> Nil )
34+
35+
done = expectedOutput (topEntity clk)
36+
clk = tbClockGen (not <$> done)
37+
rst = resetGen

tests/shouldwork/Signal/ResetGen.hs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ import Clash.Explicit.Testbench
77

88
topEntity
99
:: Clock System
10-
-> Signal System (Bool, Bool)
11-
topEntity clk = bundle (unsafeToActiveHigh r, unsafeToActiveHigh r')
10+
-> Signal System Bool
11+
topEntity clk = bundle (unsafeToActiveHigh r)
1212
where
1313
r = resetGenN (SNat @3)
14-
r' = holdReset clk enableGen (SNat @2) r
1514
{-# OPAQUE topEntity #-}
1615

1716
testBench :: Signal System Bool
@@ -22,12 +21,12 @@ testBench = done
2221
clk
2322
rst
2423
-- Note that outputVerifier' skips first sample
25-
( (True, True)
26-
:> (True, True)
27-
:> (False, True)
28-
:> (False, True)
29-
:> (False, False)
30-
:> (False, False)
24+
( True
25+
:> True
26+
:> False
27+
:> False
28+
:> False
29+
:> False
3130
:> Nil )
3231

3332
done = expectedOutput (topEntity clk)

0 commit comments

Comments
 (0)