Merge pull request #2846 from clash-lang/fix_2845

No inlining let-bound global vars with clock types

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)

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

tests/Main.hs

@@ -636,6 +636,7 @@ runClashTest = defaultMain $ clashTestRoot
         , runTest "T2628" def{hdlTargets=[VHDL], buildTargets=BuildSpecific ["TACacheServerStep"], hdlSim=[]}
         , runTest "T2831" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]}
         , runTest "T2839" 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

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