Skip to content

Commit 95352c6

Browse files
mergify[bot]rowanG077
mergify[bot]
and
rowanG077
authored
clash-prelude: Fix holdReset glitch behaviour (#3116) (#3145)
for async resets add `registerSyncReset` (cherry picked from commit b1b1477) # Conflicts: # clash-prelude/src/Clash/Signal.hs # tests/shouldwork/Signal/ResetGen.hs Co-authored-by: Rowan Goemans <goemansrowan@gmail.com>
1 parent a00e22e commit 95352c6

File tree

8 files changed

+239
-24
lines changed

8 files changed

+239
-24
lines changed

‎changelog/2026-01-08T16_58_32+01_00_add_registerSyncReset‎

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)

‎changelog/2026-01-08T16_58_32+01_00_fix_holdreset‎

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 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: 46 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-2023, 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
@@ -152,6 +152,7 @@ module Clash.Signal
152152
#endif
153153
, resetSynchronizer
154154
, resetGlitchFilter
155+
, registerSyncReset
155156
, holdReset
156157
-- * Enabling
157158
, Enable
@@ -2272,22 +2273,60 @@ unsafeSynchronizer =
22722273
hideClock (hideClock E.unsafeSynchronizer)
22732274
#endif
22742275

2275-
-- | Hold reset for a number of cycles relative to an implicit reset signal.
2276+
-- | Register a synchronous reset signal.
2277+
--
2278+
-- `registerSyncReset` delays an incoming reset by one clock cycle using a
2279+
-- register. This can be useful to break combinational paths involving reset
2280+
-- logic.
2281+
--
2282+
-- __NB__: This is not a synchronizer. Use `resetSynchronizer` to synchronize
2283+
-- a reset.
2284+
--
2285+
-- Example:
2286+
--
2287+
-- >>> registerSyncResetBool = unsafeToActiveHigh . (registerSyncReset @XilinxSystem)
2288+
-- >>> let rst = unsafeFromActiveHigh (fromList [False, True, False, False, True, False])
2289+
-- >>> sampleN 7 (exposeReset (registerSyncResetBool True) rst)
2290+
-- [True,False,True,False,False,True,False]
2291+
--
2292+
registerSyncReset
2293+
:: forall dom
2294+
. HiddenClockResetEnable dom
2295+
=> KnownDomain dom
2296+
=> DomainResetKind dom ~ 'Synchronous
2297+
=> Bool
2298+
-- ^ Initial assert value of the register if supported by the domain.
2299+
-- If True the initial reset value is asserted.
2300+
-- If False the initial reset value is de-asserted.
2301+
-> Reset dom
2302+
registerSyncReset initialValue = hideClockResetEnable E.registerSyncReset initialValue
2303+
2304+
-- | Hold reset for a number of cycles relative to an incoming reset
2305+
-- signal.
2306+
--
2307+
-- __NB__: The output of this function is combinational for @n > 1@ on domains
2308+
-- with a synchronous reset. Use `registerSyncReset` to add an output register if
2309+
-- desired.
22762310
--
22772311
-- Example:
22782312
--
2279-
-- >>> sampleN @System 8 (unsafeToActiveHigh (holdReset (SNat @2)))
2280-
-- [True,True,True,False,False,False,False,False]
2313+
-- >>> holdResetBool = unsafeToActiveHigh . (holdReset @System)
2314+
-- >>> sampleN 8 (exposeReset (holdResetBool (SNat @2)) (resetGenN (SNat @3)))
2315+
-- [True,True,True,True,True,False,False,False]
22812316
--
22822317
-- 'holdReset' holds the reset for an additional 2 clock cycles for a total
2283-
-- of 3 clock cycles where the reset is asserted.
2318+
-- of 5 clock cycles where the reset is asserted. 'holdReset' also works on
2319+
-- intermediate assertions of the reset signal:
2320+
--
2321+
-- >>> let rst = fromList [True, False, False, False, True, False, False, False]
2322+
-- >>> sampleN 8 (exposeReset (holdResetBool (SNat @2)) (unsafeFromActiveHigh rst))
2323+
-- [True,True,True,False,True,True,True,False]
22842324
--
22852325
holdReset
22862326
:: forall dom m
22872327
. HiddenClockResetEnable dom
22882328
=> SNat m
2289-
-- ^ Hold for /m/ cycles, counting from the moment the incoming reset
2290-
-- signal becomes deasserted.
2329+
-- ^ Hold for /m/ cycles
22912330
-> Reset dom
22922331
holdReset m =
22932332
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
@@ -979,6 +979,8 @@ runClashTest = defaultMain
979979
, "testBench53"]}
980980
in runTest "RWMultiTop" _opts
981981
]
982+
, runTest "HoldResetAsync" def
983+
, runTest "HoldResetSync" def
982984
, runTest "ResetGen" def
983985
,
984986
-- TODO: we do not support memory files in Vivado

‎tests/shouldwork/Signal/HoldResetAsync.hs‎

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

‎tests/shouldwork/Signal/HoldResetSync.hs‎

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 & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ 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)
1414
r' = holdReset clk enableGen (SNat @2) r
@@ -23,12 +23,12 @@ testBench = done
2323
clk
2424
rst
2525
-- Note that outputVerifier' skips first sample
26-
( (True, True)
27-
:> (True, True)
28-
:> (False, True)
29-
:> (False, True)
30-
:> (False, False)
31-
:> (False, False)
26+
( True
27+
:> True
28+
:> False
29+
:> False
30+
:> False
31+
:> False
3232
:> Nil )
3333

3434
done = expectedOutput (topEntity clk)

0 commit comments

Comments
 (0)