diff --git a/changelog/2024-11-18T14_59_34+01_00_fix2845 b/changelog/2024-11-18T14_59_34+01_00_fix2845 new file mode 100644 index 0000000000..675d46375c --- /dev/null +++ b/changelog/2024-11-18T14_59_34+01_00_fix2845 @@ -0,0 +1 @@ +FIXED: Clash generates illegal Verilog names [#2845](https://github.com/clash-lang/clash-compiler/issues/2845) diff --git a/clash-lib/src/Clash/Rewrite/WorkFree.hs b/clash-lib/src/Clash/Rewrite/WorkFree.hs index 576fdd6782..48d6417eda 100644 --- a/clash-lib/src/Clash/Rewrite/WorkFree.hs +++ b/clash-lib/src/Clash/Rewrite/WorkFree.hs @@ -155,7 +155,18 @@ isWorkFreeClockOrResetOrEnable tcm e = if isClockOrReset tcm eTy || isEnable tcm eTy then case collectArgs e of (Prim p,_) -> Just (primName p == Text.showt 'removedArg) - (Var _, []) -> Just True + -- Only local variables with a clock type are work-free. When it is a global + -- variable, it is probably backed by a clock generator, which is definitely + -- not work-free. + -- + -- Inlining let-bindings referencing a global variable with a clock type + -- can sometimes lead to the post-normalization flattening stage to generate + -- code that violates the invariants of the netlist generation stage. + -- Especially when this global binder is defined recursively such as when + -- using `tbClockGen`. + -- This then ultimately leads to bad verilog names being generated as + -- reported in: https://github.com/clash-lang/clash-compiler/issues/2845 + (Var v, []) -> Just (isLocalId v) (Data _, [_dom, Left (stripTicks -> Data _)]) -> Just True -- For Enable True/False (Literal _,_) -> Just True _ -> Just False diff --git a/tests/Main.hs b/tests/Main.hs index 5e24e96d1d..fff115e0a8 100755 --- a/tests/Main.hs +++ b/tests/Main.hs @@ -635,6 +635,7 @@ runClashTest = defaultMain $ clashTestRoot , runTest "T2781" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]} , runTest "T2628" def{hdlTargets=[VHDL], buildTargets=BuildSpecific ["TACacheServerStep"], hdlSim=[]} , runTest "T2831" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]} + , runTest "T2845" def{hdlSim=[],hdlTargets=[Verilog]} ] <> if compiledWith == Cabal then -- This tests fails without environment files present, which are only diff --git a/tests/shouldwork/Issues/T2845.hs b/tests/shouldwork/Issues/T2845.hs new file mode 100644 index 0000000000..1de593c83f --- /dev/null +++ b/tests/shouldwork/Issues/T2845.hs @@ -0,0 +1,13 @@ +module T2845 where + +import Clash.Explicit.Prelude +import Clash.Explicit.Testbench + +topEntity :: + Signal System (Unsigned 8) +topEntity = cntr + x + where + cntr = register clk noReset enableGen 0 0 + x = register clk noReset enableGen 100 0 + done = (== 100) <$> cntr + clk = tbClockGen $ not <$> done